[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 13:23:55 PST 2020
The test case is not important at all, but the crasher seems to be
rather bad. I'll send you the full test case.
On Thu, Mar 5, 2020 at 9:20 PM Richard Smith via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
>
> On Thu, 5 Mar 2020 at 06:13, Benjamin Kramer via cfe-commits <cfe-commits at lists.llvm.org> wrote:
>>
>> creduce produced this. It's a crash on invalid, but was created from a
>> valid input.
>
>
> Well, "valid" is unclear when using language extensions, but OK.
>
>>
>> $ cat r.ii
>> template <class a> auto b(a) {
>> auto c = [](auto, int) -> decltype(({})) {};
>> return c(0, 0);
>> }
>> using d = decltype(b(0));
>> bool f = d ::e;
>
>
> How important is this testcase? The fix you reverted was fixing a release-blocker; is this also a release-blocker in your view?
>
>>
>> $ clang r.ii -std=c++17 -w
>> clang-11: clang/lib/AST/Decl.cpp:2343: clang::APValue
>> *clang::VarDecl::evaluateValue(SmallVectorImpl<clang::PartialDiagnosticAt>
>> &) const: Assertion `!Init->isValueDependent()' failed.
>>
>> On Thu, Mar 5, 2020 at 2:18 PM Benjamin Kramer <benny.kra at gmail.com> wrote:
>> >
>> > 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
>> _______________________________________________
>> 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