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(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(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 f5477f30b3..d6a4094520 100644 --- 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 c808b4818f..f42ed2504b 100644 --- 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 index 0000000000..31bb3f8150 --- /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}}