[clang] bdad0a1 - PR45083: Mark statement expressions as being dependent if they appear in
Benjamin Kramer via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 5 05:18:12 PST 2020
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 at 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 at 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 at 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 at lists.llvm.org
>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list