From e0afc98742422e99f7ed4c5f82812ab64781d7e7 Mon Sep 17 00:00:00 2001 From: Eli Friedman Date: Sat, 21 Jan 2012 01:01:51 +0000 Subject: [PATCH] Make clang's AST model sizeof and typeof with potentially-evaluated operands correctly, similar to what we already do with typeid. llvm-svn: 148610 --- clang/include/clang/Sema/Sema.h | 1 + clang/lib/Parse/ParseDecl.cpp | 16 ++++++++++++++-- clang/lib/Parse/ParseExpr.cpp | 18 ++---------------- clang/lib/Sema/AnalysisBasedWarnings.cpp | 8 ++++++-- clang/lib/Sema/SemaExpr.cpp | 12 ++++++++++++ clang/test/Sema/i-c-e.c | 17 +++++++---------- clang/test/Sema/vla-2.c | 17 +++++++++++++++++ clang/test/SemaCXX/vararg-non-pod.cpp | 5 +++++ 8 files changed, 64 insertions(+), 30 deletions(-) create mode 100644 clang/test/Sema/vla-2.c diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index b6126033dab2..9735a0959812 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -2256,6 +2256,7 @@ public: void DiscardCleanupsInEvaluationContext(); ExprResult TranformToPotentiallyEvaluated(Expr *E); + ExprResult HandleExprEvaluationContextForTypeof(Expr *E); void MarkDeclarationReferenced(SourceLocation Loc, Decl *D); void MarkDeclarationsReferencedInType(SourceLocation Loc, QualType T); diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp index ad31cbd64312..cf90105655d9 100644 --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -4607,10 +4607,13 @@ void Parser::ParseBracketDeclarator(Declarator &D) { // Parse the constant-expression or assignment-expression now (depending // on dialect). - if (getLang().CPlusPlus) + if (getLang().CPlusPlus) { NumElements = ParseConstantExpression(); - else + } else { + EnterExpressionEvaluationContext Unevaluated(Actions, + Sema::ConstantEvaluated); NumElements = ParseAssignmentExpression(); + } } // If there was an error parsing the assignment-expression, recover. @@ -4647,6 +4650,8 @@ void Parser::ParseTypeofSpecifier(DeclSpec &DS) { const bool hasParens = Tok.is(tok::l_paren); + EnterExpressionEvaluationContext Unevaluated(Actions, Sema::Unevaluated); + bool isCastExpr; ParsedType CastTy; SourceRange CastRange; @@ -4682,6 +4687,13 @@ void Parser::ParseTypeofSpecifier(DeclSpec &DS) { return; } + // We might need to transform the operand if it is potentially evaluated. + Operand = Actions.HandleExprEvaluationContextForTypeof(Operand.get()); + if (Operand.isInvalid()) { + DS.SetTypeSpecError(); + return; + } + const char *PrevSpec = 0; unsigned DiagID; // Check for duplicate type specifiers (e.g. "int typeof(int)"). diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp index c766bad110d6..bdebb5e6b949 100644 --- a/clang/lib/Parse/ParseExpr.cpp +++ b/clang/lib/Parse/ParseExpr.cpp @@ -1442,14 +1442,6 @@ Parser::ParseExprAfterUnaryExprOrTypeTrait(const Token &OpTok, return ExprError(); } - // C++0x [expr.sizeof]p1: - // [...] The operand is either an expression, which is an unevaluated - // operand (Clause 5) [...] - // - // The GNU typeof and GNU/C++11 alignof extensions also behave as - // unevaluated operands. - EnterExpressionEvaluationContext Unevaluated(Actions, - Sema::Unevaluated); Operand = ParseCastExpression(true/*isUnaryExpression*/); } else { // If it starts with a '(', we know that it is either a parenthesized @@ -1459,14 +1451,6 @@ Parser::ParseExprAfterUnaryExprOrTypeTrait(const Token &OpTok, ParenParseOption ExprType = CastExpr; SourceLocation LParenLoc = Tok.getLocation(), RParenLoc; - // C++0x [expr.sizeof]p1: - // [...] The operand is either an expression, which is an unevaluated - // operand (Clause 5) [...] - // - // The GNU typeof and GNU/C++11 alignof extensions also behave as - // unevaluated operands. - EnterExpressionEvaluationContext Unevaluated(Actions, - Sema::Unevaluated); Operand = ParseParenExpression(ExprType, true/*stopIfCastExpr*/, false, CastTy, RParenLoc); CastRange = SourceRange(LParenLoc, RParenLoc); @@ -1555,6 +1539,8 @@ ExprResult Parser::ParseUnaryExprOrTypeTraitExpression() { if (OpTok.is(tok::kw_alignof)) Diag(OpTok, diag::warn_cxx98_compat_alignof); + EnterExpressionEvaluationContext Unevaluated(Actions, Sema::Unevaluated); + bool isCastExpr; ParsedType CastTy; SourceRange CastRange; diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 9b3191b02732..e2d1e8c33156 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -849,8 +849,12 @@ AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P, bool processed = false; if (const Stmt *stmt = i->stmt) { const CFGBlock *block = AC.getBlockForRegisteredExpression(stmt); - assert(block); - if (CFGReverseBlockReachabilityAnalysis *cra = AC.getCFGReachablityAnalysis()) { + CFGReverseBlockReachabilityAnalysis *cra = + AC.getCFGReachablityAnalysis(); + // FIXME: We should be able to assert that block is non-null, but + // the CFG analysis can skip potentially-evaluated expressions in + // edge cases; see test/Sema/vla-2.c. + if (block && cra) { // Can this block be reached from the entrance? if (cra->isReachable(&AC.getCFG()->getEntry(), block)) S.Diag(D.Loc, D.PD); diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index be6b9653d520..a7910aa28e2f 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -3012,6 +3012,12 @@ Sema::CreateUnaryExprOrTypeTraitExpr(Expr *E, SourceLocation OpLoc, if (isInvalid) return ExprError(); + if (ExprKind == UETT_SizeOf && E->getType()->isVariableArrayType()) { + PE = TranformToPotentiallyEvaluated(E); + if (PE.isInvalid()) return ExprError(); + E = PE.take(); + } + // C99 6.5.3.4p4: the type (an unsigned integer type) is size_t. return Owned(new (Context) UnaryExprOrTypeTraitExpr( ExprKind, E, Context.getSizeType(), OpLoc, @@ -9435,6 +9441,12 @@ void Sema::DiscardCleanupsInEvaluationContext() { ExprNeedsCleanups = false; } +ExprResult Sema::HandleExprEvaluationContextForTypeof(Expr *E) { + if (!E->getType()->isVariablyModifiedType()) + return E; + return TranformToPotentiallyEvaluated(E); +} + /// \brief Note that the given declaration was referenced in the source code. /// /// This routine should be invoke whenever a given declaration is referenced diff --git a/clang/test/Sema/i-c-e.c b/clang/test/Sema/i-c-e.c index 210a811dc5d8..a25bc1fec59f 100644 --- a/clang/test/Sema/i-c-e.c +++ b/clang/test/Sema/i-c-e.c @@ -54,24 +54,21 @@ char y[__builtin_constant_p(expr) ? -1 : 1]; char z[__builtin_constant_p(4) ? 1 : -1]; // Comma tests -int comma1[0?1,2:3]; // expected-warning {{expression result unused}} -int comma2[1||(1,2)]; // expected-warning {{expression result unused}} \ - // expected-warning {{use of logical '||' with constant operand}} \ +int comma1[0?1,2:3]; +int comma2[1||(1,2)]; // expected-warning {{use of logical '||' with constant operand}} \ // expected-note {{use '|' for a bitwise operation}} -int comma3[(1,2)]; // expected-warning {{size of static array must be an integer constant expression}} \ - // expected-warning {{expression result unused}} +int comma3[(1,2)]; // expected-warning {{size of static array must be an integer constant expression}} // Pointer + __builtin_constant_p char pbcp[__builtin_constant_p(4) ? (intptr_t)&expr : 0]; // expected-error {{variable length array declaration not allowed at file scope}} -int illegaldiv1a[1 || 1/0]; // expected-warning {{division by zero is undefined}} -int illegaldiv1b[1 && 1/0]; // expected-warning {{division by zero is undefined}} expected-error{{variable length array declaration not allowed at file scope}} +int illegaldiv1a[1 || 1/0]; +int illegaldiv1b[1 && 1/0]; //expected-error{{variable length array declaration not allowed at file scope}} -int illegaldiv2[1/0]; // expected-error {{variable length array declaration not allowed at file scope}} \ - // expected-warning {{division by zero is undefined}} +int illegaldiv2[1/0]; // expected-error {{variable length array declaration not allowed at file scope}} int illegaldiv3[INT_MIN / -1]; // expected-error {{variable length array declaration not allowed at file scope}} // PR9262 -int illegaldiv4[0 / (1 / 0)]; // expected-warning {{division by zero is undefined}} expected-error {{variable length array declaration not allowed at file scope}} +int illegaldiv4[0 / (1 / 0)]; // expected-error {{variable length array declaration not allowed at file scope}} int chooseexpr[__builtin_choose_expr(1, 1, expr)]; int realop[(__real__ 4) == 4 ? 1 : -1]; diff --git a/clang/test/Sema/vla-2.c b/clang/test/Sema/vla-2.c new file mode 100644 index 000000000000..819cab91afc5 --- /dev/null +++ b/clang/test/Sema/vla-2.c @@ -0,0 +1,17 @@ +// RUN: %clang_cc1 %s -verify -fsyntax-only -pedantic +// Check that we don't crash trying to emit warnings in a potentially-evaluated +// sizeof or typeof. (This test needs to be in a separate file because we use +// a different codepath when we have already emitted an error.) + +int PotentiallyEvaluatedSizeofWarn(int n) { + return (int)sizeof *(0 << 32,(int(*)[n])0); // expected-warning {{expression result unused}} expected-warning {{shift count >= width of type}} +} + +void PotentiallyEvaluatedTypeofWarn(int n) { + __typeof(*(0 << 32,(int(*)[n])0)) x; // expected-warning {{expression result unused}} expected-warning {{shift count >= width of type}} + (void)x; +} + +void PotentiallyEvaluatedArrayBoundWarn(int n) { + (void)*(int(*)[(0 << 32,n)])0; // FIXME: We should warn here. +} diff --git a/clang/test/SemaCXX/vararg-non-pod.cpp b/clang/test/SemaCXX/vararg-non-pod.cpp index 42c27fb30e1b..7e193ec839b0 100644 --- a/clang/test/SemaCXX/vararg-non-pod.cpp +++ b/clang/test/SemaCXX/vararg-non-pod.cpp @@ -118,3 +118,8 @@ void t8(int n, ...) { (void)__builtin_va_arg(list, Abstract); // expected-error{{second argument to 'va_arg' is of abstract type 'Abstract'}} __builtin_va_end(list); } + +int t9(int n) { + // Make sure the error works in potentially-evaluated sizeof + return (int)sizeof(*(Helper(Foo()), (int (*)[n])0)); // expected-warning{{cannot pass object of non-POD type}} +}