[cfe-commits] r59196 - in /cfe/trunk: lib/AST/Expr.cpp lib/AST/ExprConstant.cpp test/Sema/const-eval.c

[Daniel Dunbar]

Author: ddunbar
Date: Wed Nov 12 18:03:19 2008
New Revision: 59196

URL: http://llvm.org/viewvc/llvm-project?rev=59196&view=rev
Log:
Fix bug in constant evaluation exposed by 176.gcc.
 - Evaluation of , operator used bogus assumption that LHS could be
   evaluated as an integral expression even though its type is
   unspecified.

This change is making isICE very permissive of the LHS in non-evaluated 
contexts because it is not clear what predicate we would use to reject 
code here. The standard didn't offer me any guidance; opinions?

Modified:
    cfe/trunk/lib/AST/Expr.cpp
    cfe/trunk/lib/AST/ExprConstant.cpp
    cfe/trunk/test/Sema/const-eval.c

Modified: cfe/trunk/lib/AST/Expr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=59196&r1=59195&r2=59196&view=diff

==============================================================================
--- cfe/trunk/lib/AST/Expr.cpp (original)
+++ cfe/trunk/lib/AST/Expr.cpp Wed Nov 12 18:03:19 2008
@@ -870,10 +870,26 @@
     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;
@@ -945,20 +961,6 @@
     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!");

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=59196&r1=59195&r2=59196&view=diff

==============================================================================
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Wed Nov 12 18:03:19 2008
@@ -376,6 +376,7 @@
   }
   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);
 
@@ -480,7 +481,37 @@
   }
 }
 
+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())) {
@@ -585,22 +616,7 @@
     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;

Modified: cfe/trunk/test/Sema/const-eval.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/const-eval.c?rev=59196&r1=59195&r2=59196&view=diff

==============================================================================
--- cfe/trunk/test/Sema/const-eval.c (original)
+++ cfe/trunk/test/Sema/const-eval.c Wed Nov 12 18:03:19 2008
@@ -11,3 +11,5 @@
 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}}