Backout of r59196, plus a new ICE test. Sorry if this is a

little rude; I figure it's cleaner to just back this out now so
it doesn't get forgotten or mixed up with other checkins.

The modification to isICE is simply wrong; I've added a test that the
change to isICE breaks.

I'm pretty sure the modification to tryEvaluate is also wrong.
At the very least, there's some serious miscommunication going on here,
as this is going in exactly the opposite direction of r59105.  My
understanding is that tryEvaluate is not supposed to care about side
effects.  That said, a lot of the clients to tryEvaluate are
expecting it to enforce a no-side-effects policy, so we probably need
another method that provides that guarantee.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@59212 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp
index e9903df61288f9f92981b462cb9ea91a372b1afd..c54fc40ecb268bb5e943b2904bb98149aa47127f 100644 (file)
--- a/lib/AST/Expr.cpp
+++ b/lib/AST/Expr.cpp
@@ -870,26 +870,10 @@ bool Expr::isIntegerConstantExpr(llvm::APSInt &Result, ASTContext &Ctx,
     const BinaryOperator *Exp = cast<BinaryOperator>(this);
     llvm::APSInt LHS, RHS;
 
-    // Comma operator requires special handling.
-    if (Exp->getOpcode() == BinaryOperator::Comma) {
-      // C99 6.6p3: "shall not contain assignment, ..., or comma operators,
-      // *except* when they are contained within a subexpression that is not
-      // evaluated".  Note that Assignment can never happen due to constraints
-      // on the LHS subexpr, so we don't need to check it here.
-      if (isEvaluated) {
-        if (Loc) *Loc = getLocStart();
-        return false;
-      }
-
-      // The result of the constant expr is the RHS.
-      return Exp->getRHS()->isIntegerConstantExpr(Result, Ctx, Loc, 
-                                                  isEvaluated);
-    }
-    
     // Initialize result to have correct signedness and width.
     Result = llvm::APSInt(static_cast<uint32_t>(Ctx.getTypeSize(getType())),
-                          !getType()->isSignedIntegerType());
-
+                          !getType()->isSignedIntegerType());                          
+    
     // The LHS of a constant expr is always evaluated and needed.
     if (!Exp->getLHS()->isIntegerConstantExpr(LHS, Ctx, Loc, isEvaluated))
       return false;
@@ -961,6 +945,20 @@ bool Expr::isIntegerConstantExpr(llvm::APSInt &Result, ASTContext &Ctx,
     case BinaryOperator::LOr:
       Result = LHS != 0 || RHS != 0;
       break;
+      
+    case BinaryOperator::Comma:
+      // C99 6.6p3: "shall not contain assignment, ..., or comma operators,
+      // *except* when they are contained within a subexpression that is not
+      // evaluated".  Note that Assignment can never happen due to constraints
+      // on the LHS subexpr, so we don't need to check it here.
+      if (isEvaluated) {
+        if (Loc) *Loc = getLocStart();
+        return false;
+      }
+      
+      // The result of the constant expr is the RHS.
+      Result = RHS;
+      return true;
     }
     
     assert(!Exp->isAssignmentOp() && "LHS can't be a constant expr!");

diff --git a/lib/AST/ExprConstant.cpp b/lib/AST/ExprConstant.cpp
index f5477f30b30b78dce62f7b09984b4baafdade88d..d6a4094520da2449aaf91a782fdd0eac65dc1f12 100644 (file)
--- a/lib/AST/ExprConstant.cpp
+++ b/lib/AST/ExprConstant.cpp
@@ -376,7 +376,6 @@ public:
   }
   bool VisitDeclRefExpr(const DeclRefExpr *E);
   bool VisitCallExpr(const CallExpr *E);
-  bool VisitBinOpComma(const BinaryOperator *E);
   bool VisitBinaryOperator(const BinaryOperator *E);
   bool VisitUnaryOperator(const UnaryOperator *E);
 
@@ -481,37 +480,7 @@ bool IntExprEvaluator::VisitCallExpr(const CallExpr *E) {
   }
 }
 
-bool IntExprEvaluator::VisitBinOpComma(const BinaryOperator *E) {
-  llvm::APSInt RHS(32);
-
-  // Require that we be able to evaluate the LHS.
-  if (!E->getLHS()->isEvaluatable(Info.Ctx))
-    return false;
-
-  bool OldEval = Info.isEvaluated;
-  if (!EvaluateInteger(E->getRHS(), RHS, Info))
-    return false;
-  Info.isEvaluated = OldEval;
-
-  // Result of the comma is just the result of the RHS.
-  Result = RHS;
-  
-  // C99 6.6p3: "shall not contain assignment, ..., or comma operators,
-  // *except* when they are contained within a subexpression that is not
-  // evaluated".  Note that Assignment can never happen due to constraints
-  // on the LHS subexpr, so we don't need to check it here.
-  if (!Info.isEvaluated)
-    return true;
-
-  // If the value is evaluated, we can accept it as an extension.
-  return Extension(E->getOperatorLoc(), diag::ext_comma_in_constant_expr);
-}
-
 bool IntExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) {
-  // Comma operator requires special handling.
-  if (E->getOpcode() == BinaryOperator::Comma)
-    return VisitBinOpComma(E);
-  
   // The LHS of a constant expr is always evaluated and needed.
   llvm::APSInt RHS(32);
   if (!Visit(E->getLHS())) {
@@ -616,7 +585,22 @@ bool IntExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) {
     Result = Result != 0 || RHS != 0;
     Result.zextOrTrunc(getIntTypeSizeInBits(E->getType()));
     break;
-}
+      
+    
+  case BinaryOperator::Comma:
+    // Result of the comma is just the result of the RHS.
+    Result = RHS;
+
+    // C99 6.6p3: "shall not contain assignment, ..., or comma operators,
+    // *except* when they are contained within a subexpression that is not
+    // evaluated".  Note that Assignment can never happen due to constraints
+    // on the LHS subexpr, so we don't need to check it here.
+    if (!Info.isEvaluated)
+      return true;
+      
+    // If the value is evaluated, we can accept it as an extension.
+    return Extension(E->getOperatorLoc(), diag::ext_comma_in_constant_expr);
+  }
 
   Result.setIsUnsigned(E->getType()->isUnsignedIntegerType());
   return true;

diff --git a/test/Sema/const-eval.c b/test/Sema/const-eval.c
index c808b4818f7ca05daaa41f0db2c59bdfa73d6677..f42ed2504b19644fe48c2b06bb6bca4eded2dc4c 100644 (file)
--- a/test/Sema/const-eval.c
+++ b/test/Sema/const-eval.c
@@ -11,5 +11,3 @@ struct y {int x,y;};
 EVAL_EXPR(6, (int)(1+(struct y*)0))
 EVAL_EXPR(7, (int)&((struct y*)0)->y)
 EVAL_EXPR(8, (_Bool)"asdf")
-void g0(void);
-EVAL_EXPR(9, (g0(), 12)) // expected-error {{fields must have a constant size}}

diff --git a/test/Sema/i-c-e3.c b/test/Sema/i-c-e3.c
new file mode 100644 (file)
index 0000000..31bb3f8
--- /dev/null
+++ b/test/Sema/i-c-e3.c
@@ -0,0 +1,3 @@
+// RUN: clang %s -fsyntax-only -verify -pedantic
+
+int a() {int p; *(1 ? &p : (void*)(0 && (a(),1))) = 10;} // expected-error {{not assignable}}