From 51fe996231b1d7199f76e4005ff4c943d5deeecd Mon Sep 17 00:00:00 2001
From: Anders Carlsson <andersca@mac.com>
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 387ff7111b0..77d6d5f86dd 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 6b71d110043..35cec0f3442 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 3f2cd803dab..0a67e57e343 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 0b05177dd37..ef24d1a34a8 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 17495a837a6..6839310e0ec 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 0e39175817e..fd8e0464be6 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;
+ }
+}
+
--
GitLab