It's still crashing clang, reverted this and f545ede91c9d9f271e7504282cab7bf509607ead in 66addf8e8036. c-reduce is still chewing on the reproducer.
On Wed, Mar 4, 2020 at 10:20 PM Richard Smith via cfe-commits <cfe-commits@lists.llvm.org> wrote: > > We found a regression introduced by this patch; fixed in > f545ede91c9d9f271e7504282cab7bf509607ead. > > On Wed, 4 Mar 2020 at 00:30, Hans Wennborg via cfe-commits > <cfe-commits@lists.llvm.org> wrote: >> >> Pushed to 10.x as 3a843031a5ad83a00d2603f623881cb2b2bf719d. Please let >> me know if you hear about any follow-up issues. >> >> Thanks! >> >> On Wed, Mar 4, 2020 at 12:28 AM Richard Smith via cfe-commits >> <cfe-commits@lists.llvm.org> wrote: >> > >> > >> > 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 >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
