Author: Richard Smith Date: 2020-03-03T15:20:40-08:00 New Revision: bdad0a1b79273733df9acc1be4e992fa5d70ec56
URL: https://github.com/llvm/llvm-project/commit/bdad0a1b79273733df9acc1be4e992fa5d70ec56 DIFF: https://github.com/llvm/llvm-project/commit/bdad0a1b79273733df9acc1be4e992fa5d70ec56.diff LOG: PR45083: Mark statement expressions as being dependent if they appear in dependent contexts. We previously assumed they were neither value- nor instantiation-dependent under any circumstances, which would lead to crashes and other misbehavior. Added: Modified: clang/include/clang/AST/Expr.h clang/include/clang/Sema/Sema.h clang/lib/AST/ASTImporter.cpp clang/lib/Parse/ParseExpr.cpp clang/lib/Sema/SemaExpr.cpp clang/lib/Sema/SemaExprCXX.cpp clang/lib/Sema/TreeTransform.h clang/test/SemaTemplate/dependent-expr.cpp Removed: ################################################################################ diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index fcdb0b992134..87f9b883486a 100644 --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -3960,14 +3960,14 @@ class StmtExpr : public Expr { Stmt *SubStmt; SourceLocation LParenLoc, RParenLoc; public: - // FIXME: Does type-dependence need to be computed diff erently? - // FIXME: Do we need to compute instantiation instantiation-dependence for - // statements? (ugh!) StmtExpr(CompoundStmt *substmt, QualType T, - SourceLocation lp, SourceLocation rp) : + SourceLocation lp, SourceLocation rp, bool InDependentContext) : + // Note: we treat a statement-expression in a dependent context as always + // being value- and instantiation-dependent. This matches the behavior of + // lambda-expressions and GCC. Expr(StmtExprClass, T, VK_RValue, OK_Ordinary, - T->isDependentType(), false, false, false), - SubStmt(substmt), LParenLoc(lp), RParenLoc(rp) { } + T->isDependentType(), InDependentContext, InDependentContext, false), + SubStmt(substmt), LParenLoc(lp), RParenLoc(rp) {} /// Build an empty statement expression. explicit StmtExpr(EmptyShell Empty) : Expr(StmtExprClass, Empty) { } diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 2304a9718567..5739808753e3 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -4964,7 +4964,7 @@ class Sema final { LabelDecl *TheDecl); void ActOnStartStmtExpr(); - ExprResult ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt, + ExprResult ActOnStmtExpr(Scope *S, SourceLocation LPLoc, Stmt *SubStmt, SourceLocation RPLoc); // "({..})" // Handle the final expression in a statement expression. ExprResult ActOnStmtExprResult(ExprResult E); diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 52288318337d..0cf00f6ca15b 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -6631,8 +6631,9 @@ ExpectedStmt ASTNodeImporter::VisitStmtExpr(StmtExpr *E) { if (Err) return std::move(Err); - return new (Importer.getToContext()) StmtExpr( - ToSubStmt, ToType, ToLParenLoc, ToRParenLoc); + return new (Importer.getToContext()) + StmtExpr(ToSubStmt, ToType, ToLParenLoc, ToRParenLoc, + E->isInstantiationDependent()); } ExpectedStmt ASTNodeImporter::VisitUnaryOperator(UnaryOperator *E) { diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp index 584de6b87d90..b038e6935d87 100644 --- a/clang/lib/Parse/ParseExpr.cpp +++ b/clang/lib/Parse/ParseExpr.cpp @@ -2655,7 +2655,8 @@ Parser::ParseParenExpression(ParenParseOption &ExprType, bool stopIfCastExpr, // If the substmt parsed correctly, build the AST node. if (!Stmt.isInvalid()) { - Result = Actions.ActOnStmtExpr(OpenLoc, Stmt.get(), Tok.getLocation()); + Result = Actions.ActOnStmtExpr(getCurScope(), OpenLoc, Stmt.get(), + Tok.getLocation()); } else { Actions.ActOnStmtExprError(); } diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index f50a77a40510..1870e440199d 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -13911,9 +13911,8 @@ void Sema::ActOnStmtExprError() { PopExpressionEvaluationContext(); } -ExprResult -Sema::ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt, - SourceLocation RPLoc) { // "({..})" +ExprResult Sema::ActOnStmtExpr(Scope *S, SourceLocation LPLoc, Stmt *SubStmt, + SourceLocation RPLoc) { // "({..})" assert(SubStmt && isa<CompoundStmt>(SubStmt) && "Invalid action invocation!"); CompoundStmt *Compound = cast<CompoundStmt>(SubStmt); @@ -13942,9 +13941,18 @@ Sema::ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt, } } + bool IsDependentContext = false; + if (S) + IsDependentContext = S->getTemplateParamParent() != nullptr; + else + // FIXME: This is not correct when substituting inside a templated + // context that isn't a DeclContext (such as a variable template). + IsDependentContext = CurContext->isDependentContext(); + // FIXME: Check that expression type is complete/non-abstract; statement // expressions are not lvalues. - Expr *ResStmtExpr = new (Context) StmtExpr(Compound, Ty, LPLoc, RPLoc); + Expr *ResStmtExpr = + new (Context) StmtExpr(Compound, Ty, LPLoc, RPLoc, IsDependentContext); if (StmtExprMayBindToTemp) return MaybeBindToTemporary(ResStmtExpr); return ResStmtExpr; diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index c8c6d7c95a7f..e521ba1d4af1 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -6955,8 +6955,9 @@ Stmt *Sema::MaybeCreateStmtWithCleanups(Stmt *SubStmt) { // a new AsmStmtWithTemporaries. CompoundStmt *CompStmt = CompoundStmt::Create( Context, SubStmt, SourceLocation(), SourceLocation()); - Expr *E = new (Context) StmtExpr(CompStmt, Context.VoidTy, SourceLocation(), - SourceLocation()); + Expr *E = new (Context) + StmtExpr(CompStmt, Context.VoidTy, SourceLocation(), SourceLocation(), + CurContext->isDependentContext()); return MaybeCreateExprWithCleanups(E); } diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index 002b73c3a1dd..05b41aa53da6 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -2549,10 +2549,9 @@ class TreeTransform { /// /// By default, performs semantic analysis to build the new expression. /// Subclasses may override this routine to provide diff erent behavior. - ExprResult RebuildStmtExpr(SourceLocation LParenLoc, - Stmt *SubStmt, - SourceLocation RParenLoc) { - return getSema().ActOnStmtExpr(LParenLoc, SubStmt, RParenLoc); + ExprResult RebuildStmtExpr(SourceLocation LParenLoc, Stmt *SubStmt, + SourceLocation RParenLoc) { + return getSema().ActOnStmtExpr(nullptr, LParenLoc, SubStmt, RParenLoc); } /// Build a new __builtin_choose_expr expression. @@ -11888,6 +11887,8 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) { NewTrailingRequiresClause = getDerived().TransformExpr(TRC); // Create the local class that will describe the lambda. + // FIXME: KnownDependent below is wrong when substituting inside a templated + // context that isn't a DeclContext (such as a variable template). CXXRecordDecl *OldClass = E->getLambdaClass(); CXXRecordDecl *Class = getSema().createLambdaClosureType(E->getIntroducerRange(), diff --git a/clang/test/SemaTemplate/dependent-expr.cpp b/clang/test/SemaTemplate/dependent-expr.cpp index bb1e239c3490..e333ed927b9e 100644 --- a/clang/test/SemaTemplate/dependent-expr.cpp +++ b/clang/test/SemaTemplate/dependent-expr.cpp @@ -1,5 +1,4 @@ // RUN: %clang_cc1 -fsyntax-only -verify %s -// expected-no-diagnostics // PR5908 template <typename Iterator> @@ -108,3 +107,22 @@ namespace PR18152 { }; template struct A<0>; } + +template<typename T> void stmt_expr_1() { + static_assert( ({ false; }), "" ); +} +void stmt_expr_2() { + static_assert( ({ false; }), "" ); // expected-error {{failed}} +} + +namespace PR45083 { + struct A { bool x; }; + + template<typename> struct B : A { + void f() { + const int n = ({ if (x) {} 0; }); + } + }; + + template void B<int>::f(); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits