diff options
author | Richard Smith <richard-llvm@metafoo.co.uk> | 2019-02-15 00:27:53 +0000 |
---|---|---|
committer | Richard Smith <richard-llvm@metafoo.co.uk> | 2019-02-15 00:27:53 +0000 |
commit | d1f4353f6e59e74948fb8442a8fda9b0bf379700 (patch) | |
tree | 757834eaede0e8913ca27f0c43fd1c8de7d3f2be | |
parent | 7ff8f050d264a0ed5d52063d500216eb36021fe5 (diff) | |
download | clang-d1f4353f6e59e74948fb8442a8fda9b0bf379700.tar.gz |
PR40642: Fix determination of whether the final statement of a statement
expression is a discarded-value expression.
Summary:
We used to get this wrong in three ways:
1) During parsing, an expression-statement followed by the }) ending a
statement expression was always treated as producing the value of the
statement expression. That's wrong for ({ if (1) expr; })
2) During template instantiation, various kinds of statement (most
statements not appearing directly in a compound-statement) were not
treated as discarded-value expressions, resulting in missing volatile
loads (etc).
3) In all contexts, an expression-statement with attributes was not
treated as producing the value of the statement expression, eg
({ [[attr]] expr; }).
Also fix incorrect enforcement of OpenMP rule that directives can "only
be placed in the program at a position where ignoring or deleting the
directive would result in a program with correct syntax". In particular,
a label (be it goto, case, or default) should not affect whether
directives are permitted.
Reviewers: aaron.ballman, rjmccall
Subscribers: cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D57984
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@354090 91177308-0d34-0410-b5e6-96231b3b80d8
-rw-r--r-- | include/clang/AST/Expr.h | 6 | ||||
-rw-r--r-- | include/clang/AST/Stmt.h | 35 | ||||
-rw-r--r-- | include/clang/Basic/StmtNodes.td | 10 | ||||
-rw-r--r-- | include/clang/Parse/Parser.h | 68 | ||||
-rw-r--r-- | include/clang/Sema/Sema.h | 3 | ||||
-rw-r--r-- | lib/AST/Stmt.cpp | 17 | ||||
-rw-r--r-- | lib/CodeGen/CGStmt.cpp | 25 | ||||
-rw-r--r-- | lib/Parse/ParseObjc.cpp | 5 | ||||
-rw-r--r-- | lib/Parse/ParseOpenMP.cpp | 14 | ||||
-rw-r--r-- | lib/Parse/ParseStmt.cpp | 113 | ||||
-rw-r--r-- | lib/Sema/SemaExpr.cpp | 107 | ||||
-rw-r--r-- | lib/Sema/SemaStmt.cpp | 4 | ||||
-rw-r--r-- | lib/Sema/TreeTransform.h | 58 | ||||
-rw-r--r-- | test/CodeGenCXX/stmtexpr.cpp | 76 | ||||
-rw-r--r-- | test/CodeGenCXX/volatile.cpp | 18 | ||||
-rw-r--r-- | test/OpenMP/barrier_messages.cpp | 8 | ||||
-rw-r--r-- | test/OpenMP/cancel_messages.cpp | 4 | ||||
-rw-r--r-- | test/OpenMP/cancellation_point_messages.cpp | 4 | ||||
-rw-r--r-- | test/OpenMP/flush_messages.cpp | 8 | ||||
-rw-r--r-- | test/OpenMP/taskwait_messages.cpp | 8 | ||||
-rw-r--r-- | test/OpenMP/taskyield_messages.cpp | 14 |
21 files changed, 387 insertions, 218 deletions
diff --git a/include/clang/AST/Expr.h b/include/clang/AST/Expr.h index 4fec8efd2d..95804173c8 100644 --- a/include/clang/AST/Expr.h +++ b/include/clang/AST/Expr.h @@ -105,13 +105,13 @@ struct SubobjectAdjustment { /// This represents one expression. Note that Expr's are subclasses of Stmt. /// This allows an expression to be transparently used any place a Stmt is /// required. -class Expr : public Stmt { +class Expr : public ValueStmt { QualType TR; protected: Expr(StmtClass SC, QualType T, ExprValueKind VK, ExprObjectKind OK, bool TD, bool VD, bool ID, bool ContainsUnexpandedParameterPack) - : Stmt(SC) + : ValueStmt(SC) { ExprBits.TypeDependent = TD; ExprBits.ValueDependent = VD; @@ -124,7 +124,7 @@ protected: } /// Construct an empty expression. - explicit Expr(StmtClass SC, EmptyShell) : Stmt(SC) { } + explicit Expr(StmtClass SC, EmptyShell) : ValueStmt(SC) { } public: QualType getType() const { return TR; } diff --git a/include/clang/AST/Stmt.h b/include/clang/AST/Stmt.h index b892d85206..e63354c263 100644 --- a/include/clang/AST/Stmt.h +++ b/include/clang/AST/Stmt.h @@ -1584,21 +1584,44 @@ Stmt *SwitchCase::getSubStmt() { llvm_unreachable("SwitchCase is neither a CaseStmt nor a DefaultStmt!"); } +/// Represents a statement that could possibly have a value and type. This +/// covers expression-statements, as well as labels and attributed statements. +/// +/// Value statements have a special meaning when they are the last non-null +/// statement in a GNU statement expression, where they determine the value +/// of the statement expression. +class ValueStmt : public Stmt { +protected: + using Stmt::Stmt; + +public: + const Expr *getExprStmt() const; + Expr *getExprStmt() { + const ValueStmt *ConstThis = this; + return const_cast<Expr*>(ConstThis->getExprStmt()); + } + + static bool classof(const Stmt *T) { + return T->getStmtClass() >= firstValueStmtConstant && + T->getStmtClass() <= lastValueStmtConstant; + } +}; + /// LabelStmt - Represents a label, which has a substatement. For example: /// foo: return; -class LabelStmt : public Stmt { +class LabelStmt : public ValueStmt { LabelDecl *TheDecl; Stmt *SubStmt; public: /// Build a label statement. LabelStmt(SourceLocation IL, LabelDecl *D, Stmt *substmt) - : Stmt(LabelStmtClass), TheDecl(D), SubStmt(substmt) { + : ValueStmt(LabelStmtClass), TheDecl(D), SubStmt(substmt) { setIdentLoc(IL); } /// Build an empty label statement. - explicit LabelStmt(EmptyShell Empty) : Stmt(LabelStmtClass, Empty) {} + explicit LabelStmt(EmptyShell Empty) : ValueStmt(LabelStmtClass, Empty) {} SourceLocation getIdentLoc() const { return LabelStmtBits.IdentLoc; } void setIdentLoc(SourceLocation L) { LabelStmtBits.IdentLoc = L; } @@ -1627,7 +1650,7 @@ public: /// Represents an attribute applied to a statement. For example: /// [[omp::for(...)]] for (...) { ... } class AttributedStmt final - : public Stmt, + : public ValueStmt, private llvm::TrailingObjects<AttributedStmt, const Attr *> { friend class ASTStmtReader; friend TrailingObjects; @@ -1636,14 +1659,14 @@ class AttributedStmt final AttributedStmt(SourceLocation Loc, ArrayRef<const Attr *> Attrs, Stmt *SubStmt) - : Stmt(AttributedStmtClass), SubStmt(SubStmt) { + : ValueStmt(AttributedStmtClass), SubStmt(SubStmt) { AttributedStmtBits.NumAttrs = Attrs.size(); AttributedStmtBits.AttrLoc = Loc; std::copy(Attrs.begin(), Attrs.end(), getAttrArrayPtr()); } explicit AttributedStmt(EmptyShell Empty, unsigned NumAttrs) - : Stmt(AttributedStmtClass, Empty) { + : ValueStmt(AttributedStmtClass, Empty) { AttributedStmtBits.NumAttrs = NumAttrs; AttributedStmtBits.AttrLoc = SourceLocation{}; std::fill_n(getAttrArrayPtr(), NumAttrs, nullptr); diff --git a/include/clang/Basic/StmtNodes.td b/include/clang/Basic/StmtNodes.td index 9054fb11a6..2dbbee7bbf 100644 --- a/include/clang/Basic/StmtNodes.td +++ b/include/clang/Basic/StmtNodes.td @@ -11,8 +11,6 @@ class DStmt<Stmt base, bit abstract = 0> : Stmt<abstract> { // Statements def NullStmt : Stmt; def CompoundStmt : Stmt; -def LabelStmt : Stmt; -def AttributedStmt : Stmt; def IfStmt : Stmt; def SwitchStmt : Stmt; def WhileStmt : Stmt; @@ -29,6 +27,12 @@ def CaseStmt : DStmt<SwitchCase>; def DefaultStmt : DStmt<SwitchCase>; def CapturedStmt : Stmt; +// Statements that might produce a value (for example, as the last non-null +// statement in a GNU statement-expression). +def ValueStmt : Stmt<1>; +def LabelStmt : DStmt<ValueStmt>; +def AttributedStmt : DStmt<ValueStmt>; + // Asm statements def AsmStmt : Stmt<1>; def GCCAsmStmt : DStmt<AsmStmt>; @@ -53,7 +57,7 @@ def CoroutineBodyStmt : Stmt; def CoreturnStmt : Stmt; // Expressions -def Expr : Stmt<1>; +def Expr : DStmt<ValueStmt, 1>; def PredefinedExpr : DStmt<Expr>; def DeclRefExpr : DStmt<Expr>; def IntegerLiteral : DStmt<Expr>; diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h index f5c70e71b8..db0217324b 100644 --- a/include/clang/Parse/Parser.h +++ b/include/clang/Parse/Parser.h @@ -363,10 +363,28 @@ class Parser : public CodeCompletionHandler { /// just a regular sub-expression. SourceLocation ExprStatementTokLoc; - /// Tests whether an expression value is discarded based on token lookahead. - /// It will return true if the lexer is currently processing the }) - /// terminating a GNU statement expression and false otherwise. - bool isExprValueDiscarded(); + /// Flags describing a context in which we're parsing a statement. + enum class ParsedStmtContext { + /// This context permits declarations in language modes where declarations + /// are not statements. + AllowDeclarationsInC = 0x1, + /// This context permits standalone OpenMP directives. + AllowStandaloneOpenMPDirectives = 0x2, + /// This context is at the top level of a GNU statement expression. + InStmtExpr = 0x4, + + /// The context of a regular substatement. + SubStmt = 0, + /// The context of a compound-statement. + Compound = AllowDeclarationsInC | AllowStandaloneOpenMPDirectives, + + LLVM_MARK_AS_BITMASK_ENUM(InStmtExpr) + }; + + /// Act on an expression statement that might be the last statement in a + /// GNU statement expression. Checks whether we are actually at the end of + /// a statement expression and builds a suitable expression statement. + StmtResult handleExprStmt(ExprResult E, ParsedStmtContext StmtCtx); public: Parser(Preprocessor &PP, Sema &Actions, bool SkipFunctionBodies); @@ -1873,29 +1891,24 @@ private: /// A SmallVector of types. typedef SmallVector<ParsedType, 12> TypeVector; - StmtResult ParseStatement(SourceLocation *TrailingElseLoc = nullptr, - bool AllowOpenMPStandalone = false); - enum AllowedConstructsKind { - /// Allow any declarations, statements, OpenMP directives. - ACK_Any, - /// Allow only statements and non-standalone OpenMP directives. - ACK_StatementsOpenMPNonStandalone, - /// Allow statements and all executable OpenMP directives - ACK_StatementsOpenMPAnyExecutable - }; StmtResult - ParseStatementOrDeclaration(StmtVector &Stmts, AllowedConstructsKind Allowed, - SourceLocation *TrailingElseLoc = nullptr); + ParseStatement(SourceLocation *TrailingElseLoc = nullptr, + ParsedStmtContext StmtCtx = ParsedStmtContext::SubStmt); + StmtResult ParseStatementOrDeclaration( + StmtVector &Stmts, ParsedStmtContext StmtCtx, + SourceLocation *TrailingElseLoc = nullptr); StmtResult ParseStatementOrDeclarationAfterAttributes( StmtVector &Stmts, - AllowedConstructsKind Allowed, + ParsedStmtContext StmtCtx, SourceLocation *TrailingElseLoc, ParsedAttributesWithRange &Attrs); - StmtResult ParseExprStatement(); - StmtResult ParseLabeledStatement(ParsedAttributesWithRange &attrs); - StmtResult ParseCaseStatement(bool MissingCase = false, + StmtResult ParseExprStatement(ParsedStmtContext StmtCtx); + StmtResult ParseLabeledStatement(ParsedAttributesWithRange &attrs, + ParsedStmtContext StmtCtx); + StmtResult ParseCaseStatement(ParsedStmtContext StmtCtx, + bool MissingCase = false, ExprResult Expr = ExprResult()); - StmtResult ParseDefaultStatement(); + StmtResult ParseDefaultStatement(ParsedStmtContext StmtCtx); StmtResult ParseCompoundStatement(bool isStmtExpr = false); StmtResult ParseCompoundStatement(bool isStmtExpr, unsigned ScopeFlags); @@ -1918,7 +1931,7 @@ private: StmtResult ParseAsmStatement(bool &msAsm); StmtResult ParseMicrosoftAsmStatement(SourceLocation AsmLoc); StmtResult ParsePragmaLoopHint(StmtVector &Stmts, - AllowedConstructsKind Allowed, + ParsedStmtContext StmtCtx, SourceLocation *TrailingElseLoc, ParsedAttributesWithRange &Attrs); @@ -1984,7 +1997,8 @@ private: //===--------------------------------------------------------------------===// // Objective-C Statements - StmtResult ParseObjCAtStatement(SourceLocation atLoc); + StmtResult ParseObjCAtStatement(SourceLocation atLoc, + ParsedStmtContext StmtCtx); StmtResult ParseObjCTryStmt(SourceLocation atLoc); StmtResult ParseObjCThrowStmt(SourceLocation atLoc); StmtResult ParseObjCSynchronizedStmt(SourceLocation atLoc); @@ -2824,13 +2838,9 @@ private: bool AllowScopeSpecifier); /// Parses declarative or executable directive. /// - /// \param Allowed ACK_Any, if any directives are allowed, - /// ACK_StatementsOpenMPAnyExecutable - if any executable directives are - /// allowed, ACK_StatementsOpenMPNonStandalone - if only non-standalone - /// executable directives are allowed. - /// + /// \param StmtCtx The context in which we're parsing the directive. StmtResult - ParseOpenMPDeclarativeOrExecutableDirective(AllowedConstructsKind Allowed); + ParseOpenMPDeclarativeOrExecutableDirective(ParsedStmtContext StmtCtx); /// Parses clause of kind \a CKind for directive of a kind \a Kind. /// /// \param DKind Kind of current directive. diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 0c2479a020..10eff5333f 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -1403,7 +1403,6 @@ public: void PopCompoundScope(); sema::CompoundScopeInfo &getCurCompoundScope() const; - bool isCurCompoundStmtAStmtExpr() const; bool hasAnyUnrecoverableErrorsInThisFunction() const; @@ -4533,6 +4532,8 @@ public: void ActOnStartStmtExpr(); ExprResult ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt, SourceLocation RPLoc); // "({..})" + // Handle the final expression in a statement expression. + ExprResult ActOnStmtExprResult(ExprResult E); void ActOnStmtExprError(); // __builtin_offsetof(type, identifier(.identifier|[expr])*) diff --git a/lib/AST/Stmt.cpp b/lib/AST/Stmt.cpp index d1a4dc074b..f2bb289719 100644 --- a/lib/AST/Stmt.cpp +++ b/lib/AST/Stmt.cpp @@ -320,6 +320,23 @@ CompoundStmt *CompoundStmt::CreateEmpty(const ASTContext &C, return New; } +const Expr *ValueStmt::getExprStmt() const { + const Stmt *S = this; + do { + if (const auto *E = dyn_cast<Expr>(S)) + return E; + + if (const auto *LS = dyn_cast<LabelStmt>(S)) + S = LS->getSubStmt(); + else if (const auto *AS = dyn_cast<AttributedStmt>(S)) + S = AS->getSubStmt(); + else + llvm_unreachable("unknown kind of ValueStmt"); + } while (isa<ValueStmt>(S)); + + return nullptr; +} + const char *LabelStmt::getName() const { return getDecl()->getIdentifier()->getNameStart(); } diff --git a/lib/CodeGen/CGStmt.cpp b/lib/CodeGen/CGStmt.cpp index db3f4f142d..5b5113b1e7 100644 --- a/lib/CodeGen/CGStmt.cpp +++ b/lib/CodeGen/CGStmt.cpp @@ -391,26 +391,35 @@ CodeGenFunction::EmitCompoundStmtWithoutScope(const CompoundStmt &S, // at the end of a statement expression, they yield the value of their // subexpression. Handle this by walking through all labels we encounter, // emitting them before we evaluate the subexpr. + // Similar issues arise for attributed statements. const Stmt *LastStmt = S.body_back(); - while (const LabelStmt *LS = dyn_cast<LabelStmt>(LastStmt)) { - EmitLabel(LS->getDecl()); - LastStmt = LS->getSubStmt(); + while (!isa<Expr>(LastStmt)) { + if (const auto *LS = dyn_cast<LabelStmt>(LastStmt)) { + EmitLabel(LS->getDecl()); + LastStmt = LS->getSubStmt(); + } else if (const auto *AS = dyn_cast<AttributedStmt>(LastStmt)) { + // FIXME: Update this if we ever have attributes that affect the + // semantics of an expression. + LastStmt = AS->getSubStmt(); + } else { + llvm_unreachable("unknown value statement"); + } } EnsureInsertPoint(); - QualType ExprTy = cast<Expr>(LastStmt)->getType(); + const Expr *E = cast<Expr>(LastStmt); + QualType ExprTy = E->getType(); if (hasAggregateEvaluationKind(ExprTy)) { - EmitAggExpr(cast<Expr>(LastStmt), AggSlot); + EmitAggExpr(E, AggSlot); } else { // We can't return an RValue here because there might be cleanups at // the end of the StmtExpr. Because of that, we have to emit the result // here into a temporary alloca. RetAlloca = CreateMemTemp(ExprTy); - EmitAnyExprToMem(cast<Expr>(LastStmt), RetAlloca, Qualifiers(), - /*IsInit*/false); + EmitAnyExprToMem(E, RetAlloca, Qualifiers(), + /*IsInit*/ false); } - } return RetAlloca; diff --git a/lib/Parse/ParseObjc.cpp b/lib/Parse/ParseObjc.cpp index 0a1dbf7bff..5ac01f6f0a 100644 --- a/lib/Parse/ParseObjc.cpp +++ b/lib/Parse/ParseObjc.cpp @@ -2703,7 +2703,8 @@ Decl *Parser::ParseObjCMethodDefinition() { return MDecl; } -StmtResult Parser::ParseObjCAtStatement(SourceLocation AtLoc) { +StmtResult Parser::ParseObjCAtStatement(SourceLocation AtLoc, + ParsedStmtContext StmtCtx) { if (Tok.is(tok::code_completion)) { Actions.CodeCompleteObjCAtStatement(getCurScope()); cutOffParsing(); @@ -2740,7 +2741,7 @@ StmtResult Parser::ParseObjCAtStatement(SourceLocation AtLoc) { // Otherwise, eat the semicolon. ExpectAndConsumeSemi(diag::err_expected_semi_after_expr); - return Actions.ActOnExprStmt(Res, isExprValueDiscarded()); + return handleExprStmt(Res, StmtCtx); } ExprResult Parser::ParseObjCAtExpression(SourceLocation AtLoc) { diff --git a/lib/Parse/ParseOpenMP.cpp b/lib/Parse/ParseOpenMP.cpp index 5b8670e928..d0463416f5 100644 --- a/lib/Parse/ParseOpenMP.cpp +++ b/lib/Parse/ParseOpenMP.cpp @@ -1132,8 +1132,8 @@ Parser::DeclGroupPtrTy Parser::ParseOpenMPDeclarativeDirectiveWithExtDecl( /// 'target teams distribute simd' {clause} /// annot_pragma_openmp_end /// -StmtResult Parser::ParseOpenMPDeclarativeOrExecutableDirective( - AllowedConstructsKind Allowed) { +StmtResult +Parser::ParseOpenMPDeclarativeOrExecutableDirective(ParsedStmtContext StmtCtx) { assert(Tok.is(tok::annot_pragma_openmp) && "Not an OpenMP directive!"); ParenBraceBracketBalancer BalancerRAIIObj(*this); SmallVector<OMPClause *, 5> Clauses; @@ -1152,7 +1152,9 @@ StmtResult Parser::ParseOpenMPDeclarativeOrExecutableDirective( switch (DKind) { case OMPD_threadprivate: { - if (Allowed != ACK_Any) { + // FIXME: Should this be permitted in C++? + if ((StmtCtx & ParsedStmtContext::AllowDeclarationsInC) == + ParsedStmtContext()) { Diag(Tok, diag::err_omp_immediate_directive) << getOpenMPDirectiveName(DKind) << 0; } @@ -1219,7 +1221,8 @@ StmtResult Parser::ParseOpenMPDeclarativeOrExecutableDirective( case OMPD_target_enter_data: case OMPD_target_exit_data: case OMPD_target_update: - if (Allowed == ACK_StatementsOpenMPNonStandalone) { + if ((StmtCtx & ParsedStmtContext::AllowStandaloneOpenMPDirectives) == + ParsedStmtContext()) { Diag(Tok, diag::err_omp_immediate_directive) << getOpenMPDirectiveName(DKind) << 0; } @@ -1322,7 +1325,8 @@ StmtResult Parser::ParseOpenMPDeclarativeOrExecutableDirective( // If the depend clause is specified, the ordered construct is a stand-alone // directive. if (DKind == OMPD_ordered && FirstClauses[OMPC_depend].getInt()) { - if (Allowed == ACK_StatementsOpenMPNonStandalone) { + if ((StmtCtx & ParsedStmtContext::AllowStandaloneOpenMPDirectives) == + ParsedStmtContext()) { Diag(Loc, diag::err_omp_immediate_directive) << getOpenMPDirectiveName(DKind) << 1 << getOpenMPClauseName(OMPC_depend); diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp index 3fc29253f0..267003b518 100644 --- a/lib/Parse/ParseStmt.cpp +++ b/lib/Parse/ParseStmt.cpp @@ -29,17 +29,14 @@ using namespace clang; /// Parse a standalone statement (for instance, as the body of an 'if', /// 'while', or 'for'). StmtResult Parser::ParseStatement(SourceLocation *TrailingElseLoc, - bool AllowOpenMPStandalone) { + ParsedStmtContext StmtCtx) { StmtResult Res; // We may get back a null statement if we found a #pragma. Keep going until // we get an actual statement. do { StmtVector Stmts; - Res = ParseStatementOrDeclaration( - Stmts, AllowOpenMPStandalone ? ACK_StatementsOpenMPAnyExecutable - : ACK_StatementsOpenMPNonStandalone, - TrailingElseLoc); + Res = ParseStatementOrDeclaration(Stmts, StmtCtx, TrailingElseLoc); } while (!Res.isInvalid() && !Res.get()); return Res; @@ -96,7 +93,7 @@ StmtResult Parser::ParseStatement(SourceLocation *TrailingElseLoc, /// StmtResult Parser::ParseStatementOrDeclaration(StmtVector &Stmts, - AllowedConstructsKind Allowed, + ParsedStmtContext StmtCtx, SourceLocation *TrailingElseLoc) { ParenBraceBracketBalancer BalancerRAIIObj(*this); @@ -107,7 +104,7 @@ Parser::ParseStatementOrDeclaration(StmtVector &Stmts, return StmtError(); StmtResult Res = ParseStatementOrDeclarationAfterAttributes( - Stmts, Allowed, TrailingElseLoc, Attrs); + Stmts, StmtCtx, TrailingElseLoc, Attrs); assert((Attrs.empty() || Res.isInvalid() || Res.isUsable()) && "attributes on empty statement"); @@ -147,10 +144,9 @@ private: }; } -StmtResult -Parser::ParseStatementOrDeclarationAfterAttributes(StmtVector &Stmts, - AllowedConstructsKind Allowed, SourceLocation *TrailingElseLoc, - ParsedAttributesWithRange &Attrs) { +StmtResult Parser::ParseStatementOrDeclarationAfterAttributes( + StmtVector &Stmts, ParsedStmtContext StmtCtx, + SourceLocation *TrailingElseLoc, ParsedAttributesWithRange &Attrs) { const char *SemiError = nullptr; StmtResult Res; @@ -165,7 +161,7 @@ Retry: { ProhibitAttributes(Attrs); // TODO: is it correct? AtLoc = ConsumeToken(); // consume @ - return ParseObjCAtStatement(AtLoc); + return ParseObjCAtStatement(AtLoc, StmtCtx); } case tok::code_completion: @@ -177,7 +173,7 @@ Retry: Token Next = NextToken(); if (Next.is(tok::colon)) { // C99 6.8.1: labeled-statement // identifier ':' statement - return ParseLabeledStatement(Attrs); + return ParseLabeledStatement(Attrs, StmtCtx); } // Look up the identifier, and typo-correct it to a keyword if it's not @@ -207,7 +203,8 @@ Retry: default: { if ((getLangOpts().CPlusPlus || getLangOpts().MicrosoftExt || - Allowed == ACK_Any) && + (StmtCtx & ParsedStmtContext::AllowDeclarationsInC) != + ParsedStmtContext()) && isDeclarationStatement()) { SourceLocation DeclStart = Tok.getLocation(), DeclEnd; DeclGroupPtrTy Decl = ParseDeclaration(DeclaratorContext::BlockContext, @@ -220,13 +217,13 @@ Retry: return StmtError(); } - return ParseExprStatement(); + return ParseExprStatement(StmtCtx); } case tok::kw_case: // C99 6.8.1: labeled-statement - return ParseCaseStatement(); + return ParseCaseStatement(StmtCtx); case tok::kw_default: // C99 6.8.1: labeled-statement - return ParseDefaultStatement(); + return ParseDefaultStatement(StmtCtx); case tok::l_brace: // C99 6.8.2: compound-statement return ParseCompoundStatement(); @@ -363,7 +360,7 @@ Retry: case tok::annot_pragma_openmp: ProhibitAttributes(Attrs); - return ParseOpenMPDeclarativeOrExecutableDirective(Allowed); + return ParseOpenMPDeclarativeOrExecutableDirective(StmtCtx); case tok::annot_pragma_ms_pointers_to_members: ProhibitAttributes(Attrs); @@ -382,7 +379,7 @@ Retry: case tok::annot_pragma_loop_hint: ProhibitAttributes(Attrs); - return ParsePragmaLoopHint(Stmts, Allowed, TrailingElseLoc, Attrs); + return ParsePragmaLoopHint(Stmts, StmtCtx, TrailingElseLoc, Attrs); case tok::annot_pragma_dump: HandlePragmaDump(); @@ -407,7 +404,7 @@ Retry: } /// Parse an expression statement. -StmtResult Parser::ParseExprStatement() { +StmtResult Parser::ParseExprStatement(ParsedStmtContext StmtCtx) { // If a case keyword is missing, this is where it should be inserted. Token OldToken = Tok; @@ -433,12 +430,12 @@ StmtResult Parser::ParseExprStatement() { << FixItHint::CreateInsertion(OldToken.getLocation(), "case "); // Recover parsing as a case statement. - return ParseCaseStatement(/*MissingCase=*/true, Expr); + return ParseCaseStatement(StmtCtx, /*MissingCase=*/true, Expr); } // Otherwise, eat the semicolon. ExpectAndConsumeSemi(diag::err_expected_semi_after_expr); - return Actions.ActOnExprStmt(Expr, isExprValueDiscarded()); + return handleExprStmt(Expr, StmtCtx); } /// ParseSEHTryBlockCommon @@ -577,10 +574,15 @@ StmtResult Parser::ParseSEHLeaveStatement() { /// identifier ':' statement /// [GNU] identifier ':' attributes[opt] statement /// -StmtResult Parser::ParseLabeledStatement(ParsedAttributesWithRange &attrs) { +StmtResult Parser::ParseLabeledStatement(ParsedAttributesWithRange &attrs, + ParsedStmtContext StmtCtx) { assert(Tok.is(tok::identifier) && Tok.getIdentifierInfo() && "Not an identifier!"); + // The substatement is always a 'statement', not a 'declaration', but is + // otherwise in the same context as the labeled-statement. + StmtCtx &= ~ParsedStmtContext::AllowDeclarationsInC; + Token IdentTok = Tok; // Save the whole token. ConsumeToken(); // eat the identifier. @@ -610,9 +612,8 @@ StmtResult Parser::ParseLabeledStatement(ParsedAttributesWithRange &attrs) { // statement, but that doesn't work correctly (because ProhibitAttributes // can't handle GNU attributes), so only call it in the one case where // GNU attributes are allowed. - SubStmt = ParseStatementOrDeclarationAfterAttributes( - Stmts, /*Allowed=*/ACK_StatementsOpenMPNonStandalone, nullptr, - TempAttrs); + SubStmt = ParseStatementOrDeclarationAfterAttributes(Stmts, StmtCtx, + nullptr, TempAttrs); if (!TempAttrs.empty() && !SubStmt.isInvalid()) SubStmt = Actions.ProcessStmtAttributes(SubStmt.get(), TempAttrs, TempAttrs.Range); @@ -623,7 +624,7 @@ StmtResult Parser::ParseLabeledStatement(ParsedAttributesWithRange &attrs) { // If we've not parsed a statement yet, parse one now. if (!SubStmt.isInvalid() && !SubStmt.isUsable()) - SubStmt = ParseStatement(); + SubStmt = ParseStatement(nullptr, StmtCtx); // Broken substmt shouldn't prevent the label from being added to the AST. if (SubStmt.isInvalid()) @@ -643,9 +644,14 @@ StmtResult Parser::ParseLabeledStatement(ParsedAttributesWithRange &attrs) { /// 'case' constant-expression ':' statement /// [GNU] 'case' constant-expression '...' constant-expression ':' statement /// -StmtResult Parser::ParseCaseStatement(bool MissingCase, ExprResult Expr) { +StmtResult Parser::ParseCaseStatement(ParsedStmtContext StmtCtx, + bool MissingCase, ExprResult Expr) { assert((MissingCase || Tok.is(tok::kw_case)) && "Not a case stmt!"); + // The substatement is always a 'statement', not a 'declaration', but is + // otherwise in the same context as the labeled-statement. + StmtCtx &= ~ParsedStmtContext::AllowDeclarationsInC; + // It is very very common for code to contain many case statements recursively // nested, as in (but usually without indentation): // case 1: @@ -737,8 +743,7 @@ StmtResult Parser::ParseCaseStatement(bool MissingCase, ExprResult Expr) { // continue parsing the sub-stmt. if (Case.isInvalid()) { if (TopLevelCase.isInvalid()) // No parsed case stmts. - return ParseStatement(/*TrailingElseLoc=*/nullptr, - /*AllowOpenMPStandalone=*/true); + return ParseStatement(/*TrailingElseLoc=*/nullptr, StmtCtx); // Otherwise, just don't add it as a nested case. } else { // If this is the first case statement we parsed, it becomes TopLevelCase. @@ -758,8 +763,7 @@ StmtResult Parser::ParseCaseStatement(bool MissingCase, ExprResult Expr) { StmtResult SubStmt; if (Tok.isNot(tok::r_brace)) { - SubStmt = ParseStatement(/*TrailingElseLoc=*/nullptr, - /*AllowOpenMPStandalone=*/true); + SubStmt = ParseStatement(/*TrailingElseLoc=*/nullptr, StmtCtx); } else { // Nicely diagnose the common error "switch (X) { case 4: }", which is // not valid. If ColonLoc doesn't point to a valid text location, there was @@ -789,8 +793,13 @@ StmtResult Parser::ParseCaseStatement(bool MissingCase, ExprResult Expr) { /// 'default' ':' statement /// Note that this does not parse the 'statement' at the end. /// -StmtResult Parser::ParseDefaultStatement() { +StmtResult Parser::ParseDefaultStatement(ParsedStmtContext StmtCtx) { assert(Tok.is(tok::kw_default) && "Not a default stmt!"); + + // The substatement is always a 'statement', not a 'declaration', but is + // otherwise in the same context as the labeled-statement. + StmtCtx &= ~ParsedStmtContext::AllowDeclarationsInC; + SourceLocation DefaultLoc = ConsumeToken(); // eat the 'default'. SourceLocation ColonLoc; @@ -811,8 +820,7 @@ StmtResult Parser::ParseDefaultStatement() { StmtResult SubStmt; if (Tok.isNot(tok::r_brace)) { - SubStmt = ParseStatement(/*TrailingElseLoc=*/nullptr, - /*AllowOpenMPStandalone=*/true); + SubStmt = ParseStatement(/*TrailingElseLoc=*/nullptr, StmtCtx); } else { // Diagnose the common error "switch (X) {... default: }", which is // not valid. @@ -943,7 +951,8 @@ bool Parser::ConsumeNullStmt(StmtVector &Stmts) { EndLoc = Tok.getLocation(); // Don't just ConsumeToken() this tok::semi, do store it in AST. - StmtResult R = ParseStatementOrDeclaration(Stmts, ACK_Any); + StmtResult R = + ParseStatementOrDeclaration(Stmts, ParsedStmtContext::SubStmt); if (R.isUsable()) Stmts.push_back(R.get()); } @@ -957,14 +966,18 @@ bool Parser::ConsumeNullStmt(StmtVector &Stmts) { return true; } -bool Parser::isExprValueDiscarded() { - if (Actions.isCurCompoundStmtAStmtExpr()) { - // Look to see if the next two tokens close the statement expression; +StmtResult Parser::handleExprStmt(ExprResult E, ParsedStmtContext StmtCtx) { + bool IsStmtExprResult = false; + if ((StmtCtx & ParsedStmtContext::InStmtExpr) != ParsedStmtContext()) { + // Look ahead to see if the next two tokens close the statement expression; // if so, this expression statement is the last statement in a // statment expression. - return Tok.isNot(tok::r_brace) || NextToken().isNot(tok::r_paren); + IsStmtExprResult = Tok.is(tok::r_brace) && NextToken().is(tok::r_paren); } - return true; + + if (IsStmtExprResult) + E = Actions.ActOnStmtExprResult(E); + return Actions.ActOnExprStmt(E, /*DiscardedValue=*/!IsStmtExprResult); } /// ParseCompoundStatementBody - Parse a sequence of statements and invoke the @@ -1022,6 +1035,10 @@ StmtResult Parser::ParseCompoundStatementBody(bool isStmtExpr) { Stmts.push_back(R.get()); } + ParsedStmtContext SubStmtCtx = + ParsedStmtContext::Compound | + (isStmtExpr ? ParsedStmtContext::InStmtExpr : ParsedStmtContext()); + while (!tryParseMisplacedModuleImport() && Tok.isNot(tok::r_brace) && Tok.isNot(tok::eof)) { if (Tok.is(tok::annot_pragma_unused)) { @@ -1034,7 +1051,7 @@ StmtResult Parser::ParseCompoundStatementBody(bool isStmtExpr) { StmtResult R; if (Tok.isNot(tok::kw___extension__)) { - R = ParseStatementOrDeclaration(Stmts, ACK_Any); + R = ParseStatementOrDeclaration(Stmts, SubStmtCtx); } else { // __extension__ can start declarations and it can also be a unary // operator for expressions. Consume multiple __extension__ markers here @@ -1067,11 +1084,12 @@ StmtResult Parser::ParseCompoundStatementBody(bool isStmtExpr) { continue; } - // FIXME: Use attributes? // Eat the semicolon at the end of stmt and convert the expr into a // statement. ExpectAndConsumeSemi(diag::err_expected_semi_after_expr); - R = Actions.ActOnExprStmt(Res, isExprValueDiscarded()); + R = handleExprStmt(Res, SubStmtCtx); + if (R.isUsable()) + R = Actions.ProcessStmtAttributes(R.get(), attrs, attrs.Range); } } @@ -2001,7 +2019,7 @@ StmtResult Parser::ParseReturnStatement() { } StmtResult Parser::ParsePragmaLoopHint(StmtVector &Stmts, - AllowedConstructsKind Allowed, + ParsedStmtContext StmtCtx, SourceLocation *TrailingElseLoc, ParsedAttributesWithRange &Attrs) { // Create temporary attribute list. @@ -2024,7 +2042,7 @@ StmtResult Parser::ParsePragmaLoopHint(StmtVector &Stmts, MaybeParseCXX11Attributes(Attrs); StmtResult S = ParseStatementOrDeclarationAfterAttributes( - Stmts, Allowed, TrailingElseLoc, Attrs); + Stmts, StmtCtx, TrailingElseLoc, Attrs); Attrs.takeAllFrom(TempAttrs); return S; @@ -2329,7 +2347,8 @@ void Parser::ParseMicrosoftIfExistsStatement(StmtVector &Stmts) { // Condition is true, parse the statements. while (Tok.isNot(tok::r_brace)) { - StmtResult R = ParseStatementOrDeclaration(Stmts, ACK_Any); + StmtResult R = + ParseStatementOrDeclaration(Stmts, ParsedStmtContext::Compound); if (R.isUsable()) Stmts.push_back(R.get()); } diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index b992f3387e..00b4cddc6b 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -13301,29 +13301,6 @@ ExprResult Sema::ActOnAddrLabel(SourceLocation OpLoc, SourceLocation LabLoc, Context.getPointerType(Context.VoidTy)); } -/// Given the last statement in a statement-expression, check whether -/// the result is a producing expression (like a call to an -/// ns_returns_retained function) and, if so, rebuild it to hoist the -/// release out of the full-expression. Otherwise, return null. -/// Cannot fail. -static Expr *maybeRebuildARCConsumingStmt(Stmt *Statement) { - // Should always be wrapped with one of these. - ExprWithCleanups *cleanups = dyn_cast<ExprWithCleanups>(Statement); - if (!cleanups) return nullptr; - - ImplicitCastExpr *cast = dyn_cast<ImplicitCastExpr>(cleanups->getSubExpr()); - if (!cast || cast->getCastKind() != CK_ARCConsumeObject) - return nullptr; - - // Splice out the cast. This shouldn't modify any interesting - // features of the statement. - Expr *producer = cast->getSubExpr(); - assert(producer->getType() == cast->getType()); - assert(producer->getValueKind() == cast->getValueKind()); - cleanups->setSubExpr(producer); - return cleanups; -} - void Sema::ActOnStartStmtExpr() { PushExpressionEvaluationContext(ExprEvalContexts.back().Context); } @@ -13357,47 +13334,10 @@ Sema::ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt, QualType Ty = Context.VoidTy; bool StmtExprMayBindToTemp = false; if (!Compound->body_empty()) { - Stmt *LastStmt = Compound->body_back(); - LabelStmt *LastLabelStmt = nullptr; - // If LastStmt is a label, skip down through into the body. - while (LabelStmt *Label = dyn_cast<LabelStmt>(LastStmt)) { - LastLabelStmt = Label; - LastStmt = Label->getSubStmt(); - } - - if (Expr *LastE = dyn_cast<Expr>(LastStmt)) { - // Do function/array conversion on the last expression, but not - // lvalue-to-rvalue. However, initialize an unqualified type. - ExprResult LastExpr = DefaultFunctionArrayConversion(LastE); - if (LastExpr.isInvalid()) - return ExprError(); - Ty = LastExpr.get()->getType().getUnqualifiedType(); - - if (!Ty->isDependentType() && !LastExpr.get()->isTypeDependent()) { - // In ARC, if the final expression ends in a consume, splice - // the consume out and bind it later. In the alternate case - // (when dealing with a retainable type), the result - // initialization will create a produce. In both cases the - // result will be +1, and we'll need to balance that out with - // a bind. - if (Expr *rebuiltLastStmt - = maybeRebuildARCConsumingStmt(LastExpr.get())) { - LastExpr = rebuiltLastStmt; - } else { - LastExpr = PerformCopyInitialization( - InitializedEntity::InitializeStmtExprResult(LPLoc, Ty), - SourceLocation(), LastExpr); - } - - if (LastExpr.isInvalid()) - return ExprError(); - if (LastExpr.get() != nullptr) { - if (!LastLabelStmt) - Compound->setLastStmt(LastExpr.get()); - else - LastLabelStmt->setSubStmt(LastExpr.get()); - StmtExprMayBindToTemp = true; - } + if (const auto *LastStmt = dyn_cast<ValueStmt>(Compound->body_back())) { + if (const Expr *Value = LastStmt->getExprStmt()) { + StmtExprMayBindToTemp = true; + Ty = Value->getType(); } } } @@ -13410,6 +13350,37 @@ Sema::ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt, return ResStmtExpr; } +ExprResult Sema::ActOnStmtExprResult(ExprResult ER) { + if (ER.isInvalid()) + return ExprError(); + + // Do function/array conversion on the last expression, but not + // lvalue-to-rvalue. However, initialize an unqualified type. + ER = DefaultFunctionArrayConversion(ER.get()); + if (ER.isInvalid()) + return ExprError(); + Expr *E = ER.get(); + + if (E->isTypeDependent()) + return E; + + // In ARC, if the final expression ends in a consume, splice + // the consume out and bind it later. In the alternate case + // (when dealing with a retainable type), the result + // initialization will create a produce. In both cases the + // result will be +1, and we'll need to balance that out with + // a bind. + auto *Cast = dyn_cast<ImplicitCastExpr>(E); + if (Cast && Cast->getCastKind() == CK_ARCConsumeObject) + return Cast->getSubExpr(); + + // FIXME: Provide a better location for the initialization. + return PerformCopyInitialization( + InitializedEntity::InitializeStmtExprResult( + E->getBeginLoc(), E->getType().getUnqualifiedType()), + SourceLocation(), E); +} + ExprResult Sema::BuildBuiltinOffsetOf(SourceLocation BuiltinLoc, TypeSourceInfo *TInfo, ArrayRef<OffsetOfComponent> Components, @@ -14490,14 +14461,6 @@ namespace { // Make sure we redo semantic analysis bool AlwaysRebuild() { return true; } - // Make sure we handle LabelStmts correctly. - // FIXME: This does the right thing, but maybe we need a more general - // fix to TreeTransform? - StmtResult TransformLabelStmt(LabelStmt *S) { - S->getDecl()->setStmt(nullptr); - return BaseTransform::TransformLabelStmt(S); - } - // We need to special-case DeclRefExprs referring to FieldDecls which // are not part of a member pointer formation; normal TreeTransforming // doesn't catch this case because of the way we represent them in the AST. diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp index e4b4357653..5f04139090 100644 --- a/lib/Sema/SemaStmt.cpp +++ b/lib/Sema/SemaStmt.cpp @@ -346,10 +346,6 @@ sema::CompoundScopeInfo &Sema::getCurCompoundScope() const { return getCurFunction()->CompoundScopes.back(); } -bool Sema::isCurCompoundStmtAStmtExpr() const { - return getCurCompoundScope().IsStmtExpr; -} - StmtResult Sema::ActOnCompoundStmt(SourceLocation L, SourceLocation R, ArrayRef<Stmt *> Elts, bool isStmtExpr) { const unsigned NumElts = Elts.size(); diff --git a/lib/Sema/TreeTransform.h b/lib/Sema/TreeTransform.h index e411a2ad47..ff33028ebb 100644 --- a/lib/Sema/TreeTransform.h +++ b/lib/Sema/TreeTransform.h @@ -318,6 +318,13 @@ public: TypeSourceInfo *TransformTypeWithDeducedTST(TypeSourceInfo *DI); /// @} + /// The reason why the value of a statement is not discarded, if any. + enum StmtDiscardKind { + SDK_Discarded, + SDK_NotDiscarded, + SDK_StmtExprResult, + }; + /// Transform the given statement. /// /// By default, this routine transforms a statement by delegating to the @@ -327,7 +334,7 @@ public: /// other mechanism. /// /// \returns the transformed statement. - StmtResult TransformStmt(Stmt *S, bool DiscardedValue = false); + StmtResult TransformStmt(Stmt *S, StmtDiscardKind SDK = SDK_Discarded); /// Transform the given statement. /// @@ -672,6 +679,9 @@ public: #define STMT(Node, Parent) \ LLVM_ATTRIBUTE_NOINLINE \ StmtResult Transform##Node(Node *S); +#define VALUESTMT(Node, Parent) \ + LLVM_ATTRIBUTE_NOINLINE \ + StmtResult Transform##Node(Node *S, StmtDiscardKind SDK); #define EXPR(Node, Parent) \ LLVM_ATTRIBUTE_NOINLINE \ ExprResult Transform##Node(Node *E); @@ -3270,7 +3280,7 @@ private: }; template <typename Derived> -StmtResult TreeTransform<Derived>::TransformStmt(Stmt *S, bool DiscardedValue) { +StmtResult TreeTransform<Derived>::TransformStmt(Stmt *S, StmtDiscardKind SDK) { if (!S) return S; @@ -3278,8 +3288,12 @@ StmtResult TreeTransform<Derived>::TransformStmt(Stmt *S, bool DiscardedValue) { case Stmt::NoStmtClass: break; // Transform individual statement nodes + // Pass SDK into statements that can produce a value #define STMT(Node, Parent) \ case Stmt::Node##Class: return getDerived().Transform##Node(cast<Node>(S)); +#define VALUESTMT(Node, Parent) \ + case Stmt::Node##Class: \ + return getDerived().Transform##Node(cast<Node>(S), SDK); #define ABSTRACT_STMT(Node) #define EXPR(Node, Parent) #include "clang/AST/StmtNodes.inc" @@ -3291,10 +3305,10 @@ StmtResult TreeTransform<Derived>::TransformStmt(Stmt *S, bool DiscardedValue) { #include "clang/AST/StmtNodes.inc" { ExprResult E = getDerived().TransformExpr(cast<Expr>(S)); - if (E.isInvalid()) - return StmtError(); - return getSema().ActOnExprStmt(E, DiscardedValue); + if (SDK == SDK_StmtExprResult) + E = getSema().ActOnStmtExprResult(E); + return getSema().ActOnExprStmt(E, SDK == SDK_Discarded); } } @@ -6522,8 +6536,9 @@ TreeTransform<Derived>::TransformCompoundStmt(CompoundStmt *S, bool SubStmtChanged = false; SmallVector<Stmt*, 8> Statements; for (auto *B : S->body()) { - StmtResult Result = - getDerived().TransformStmt(B, !IsStmtExpr || B != S->body_back()); + StmtResult Result = getDerived().TransformStmt( + B, + IsStmtExpr && B == S->body_back() ? SDK_StmtExprResult : SDK_Discarded); if (Result.isInvalid()) { // Immediately fail if this was a DeclStmt, since it's very @@ -6586,7 +6601,8 @@ TreeTransform<Derived>::TransformCaseStmt(CaseStmt *S) { return StmtError(); // Transform the statement following the case - StmtResult SubStmt = getDerived().TransformStmt(S->getSubStmt()); + StmtResult SubStmt = + getDerived().TransformStmt(S->getSubStmt()); if (SubStmt.isInvalid()) return StmtError(); @@ -6594,11 +6610,11 @@ TreeTransform<Derived>::TransformCaseStmt(CaseStmt *S) { return getDerived().RebuildCaseStmtBody(Case.get(), SubStmt.get()); } -template<typename Derived> -StmtResult -TreeTransform<Derived>::TransformDefaultStmt(DefaultStmt *S) { +template <typename Derived> +StmtResult TreeTransform<Derived>::TransformDefaultStmt(DefaultStmt *S) { // Transform the statement following the default case - StmtResult SubStmt = getDerived().TransformStmt(S->getSubStmt()); + StmtResult SubStmt = + getDerived().TransformStmt(S->getSubStmt()); if (SubStmt.isInvalid()) return StmtError(); @@ -6609,8 +6625,8 @@ TreeTransform<Derived>::TransformDefaultStmt(DefaultStmt *S) { template<typename Derived> StmtResult -TreeTransform<Derived>::TransformLabelStmt(LabelStmt *S) { - StmtResult SubStmt = getDerived().TransformStmt(S->getSubStmt()); +TreeTransform<Derived>::TransformLabelStmt(LabelStmt *S, StmtDiscardKind SDK) { + StmtResult SubStmt = getDerived().TransformStmt(S->getSubStmt(), SDK); if (SubStmt.isInvalid()) return StmtError(); @@ -6619,6 +6635,11 @@ TreeTransform<Derived>::TransformLabelStmt(LabelStmt *S) { if (!LD) return StmtError(); + // If we're transforming "in-place" (we're not creating new local + // declarations), assume we're replacing the old label statement + // and clear out the reference to it. + if (LD == S->getDecl()) + S->getDecl()->setStmt(nullptr); // FIXME: Pass the real colon location in. return getDerived().RebuildLabelStmt(S->getIdentLoc(), @@ -6644,7 +6665,9 @@ const Attr *TreeTransform<Derived>::TransformAttr(const Attr *R) { } template <typename Derived> -StmtResult TreeTransform<Derived>::TransformAttributedStmt(AttributedStmt *S) { +StmtResult +TreeTransform<Derived>::TransformAttributedStmt(AttributedStmt *S, + StmtDiscardKind SDK) { bool AttrsChanged = false; SmallVector<const Attr *, 1> Attrs; @@ -6655,7 +6678,7 @@ StmtResult TreeTransform<Derived>::TransformAttributedStmt(AttributedStmt *S) { Attrs.push_back(R); } - StmtResult SubStmt = getDerived().TransformStmt(S->getSubStmt()); + StmtResult SubStmt = getDerived().TransformStmt(S->getSubStmt(), SDK); if (SubStmt.isInvalid()) return StmtError(); @@ -7360,7 +7383,8 @@ StmtResult TreeTransform<Derived>::TransformObjCForCollectionStmt( ObjCForCollectionStmt *S) { // Transform the element statement. - StmtResult Element = getDerived().TransformStmt(S->getElement()); + StmtResult Element = + getDerived().TransformStmt(S->getElement(), SDK_NotDiscarded); if (Element.isInvalid()) return StmtError(); diff --git a/test/CodeGenCXX/stmtexpr.cpp b/test/CodeGenCXX/stmtexpr.cpp index 4586a3c38f..fe5ff2c7de 100644 --- a/test/CodeGenCXX/stmtexpr.cpp +++ b/test/CodeGenCXX/stmtexpr.cpp @@ -190,3 +190,79 @@ extern "C" int cleanup_exit_complex(bool b) { // CHECK: %[[v2:[^ ]*]] = load float, float* %[[tmp2]] // CHECK: store float %[[v1]], float* %v.realp // CHECK: store float %[[v2]], float* %v.imagp + +extern "C" void then(int); + +// CHECK-LABEL: @{{.*}}volatile_load +void volatile_load() { + volatile int n; + + // CHECK-NOT: load volatile + // CHECK: load volatile + // CHECK-NOT: load volatile + ({n;}); + + // CHECK-LABEL: @then(i32 1) + then(1); + + // CHECK-NOT: load volatile + // CHECK: load volatile + // CHECK-NOT: load volatile + ({goto lab; lab: n;}); + + // CHECK-LABEL: @then(i32 2) + then(2); + + // CHECK-NOT: load volatile + // CHECK: load volatile + // CHECK-NOT: load volatile + ({[[gsl::suppress("foo")]] n;}); + + // CHECK-LABEL: @then(i32 3) + then(3); + + // CHECK-NOT: load volatile + // CHECK: load volatile + // CHECK-NOT: load volatile + ({if (true) n;}); + + // CHECK: } +} + +// CHECK-LABEL: @{{.*}}volatile_load_template +template<typename T> +void volatile_load_template() { + volatile T n; + + // CHECK-NOT: load volatile + // CHECK: load volatile + // CHECK-NOT: load volatile + ({n;}); + + // CHECK-LABEL: @then(i32 1) + then(1); + + // CHECK-NOT: load volatile + // CHECK: load volatile + // CHECK-NOT: load volatile + ({goto lab; lab: n;}); + + // CHECK-LABEL: @then(i32 2) + then(2); + + // CHECK-NOT: load volatile + // CHECK: load volatile + // CHECK-NOT: load volatile + ({[[gsl::suppress("foo")]] n;}); + + // CHECK-LABEL: @then(i32 3) + then(3); + + // CHECK-NOT: load volatile + // CHECK: load volatile + // CHECK-NOT: load volatile + ({if (true) n;}); + + // CHECK: } +} +template void volatile_load_template<int>(); diff --git a/test/CodeGenCXX/volatile.cpp b/test/CodeGenCXX/volatile.cpp index 9c0271b21d..cf49aed4de 100644 --- a/test/CodeGenCXX/volatile.cpp +++ b/test/CodeGenCXX/volatile.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -emit-llvm -std=c++98 -o - | FileCheck %s +// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -emit-llvm -std=c++98 -o - | FileCheck -check-prefix=CHECK -check-prefix=CHECK98 %s // RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -emit-llvm -std=c++11 -o - | FileCheck -check-prefix=CHECK -check-prefix=CHECK11 %s // Check that IR gen doesn't try to do an lvalue-to-rvalue conversion @@ -33,3 +33,19 @@ namespace test1 { *x; } } + +namespace PR40642 { + template <class T> struct S { + // CHECK-LABEL: define {{.*}} @_ZN7PR406421SIiE3fooEv( + void foo() { + // CHECK98-NOT: load volatile + // CHECK11: load volatile + if (true) + reinterpret_cast<const volatile unsigned char *>(m_ptr)[0]; + // CHECK: } + } + int *m_ptr; + }; + + void f(S<int> *x) { x->foo(); } +} diff --git a/test/OpenMP/barrier_messages.cpp b/test/OpenMP/barrier_messages.cpp index 137453a893..42edc9f46a 100644 --- a/test/OpenMP/barrier_messages.cpp +++ b/test/OpenMP/barrier_messages.cpp @@ -29,7 +29,7 @@ T tmain(T argc) { #pragma omp barrier // expected-error {{'#pragma omp barrier' cannot be an immediate substatement}} switch (argc) case 1: -#pragma omp barrier +#pragma omp barrier // expected-error {{'#pragma omp barrier' cannot be an immediate substatement}} switch (argc) case 1: { #pragma omp barrier @@ -49,7 +49,7 @@ T tmain(T argc) { #pragma omp barrier } label: -#pragma omp barrier // expected-error {{'#pragma omp barrier' cannot be an immediate substatement}} +#pragma omp barrier label1 : { #pragma omp barrier } @@ -83,7 +83,7 @@ int main(int argc, char **argv) { #pragma omp barrier // expected-error {{'#pragma omp barrier' cannot be an immediate substatement}} switch (argc) case 1: -#pragma omp barrier +#pragma omp barrier // expected-error {{'#pragma omp barrier' cannot be an immediate substatement}} switch (argc) case 1: { #pragma omp barrier @@ -103,7 +103,7 @@ int main(int argc, char **argv) { #pragma omp barrier } label: -#pragma omp barrier // expected-error {{'#pragma omp barrier' cannot be an immediate substatement}} +#pragma omp barrier label1 : { #pragma omp barrier } diff --git a/test/OpenMP/cancel_messages.cpp b/test/OpenMP/cancel_messages.cpp index b14c42f395..f78b28c1cb 100644 --- a/test/OpenMP/cancel_messages.cpp +++ b/test/OpenMP/cancel_messages.cpp @@ -63,7 +63,7 @@ int main(int argc, char **argv) { #pragma omp cancel parallel // expected-error {{'#pragma omp cancel' cannot be an immediate substatement}} expected-error {{orphaned 'omp cancel' directives are prohibited; perhaps you forget to enclose the directive into a region?}} switch (argc) case 1: -#pragma omp cancel sections // expected-error {{orphaned 'omp cancel' directives are prohibited; perhaps you forget to enclose the directive into a region?}} +#pragma omp cancel sections // expected-error {{'#pragma omp cancel' cannot be an immediate substatement}} expected-error {{orphaned 'omp cancel' directives are prohibited; perhaps you forget to enclose the directive into a region?}} switch (argc) case 1: { #pragma omp cancel for // expected-error {{orphaned 'omp cancel' directives are prohibited; perhaps you forget to enclose the directive into a region?}} @@ -83,7 +83,7 @@ int main(int argc, char **argv) { #pragma omp cancel taskgroup // expected-error {{orphaned 'omp cancel' directives are prohibited; perhaps you forget to enclose the directive into a region?}} } label: -#pragma omp cancel parallel // expected-error {{'#pragma omp cancel' cannot be an immediate substatement}} expected-error {{orphaned 'omp cancel' directives are prohibited; perhaps you forget to enclose the directive into a region?}} +#pragma omp cancel parallel // expected-error {{orphaned 'omp cancel' directives are prohibited; perhaps you forget to enclose the directive into a region?}} label1 : { #pragma omp cancel sections // expected-error {{orphaned 'omp cancel' directives are prohibited; perhaps you forget to enclose the directive into a region?}} } diff --git a/test/OpenMP/cancellation_point_messages.cpp b/test/OpenMP/cancellation_point_messages.cpp index 2bf667c407..058818df71 100644 --- a/test/OpenMP/cancellation_point_messages.cpp +++ b/test/OpenMP/cancellation_point_messages.cpp @@ -63,7 +63,7 @@ int main(int argc, char **argv) { #pragma omp cancellation point parallel // expected-error {{'#pragma omp cancellation point' cannot be an immediate substatement}} expected-error {{orphaned 'omp cancellation point' directives are prohibited; perhaps you forget to enclose the directive into a region?}} switch (argc) case 1: -#pragma omp cancellation point sections // expected-error {{orphaned 'omp cancellation point' directives are prohibited; perhaps you forget to enclose the directive into a region?}} +#pragma omp cancellation point sections // expected-error {{'#pragma omp cancellation point' cannot be an immediate substatement}} expected-error {{orphaned 'omp cancellation point' directives are prohibited; perhaps you forget to enclose the directive into a region?}} switch (argc) case 1: { #pragma omp cancellation point for // expected-error {{orphaned 'omp cancellation point' directives are prohibited; perhaps you forget to enclose the directive into a region?}} @@ -83,7 +83,7 @@ int main(int argc, char **argv) { #pragma omp cancellation point taskgroup // expected-error {{orphaned 'omp cancellation point' directives are prohibited; perhaps you forget to enclose the directive into a region?}} } label: -#pragma omp cancellation point parallel // expected-error {{'#pragma omp cancellation point' cannot be an immediate substatement}} expected-error {{orphaned 'omp cancellation point' directives are prohibited; perhaps you forget to enclose the directive into a region?}} +#pragma omp cancellation point parallel // expected-error {{orphaned 'omp cancellation point' directives are prohibited; perhaps you forget to enclose the directive into a region?}} label1 : { #pragma omp cancellation point sections // expected-error {{orphaned 'omp cancellation point' directives are prohibited; perhaps you forget to enclose the directive into a region?}} } diff --git a/test/OpenMP/flush_messages.cpp b/test/OpenMP/flush_messages.cpp index da1d0c2412..f4456489a9 100644 --- a/test/OpenMP/flush_messages.cpp +++ b/test/OpenMP/flush_messages.cpp @@ -33,7 +33,7 @@ T tmain(T argc) { #pragma omp flush // expected-error {{'#pragma omp flush' cannot be an immediate substatement}} switch (argc) case 1: -#pragma omp flush +#pragma omp flush // expected-error {{'#pragma omp flush' cannot be an immediate substatement}} switch (argc) case 1: { #pragma omp flush @@ -53,7 +53,7 @@ T tmain(T argc) { #pragma omp flush } label: -#pragma omp flush // expected-error {{'#pragma omp flush' cannot be an immediate substatement}} +#pragma omp flush label1 : { #pragma omp flush } @@ -97,7 +97,7 @@ int main(int argc, char **argv) { #pragma omp flush // expected-error {{'#pragma omp flush' cannot be an immediate substatement}} switch (argc) case 1: -#pragma omp flush +#pragma omp flush // expected-error {{'#pragma omp flush' cannot be an immediate substatement}} switch (argc) case 1: { #pragma omp flush @@ -117,7 +117,7 @@ int main(int argc, char **argv) { #pragma omp flush } label: -#pragma omp flush // expected-error {{'#pragma omp flush' cannot be an immediate substatement}} +#pragma omp flush label1 : { #pragma omp flush } diff --git a/test/OpenMP/taskwait_messages.cpp b/test/OpenMP/taskwait_messages.cpp index 46efff5972..55bf2ddcad 100644 --- a/test/OpenMP/taskwait_messages.cpp +++ b/test/OpenMP/taskwait_messages.cpp @@ -29,7 +29,7 @@ T tmain(T argc) { #pragma omp taskwait // expected-error {{'#pragma omp taskwait' cannot be an immediate substatement}} switch (argc) case 1: -#pragma omp taskwait +#pragma omp taskwait // expected-error {{'#pragma omp taskwait' cannot be an immediate substatement}} switch (argc) case 1: { #pragma omp taskwait @@ -49,7 +49,7 @@ T tmain(T argc) { #pragma omp taskwait } label: -#pragma omp taskwait // expected-error {{'#pragma omp taskwait' cannot be an immediate substatement}} +#pragma omp taskwait label1 : { #pragma omp taskwait } @@ -83,7 +83,7 @@ int main(int argc, char **argv) { #pragma omp taskwait // expected-error {{'#pragma omp taskwait' cannot be an immediate substatement}} switch (argc) case 1: -#pragma omp taskwait +#pragma omp taskwait // expected-error {{'#pragma omp taskwait' cannot be an immediate substatement}} switch (argc) case 1: { #pragma omp taskwait @@ -103,7 +103,7 @@ int main(int argc, char **argv) { #pragma omp taskwait } label: -#pragma omp taskwait // expected-error {{'#pragma omp taskwait' cannot be an immediate substatement}} +#pragma omp taskwait label1 : { #pragma omp taskwait } diff --git a/test/OpenMP/taskyield_messages.cpp b/test/OpenMP/taskyield_messages.cpp index fd98a8786f..48f741df91 100644 --- a/test/OpenMP/taskyield_messages.cpp +++ b/test/OpenMP/taskyield_messages.cpp @@ -29,7 +29,7 @@ T tmain(T argc) { #pragma omp taskyield // expected-error {{'#pragma omp taskyield' cannot be an immediate substatement}} switch (argc) case 1: -#pragma omp taskyield +#pragma omp taskyield // expected-error {{'#pragma omp taskyield' cannot be an immediate substatement}} switch (argc) case 1: { #pragma omp taskyield @@ -49,10 +49,13 @@ T tmain(T argc) { #pragma omp taskyield } label: -#pragma omp taskyield // expected-error {{'#pragma omp taskyield' cannot be an immediate substatement}} +#pragma omp taskyield label1 : { #pragma omp taskyield } +if (1) + label2: +#pragma omp taskyield // expected-error {{'#pragma omp taskyield' cannot be an immediate substatement}} return T(); } @@ -83,7 +86,7 @@ int main(int argc, char **argv) { #pragma omp taskyield // expected-error {{'#pragma omp taskyield' cannot be an immediate substatement}} switch (argc) case 1: -#pragma omp taskyield +#pragma omp taskyield // expected-error {{'#pragma omp taskyield' cannot be an immediate substatement}} switch (argc) case 1: { #pragma omp taskyield @@ -103,10 +106,13 @@ int main(int argc, char **argv) { #pragma omp taskyield } label: -#pragma omp taskyield // expected-error {{'#pragma omp taskyield' cannot be an immediate substatement}} +#pragma omp taskyield label1 : { #pragma omp taskyield } +if (1) + label2: +#pragma omp taskyield // expected-error {{'#pragma omp taskyield' cannot be an immediate substatement}} return tmain(argc); } |