From 66addf8e803618758457e4d578c5084e322ca448 Mon Sep 17 00:00:00 2001 From: Benjamin Kramer Date: Thu, 5 Mar 2020 13:52:05 +0100 Subject: [PATCH] Revert "Fix regression in bdad0a1: force rebuilding of StmtExpr nodes in", "PR45083: Mark statement expressions as being dependent if they appear in" This reverts commit f545ede91c9d9f271e7504282cab7bf509607ead. This reverts commit bdad0a1b79273733df9acc1be4e992fa5d70ec56. This crashes clang. I'll follow up with reproduction instructions. --- clang/include/clang/AST/Expr.h | 12 +++++----- clang/include/clang/Sema/Sema.h | 6 ++--- clang/lib/AST/ASTImporter.cpp | 5 ++-- clang/lib/Parse/ParseExpr.cpp | 3 +-- clang/lib/Sema/SemaExpr.cpp | 14 ++++------- clang/lib/Sema/SemaExprCXX.cpp | 5 ++-- clang/lib/Sema/TreeTransform.h | 19 ++++++--------- clang/test/SemaTemplate/dependent-expr.cpp | 27 +--------------------- 8 files changed, 25 insertions(+), 66 deletions(-) diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index ffc1f54fe82d..7271dbb830a2 100644 --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -3959,14 +3959,14 @@ class StmtExpr : public Expr { Stmt *SubStmt; SourceLocation LParenLoc, RParenLoc; public: + // FIXME: Does type-dependence need to be computed differently? + // FIXME: Do we need to compute instantiation instantiation-dependence for + // statements? (ugh!) StmtExpr(CompoundStmt *substmt, QualType T, - 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. + SourceLocation lp, SourceLocation rp) : Expr(StmtExprClass, T, VK_RValue, OK_Ordinary, - T->isDependentType(), InDependentContext, InDependentContext, false), - SubStmt(substmt), LParenLoc(lp), RParenLoc(rp) {} + T->isDependentType(), false, false, 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 8e4d0828e2d0..2304a9718567 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -4964,10 +4964,8 @@ public: LabelDecl *TheDecl); void ActOnStartStmtExpr(); - ExprResult ActOnStmtExpr(Scope *S, SourceLocation LPLoc, Stmt *SubStmt, - SourceLocation RPLoc); - ExprResult BuildStmtExpr(SourceLocation LPLoc, Stmt *SubStmt, - SourceLocation RPLoc, bool IsDependent); + ExprResult ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt, + SourceLocation RPLoc); // "({..})" // Handle the final expression in a statement expression. ExprResult ActOnStmtExprResult(ExprResult E); void ActOnStmtExprError(); diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index ddbd3699bdc2..9f174e9c2440 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -6631,9 +6631,8 @@ ExpectedStmt ASTNodeImporter::VisitStmtExpr(StmtExpr *E) { if (Err) return std::move(Err); - return new (Importer.getToContext()) - StmtExpr(ToSubStmt, ToType, ToLParenLoc, ToRParenLoc, - E->isInstantiationDependent()); + return new (Importer.getToContext()) StmtExpr( + ToSubStmt, ToType, ToLParenLoc, ToRParenLoc); } ExpectedStmt ASTNodeImporter::VisitUnaryOperator(UnaryOperator *E) { diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp index b038e6935d87..584de6b87d90 100644 --- a/clang/lib/Parse/ParseExpr.cpp +++ b/clang/lib/Parse/ParseExpr.cpp @@ -2655,8 +2655,7 @@ Parser::ParseParenExpression(ParenParseOption &ExprType, bool stopIfCastExpr, // If the substmt parsed correctly, build the AST node. if (!Stmt.isInvalid()) { - Result = Actions.ActOnStmtExpr(getCurScope(), OpenLoc, Stmt.get(), - Tok.getLocation()); + Result = Actions.ActOnStmtExpr(OpenLoc, Stmt.get(), Tok.getLocation()); } else { Actions.ActOnStmtExprError(); } diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 291c38ab20b4..f50a77a40510 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -13911,14 +13911,9 @@ void Sema::ActOnStmtExprError() { PopExpressionEvaluationContext(); } -ExprResult Sema::ActOnStmtExpr(Scope *S, SourceLocation LPLoc, Stmt *SubStmt, - SourceLocation RPLoc) { - return BuildStmtExpr(LPLoc, SubStmt, RPLoc, - S->getTemplateParamParent() != nullptr); -} - -ExprResult Sema::BuildStmtExpr(SourceLocation LPLoc, Stmt *SubStmt, - SourceLocation RPLoc, bool IsDependent) { +ExprResult +Sema::ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt, + SourceLocation RPLoc) { // "({..})" assert(SubStmt && isa(SubStmt) && "Invalid action invocation!"); CompoundStmt *Compound = cast(SubStmt); @@ -13949,8 +13944,7 @@ ExprResult Sema::BuildStmtExpr(SourceLocation LPLoc, Stmt *SubStmt, // FIXME: Check that expression type is complete/non-abstract; statement // expressions are not lvalues. - Expr *ResStmtExpr = - new (Context) StmtExpr(Compound, Ty, LPLoc, RPLoc, IsDependent); + Expr *ResStmtExpr = new (Context) StmtExpr(Compound, Ty, LPLoc, RPLoc); if (StmtExprMayBindToTemp) return MaybeBindToTemporary(ResStmtExpr); return ResStmtExpr; diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index b2e21bebd38f..3a4208c44530 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -6946,9 +6946,8 @@ 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(), - CurContext->isDependentContext()); + Expr *E = new (Context) StmtExpr(CompStmt, Context.VoidTy, SourceLocation(), + SourceLocation()); return MaybeCreateExprWithCleanups(E); } diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index 382496dad3d5..002b73c3a1dd 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -2549,9 +2549,10 @@ public: /// /// By default, performs semantic analysis to build the new expression. /// Subclasses may override this routine to provide different behavior. - ExprResult RebuildStmtExpr(SourceLocation LParenLoc, Stmt *SubStmt, - SourceLocation RParenLoc, bool IsDependent) { - return getSema().BuildStmtExpr(LParenLoc, SubStmt, RParenLoc, IsDependent); + ExprResult RebuildStmtExpr(SourceLocation LParenLoc, + Stmt *SubStmt, + SourceLocation RParenLoc) { + return getSema().ActOnStmtExpr(LParenLoc, SubStmt, RParenLoc); } /// Build a new __builtin_choose_expr expression. @@ -10432,20 +10433,16 @@ TreeTransform::TransformStmtExpr(StmtExpr *E) { return ExprError(); } - // FIXME: This is not correct when substituting inside a templated - // context that isn't a DeclContext (such as a variable template). - bool IsDependent = getSema().CurContext->isDependentContext(); - if (!getDerived().AlwaysRebuild() && - IsDependent == E->isInstantiationDependent() && SubStmt.get() == E->getSubStmt()) { // Calling this an 'error' is unintuitive, but it does the right thing. SemaRef.ActOnStmtExprError(); return SemaRef.MaybeBindToTemporary(E); } - return getDerived().RebuildStmtExpr(E->getLParenLoc(), SubStmt.get(), - E->getRParenLoc(), IsDependent); + return getDerived().RebuildStmtExpr(E->getLParenLoc(), + SubStmt.get(), + E->getRParenLoc()); } template @@ -11891,8 +11888,6 @@ TreeTransform::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 97d157f86424..bb1e239c3490 100644 --- a/clang/test/SemaTemplate/dependent-expr.cpp +++ b/clang/test/SemaTemplate/dependent-expr.cpp @@ -1,4 +1,5 @@ // RUN: %clang_cc1 -fsyntax-only -verify %s +// expected-no-diagnostics // PR5908 template @@ -107,29 +108,3 @@ namespace PR18152 { }; template struct A<0>; } - -template void stmt_expr_1() { - static_assert( ({ false; }), "" ); -} -void stmt_expr_2() { - static_assert( ({ false; }), "" ); // expected-error {{failed}} -} - -namespace PR45083 { - struct A { bool x; }; - - template struct B : A { - void f() { - const int n = ({ if (x) {} 0; }); - } - }; - - template void B::f(); - - // Make sure we properly rebuild statement expression AST nodes even if the - // only thing that changes is the "is dependent" flag. - template void f() { - decltype(({})) x; // expected-error {{incomplete type}} - } - template void f(); // expected-note {{instantiation of}} -} -- 2.34.1