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

1 files changed, 16 insertions, 18 deletions

diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp
index e9903df612..c54fc40ecb 100644
--- 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!");