From 51fe996231b1d7199f76e4005ff4c943d5deeecd Mon Sep 17 00:00:00 2001 From: Anders Carlsson Date: Sat, 22 Nov 2008 21:04:56 +0000 Subject: [PATCH] Use Expr::Evaluate for case statements. Fixes PR2525 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@59881 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/Expr.h | 4 ++++ lib/AST/ExprConstant.cpp | 36 +++++++++++++++++++++++++++--------- lib/CodeGen/CGStmt.cpp | 6 +++--- lib/Sema/SemaDecl.cpp | 2 +- lib/Sema/SemaStmt.cpp | 17 +++++++++++++---- test/Sema/switch.c | 22 ++++++++++++++++++++++ 6 files changed, 70 insertions(+), 17 deletions(-) diff --git a/include/clang/AST/Expr.h b/include/clang/AST/Expr.h index 387ff7111b..77d6d5f86d 100644 --- a/include/clang/AST/Expr.h +++ b/include/clang/AST/Expr.h @@ -147,6 +147,10 @@ public: /// folded, but discard the result. bool isEvaluatable(ASTContext &Ctx) const; + /// EvaluateAsInt - Call Evaluate and return the folded integer. This + /// must be called on an expression that constant folds to an integer. + llvm::APSInt EvaluateAsInt(ASTContext &Ctx) const; + /// hasGlobalStorage - Return true if this expression has static storage /// duration. This means that the address of this expression is a link-time /// constant. diff --git a/lib/AST/ExprConstant.cpp b/lib/AST/ExprConstant.cpp index 6b71d11004..35cec0f344 100644 --- a/lib/AST/ExprConstant.cpp +++ b/lib/AST/ExprConstant.cpp @@ -537,19 +537,28 @@ bool IntExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) { // These need to be handled specially because the operands aren't // necessarily integral bool bres; - if (!HandleConversionToBool(E->getLHS(), bres, Info)) { + + if (HandleConversionToBool(E->getLHS(), bres, Info)) { + // We were able to evaluate the LHS, see if we can get away with not + // evaluating the RHS: 0 && X -> 0, 1 || X -> 1 + } else { // We can't evaluate the LHS; however, sometimes the result // is determined by the RHS: X && 0 -> 0, X || 1 -> 1. - if (HandleConversionToBool(E->getRHS(), bres, Info) && - bres == (E->getOpcode() == BinaryOperator::LOr)) { - Result.zextOrTrunc(getIntTypeSizeInBits(E->getType())); - Result.setIsUnsigned(E->getType()->isUnsignedIntegerType()); - Result = bres; - return true; + if (!HandleConversionToBool(E->getRHS(), bres, Info)) { + // We can't evaluate. + return false; } + } - // Really can't evaluate - return false; + // FIXME: If we evaluate the RHS, we need to check if the LHS has + // any side effects. + + if (bres == (E->getOpcode() == BinaryOperator::LOr) || + !bres == (E->getOpcode() == BinaryOperator::LAnd)) { + Result.zextOrTrunc(getIntTypeSizeInBits(E->getType())); + Result.setIsUnsigned(E->getType()->isUnsignedIntegerType()); + Result = bres; + return true; } bool bres2; @@ -1176,3 +1185,12 @@ bool Expr::isEvaluatable(ASTContext &Ctx) const { APValue V; return Evaluate(V, Ctx); } + +APSInt Expr::EvaluateAsInt(ASTContext &Ctx) const { + APValue V; + bool Result = Evaluate(V, Ctx); + assert(Result && "Could not evaluate expression"); + assert(V.isInt() && "Expression did not evaluate to integer"); + + return V.getInt(); +} diff --git a/lib/CodeGen/CGStmt.cpp b/lib/CodeGen/CGStmt.cpp index 3f2cd803da..0a67e57e34 100644 --- a/lib/CodeGen/CGStmt.cpp +++ b/lib/CodeGen/CGStmt.cpp @@ -533,8 +533,8 @@ void CodeGenFunction::EmitContinueStmt(const ContinueStmt &S) { void CodeGenFunction::EmitCaseStmtRange(const CaseStmt &S) { assert(S.getRHS() && "Expected RHS value in CaseStmt"); - llvm::APSInt LHS = S.getLHS()->getIntegerConstantExprValue(getContext()); - llvm::APSInt RHS = S.getRHS()->getIntegerConstantExprValue(getContext()); + llvm::APSInt LHS = S.getLHS()->EvaluateAsInt(getContext()); + llvm::APSInt RHS = S.getRHS()->EvaluateAsInt(getContext()); // Emit the code for this case. We do this first to make sure it is // properly chained from our predecessor before generating the @@ -594,7 +594,7 @@ void CodeGenFunction::EmitCaseStmt(const CaseStmt &S) { EmitBlock(createBasicBlock("sw.bb")); llvm::BasicBlock *CaseDest = Builder.GetInsertBlock(); - llvm::APSInt CaseVal = S.getLHS()->getIntegerConstantExprValue(getContext()); + llvm::APSInt CaseVal = S.getLHS()->EvaluateAsInt(getContext()); SwitchInsn->addCase(llvm::ConstantInt::get(CaseVal), CaseDest); EmitStmt(S.getSubStmt()); } diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index 0b05177dd3..ef24d1a34a 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -1488,7 +1488,7 @@ static const Expr* FindExpressionBaseAddress(const Expr* E) { } } -bool Sema::CheckArithmeticConstantExpression(const Expr* Init) { +bool Sema::CheckArithmeticConstantExpression(const Expr* Init) { switch (Init->getStmtClass()) { default: InitializerElementNotConstant(Init); diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp index 17495a837a..6839310e0e 100644 --- a/lib/Sema/SemaStmt.cpp +++ b/lib/Sema/SemaStmt.cpp @@ -12,6 +12,7 @@ //===----------------------------------------------------------------------===// #include "Sema.h" +#include "clang/AST/APValue.h" #include "clang/AST/ASTContext.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/Expr.h" @@ -127,14 +128,22 @@ Sema::ActOnCaseStmt(SourceLocation CaseLoc, ExprTy *lhsval, SourceLocation ExpLoc; // C99 6.8.4.2p3: The expression shall be an integer constant. - if (!LHSVal->isIntegerConstantExpr(Context, &ExpLoc)) { + // However, GCC allows any evaluatable integer expression. + // FIXME: Should we warn if this is evaluatable but not an I-C-E? + APValue Result; + + if (!LHSVal->Evaluate(Result, Context) || !Result.isInt()) { + // FIXME: Evaluate doesn't return the SourceLocation that it failed to + // evaluate. + ExpLoc = LHSVal->getExprLoc(); Diag(ExpLoc, diag::err_case_label_not_integer_constant_expr) << LHSVal->getSourceRange(); return SubStmt; } // GCC extension: The expression shall be an integer constant. - if (RHSVal && !RHSVal->isIntegerConstantExpr(Context, &ExpLoc)) { + if (RHSVal && !RHSVal->Evaluate(Result, Context) || !Result.isInt()) { + ExpLoc = RHSVal->getExprLoc(); Diag(ExpLoc, diag::err_case_label_not_integer_constant_expr) << RHSVal->getSourceRange(); RHSVal = 0; // Recover by just forgetting about it. @@ -394,7 +403,7 @@ Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, StmtTy *Switch, // We already verified that the expression has a i-c-e value (C99 // 6.8.4.2p3) - get that value now. Expr *Lo = CS->getLHS(); - llvm::APSInt LoVal = Lo->getIntegerConstantExprValue(Context); + llvm::APSInt LoVal = Lo->EvaluateAsInt(Context); // Convert the value to the same width/sign as the condition. ConvertIntegerToTypeWarnOnOverflow(LoVal, CondWidth, CondIsSigned, @@ -444,7 +453,7 @@ Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, StmtTy *Switch, for (unsigned i = 0, e = CaseRanges.size(); i != e; ++i) { CaseStmt *CR = CaseRanges[i].second; Expr *Hi = CR->getRHS(); - llvm::APSInt HiVal = Hi->getIntegerConstantExprValue(Context); + llvm::APSInt HiVal = Hi->EvaluateAsInt(Context); // Convert the value to the same width/sign as the condition. ConvertIntegerToTypeWarnOnOverflow(HiVal, CondWidth, CondIsSigned, diff --git a/test/Sema/switch.c b/test/Sema/switch.c index 0e39175817..fd8e0464be 100644 --- a/test/Sema/switch.c +++ b/test/Sema/switch.c @@ -28,3 +28,25 @@ void test3(void) { switch (0); } +extern int g(); + +void test4() +{ + switch (1) { + case 0 && g(): + case 1 || g(): + break; + } + + switch(1) { + case g(): // expected-error {{case label does not reduce to an integer constant}} + case 0 ... g(): // expected-error {{case label does not reduce to an integer constant}} + break; + } + + switch (1) { + case 0 && g() ... 1 || g(): + break; + } +} + -- 2.40.0