<div dir="ltr"><div dir="ltr">On Thu, 5 Mar 2020 at 06:13, Benjamin Kramer via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">creduce produced this. It's a crash on invalid, but was created from a<br>
valid input.<br></blockquote><div><br></div><div></div><div>Well, "valid" is unclear when using language extensions, but OK.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
$ cat r.ii<br>
template <class a> auto b(a) {<br>
  auto c = [](auto, int) -> decltype(({})) {};<br>
  return c(0, 0);<br>
}<br>
using d = decltype(b(0));<br>
bool f = d ::e;<br></blockquote><div><br></div><div>How important is this testcase? The fix you reverted was fixing a release-blocker; is this also a release-blocker in your view?<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
$ clang r.ii -std=c++17 -w<br>
clang-11: clang/lib/AST/Decl.cpp:2343: clang::APValue<br>
*clang::VarDecl::evaluateValue(SmallVectorImpl<clang::PartialDiagnosticAt><br>
&) const: Assertion `!Init->isValueDependent()' failed.<br>
<br>
On Thu, Mar 5, 2020 at 2:18 PM Benjamin Kramer <<a href="mailto:benny.kra@gmail.com" target="_blank">benny.kra@gmail.com</a>> wrote:<br>
><br>
> It's still crashing clang, reverted this and<br>
> f545ede91c9d9f271e7504282cab7bf509607ead in 66addf8e8036. c-reduce is<br>
> still chewing on the reproducer.<br>
><br>
> On Wed, Mar 4, 2020 at 10:20 PM Richard Smith via cfe-commits<br>
> <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br>
> ><br>
> > We found a regression introduced by this patch; fixed in f545ede91c9d9f271e7504282cab7bf509607ead.<br>
> ><br>
> > On Wed, 4 Mar 2020 at 00:30, Hans Wennborg via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br>
> >><br>
> >> Pushed to 10.x as 3a843031a5ad83a00d2603f623881cb2b2bf719d. Please let<br>
> >> me know if you hear about any follow-up issues.<br>
> >><br>
> >> Thanks!<br>
> >><br>
> >> On Wed, Mar 4, 2020 at 12:28 AM Richard Smith via cfe-commits<br>
> >> <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br>
> >> ><br>
> >> ><br>
> >> > Author: Richard Smith<br>
> >> > Date: 2020-03-03T15:20:40-08:00<br>
> >> > New Revision: bdad0a1b79273733df9acc1be4e992fa5d70ec56<br>
> >> ><br>
> >> > URL: <a href="https://github.com/llvm/llvm-project/commit/bdad0a1b79273733df9acc1be4e992fa5d70ec56" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/bdad0a1b79273733df9acc1be4e992fa5d70ec56</a><br>
> >> > DIFF: <a href="https://github.com/llvm/llvm-project/commit/bdad0a1b79273733df9acc1be4e992fa5d70ec56.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/bdad0a1b79273733df9acc1be4e992fa5d70ec56.diff</a><br>
> >> ><br>
> >> > LOG: PR45083: Mark statement expressions as being dependent if they appear in<br>
> >> > dependent contexts.<br>
> >> ><br>
> >> > We previously assumed they were neither value- nor<br>
> >> > instantiation-dependent under any circumstances, which would lead to<br>
> >> > crashes and other misbehavior.<br>
> >> ><br>
> >> > Added:<br>
> >> ><br>
> >> ><br>
> >> > Modified:<br>
> >> >     clang/include/clang/AST/Expr.h<br>
> >> >     clang/include/clang/Sema/Sema.h<br>
> >> >     clang/lib/AST/ASTImporter.cpp<br>
> >> >     clang/lib/Parse/ParseExpr.cpp<br>
> >> >     clang/lib/Sema/SemaExpr.cpp<br>
> >> >     clang/lib/Sema/SemaExprCXX.cpp<br>
> >> >     clang/lib/Sema/TreeTransform.h<br>
> >> >     clang/test/SemaTemplate/dependent-expr.cpp<br>
> >> ><br>
> >> > Removed:<br>
> >> ><br>
> >> ><br>
> >> ><br>
> >> > ################################################################################<br>
> >> > diff  --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h<br>
> >> > index fcdb0b992134..87f9b883486a 100644<br>
> >> > --- a/clang/include/clang/AST/Expr.h<br>
> >> > +++ b/clang/include/clang/AST/Expr.h<br>
> >> > @@ -3960,14 +3960,14 @@ class StmtExpr : public Expr {<br>
> >> >    Stmt *SubStmt;<br>
> >> >    SourceLocation LParenLoc, RParenLoc;<br>
> >> >  public:<br>
> >> > -  // FIXME: Does type-dependence need to be computed<br>
> >> > diff erently?<br>
> >> > -  // FIXME: Do we need to compute instantiation instantiation-dependence for<br>
> >> > -  // statements? (ugh!)<br>
> >> >    StmtExpr(CompoundStmt *substmt, QualType T,<br>
> >> > -           SourceLocation lp, SourceLocation rp) :<br>
> >> > +           SourceLocation lp, SourceLocation rp, bool InDependentContext) :<br>
> >> > +    // Note: we treat a statement-expression in a dependent context as always<br>
> >> > +    // being value- and instantiation-dependent. This matches the behavior of<br>
> >> > +    // lambda-expressions and GCC.<br>
> >> >      Expr(StmtExprClass, T, VK_RValue, OK_Ordinary,<br>
> >> > -         T->isDependentType(), false, false, false),<br>
> >> > -    SubStmt(substmt), LParenLoc(lp), RParenLoc(rp) { }<br>
> >> > +         T->isDependentType(), InDependentContext, InDependentContext, false),<br>
> >> > +    SubStmt(substmt), LParenLoc(lp), RParenLoc(rp) {}<br>
> >> ><br>
> >> >    /// Build an empty statement expression.<br>
> >> >    explicit StmtExpr(EmptyShell Empty) : Expr(StmtExprClass, Empty) { }<br>
> >> ><br>
> >> > diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h<br>
> >> > index 2304a9718567..5739808753e3 100644<br>
> >> > --- a/clang/include/clang/Sema/Sema.h<br>
> >> > +++ b/clang/include/clang/Sema/Sema.h<br>
> >> > @@ -4964,7 +4964,7 @@ class Sema final {<br>
> >> >                              LabelDecl *TheDecl);<br>
> >> ><br>
> >> >    void ActOnStartStmtExpr();<br>
> >> > -  ExprResult ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt,<br>
> >> > +  ExprResult ActOnStmtExpr(Scope *S, SourceLocation LPLoc, Stmt *SubStmt,<br>
> >> >                             SourceLocation RPLoc); // "({..})"<br>
> >> >    // Handle the final expression in a statement expression.<br>
> >> >    ExprResult ActOnStmtExprResult(ExprResult E);<br>
> >> ><br>
> >> > diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp<br>
> >> > index 52288318337d..0cf00f6ca15b 100644<br>
> >> > --- a/clang/lib/AST/ASTImporter.cpp<br>
> >> > +++ b/clang/lib/AST/ASTImporter.cpp<br>
> >> > @@ -6631,8 +6631,9 @@ ExpectedStmt ASTNodeImporter::VisitStmtExpr(StmtExpr *E) {<br>
> >> >    if (Err)<br>
> >> >      return std::move(Err);<br>
> >> ><br>
> >> > -  return new (Importer.getToContext()) StmtExpr(<br>
> >> > -      ToSubStmt, ToType, ToLParenLoc, ToRParenLoc);<br>
> >> > +  return new (Importer.getToContext())<br>
> >> > +      StmtExpr(ToSubStmt, ToType, ToLParenLoc, ToRParenLoc,<br>
> >> > +               E->isInstantiationDependent());<br>
> >> >  }<br>
> >> ><br>
> >> >  ExpectedStmt ASTNodeImporter::VisitUnaryOperator(UnaryOperator *E) {<br>
> >> ><br>
> >> > diff  --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp<br>
> >> > index 584de6b87d90..b038e6935d87 100644<br>
> >> > --- a/clang/lib/Parse/ParseExpr.cpp<br>
> >> > +++ b/clang/lib/Parse/ParseExpr.cpp<br>
> >> > @@ -2655,7 +2655,8 @@ Parser::ParseParenExpression(ParenParseOption &ExprType, bool stopIfCastExpr,<br>
> >> ><br>
> >> >        // If the substmt parsed correctly, build the AST node.<br>
> >> >        if (!Stmt.isInvalid()) {<br>
> >> > -        Result = Actions.ActOnStmtExpr(OpenLoc, Stmt.get(), Tok.getLocation());<br>
> >> > +        Result = Actions.ActOnStmtExpr(getCurScope(), OpenLoc, Stmt.get(),<br>
> >> > +                                       Tok.getLocation());<br>
> >> >        } else {<br>
> >> >          Actions.ActOnStmtExprError();<br>
> >> >        }<br>
> >> ><br>
> >> > diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp<br>
> >> > index f50a77a40510..1870e440199d 100644<br>
> >> > --- a/clang/lib/Sema/SemaExpr.cpp<br>
> >> > +++ b/clang/lib/Sema/SemaExpr.cpp<br>
> >> > @@ -13911,9 +13911,8 @@ void Sema::ActOnStmtExprError() {<br>
> >> >    PopExpressionEvaluationContext();<br>
> >> >  }<br>
> >> ><br>
> >> > -ExprResult<br>
> >> > -Sema::ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt,<br>
> >> > -                    SourceLocation RPLoc) { // "({..})"<br>
> >> > +ExprResult Sema::ActOnStmtExpr(Scope *S, SourceLocation LPLoc, Stmt *SubStmt,<br>
> >> > +                               SourceLocation RPLoc) { // "({..})"<br>
> >> >    assert(SubStmt && isa<CompoundStmt>(SubStmt) && "Invalid action invocation!");<br>
> >> >    CompoundStmt *Compound = cast<CompoundStmt>(SubStmt);<br>
> >> ><br>
> >> > @@ -13942,9 +13941,18 @@ Sema::ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt,<br>
> >> >      }<br>
> >> >    }<br>
> >> ><br>
> >> > +  bool IsDependentContext = false;<br>
> >> > +  if (S)<br>
> >> > +    IsDependentContext = S->getTemplateParamParent() != nullptr;<br>
> >> > +  else<br>
> >> > +    // FIXME: This is not correct when substituting inside a templated<br>
> >> > +    // context that isn't a DeclContext (such as a variable template).<br>
> >> > +    IsDependentContext = CurContext->isDependentContext();<br>
> >> > +<br>
> >> >    // FIXME: Check that expression type is complete/non-abstract; statement<br>
> >> >    // expressions are not lvalues.<br>
> >> > -  Expr *ResStmtExpr = new (Context) StmtExpr(Compound, Ty, LPLoc, RPLoc);<br>
> >> > +  Expr *ResStmtExpr =<br>
> >> > +      new (Context) StmtExpr(Compound, Ty, LPLoc, RPLoc, IsDependentContext);<br>
> >> >    if (StmtExprMayBindToTemp)<br>
> >> >      return MaybeBindToTemporary(ResStmtExpr);<br>
> >> >    return ResStmtExpr;<br>
> >> ><br>
> >> > diff  --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp<br>
> >> > index c8c6d7c95a7f..e521ba1d4af1 100644<br>
> >> > --- a/clang/lib/Sema/SemaExprCXX.cpp<br>
> >> > +++ b/clang/lib/Sema/SemaExprCXX.cpp<br>
> >> > @@ -6955,8 +6955,9 @@ Stmt *Sema::MaybeCreateStmtWithCleanups(Stmt *SubStmt) {<br>
> >> >    // a new AsmStmtWithTemporaries.<br>
> >> >    CompoundStmt *CompStmt = CompoundStmt::Create(<br>
> >> >        Context, SubStmt, SourceLocation(), SourceLocation());<br>
> >> > -  Expr *E = new (Context) StmtExpr(CompStmt, Context.VoidTy, SourceLocation(),<br>
> >> > -                                   SourceLocation());<br>
> >> > +  Expr *E = new (Context)<br>
> >> > +      StmtExpr(CompStmt, Context.VoidTy, SourceLocation(), SourceLocation(),<br>
> >> > +               CurContext->isDependentContext());<br>
> >> >    return MaybeCreateExprWithCleanups(E);<br>
> >> >  }<br>
> >> ><br>
> >> ><br>
> >> > diff  --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h<br>
> >> > index 002b73c3a1dd..05b41aa53da6 100644<br>
> >> > --- a/clang/lib/Sema/TreeTransform.h<br>
> >> > +++ b/clang/lib/Sema/TreeTransform.h<br>
> >> > @@ -2549,10 +2549,9 @@ class TreeTransform {<br>
> >> >    ///<br>
> >> >    /// By default, performs semantic analysis to build the new expression.<br>
> >> >    /// Subclasses may override this routine to provide<br>
> >> > diff erent behavior.<br>
> >> > -  ExprResult RebuildStmtExpr(SourceLocation LParenLoc,<br>
> >> > -                                   Stmt *SubStmt,<br>
> >> > -                                   SourceLocation RParenLoc) {<br>
> >> > -    return getSema().ActOnStmtExpr(LParenLoc, SubStmt, RParenLoc);<br>
> >> > +  ExprResult RebuildStmtExpr(SourceLocation LParenLoc, Stmt *SubStmt,<br>
> >> > +                             SourceLocation RParenLoc) {<br>
> >> > +    return getSema().ActOnStmtExpr(nullptr, LParenLoc, SubStmt, RParenLoc);<br>
> >> >    }<br>
> >> ><br>
> >> >    /// Build a new __builtin_choose_expr expression.<br>
> >> > @@ -11888,6 +11887,8 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {<br>
> >> >      NewTrailingRequiresClause = getDerived().TransformExpr(TRC);<br>
> >> ><br>
> >> >    // Create the local class that will describe the lambda.<br>
> >> > +  // FIXME: KnownDependent below is wrong when substituting inside a templated<br>
> >> > +  // context that isn't a DeclContext (such as a variable template).<br>
> >> >    CXXRecordDecl *OldClass = E->getLambdaClass();<br>
> >> >    CXXRecordDecl *Class<br>
> >> >      = getSema().createLambdaClosureType(E->getIntroducerRange(),<br>
> >> ><br>
> >> > diff  --git a/clang/test/SemaTemplate/dependent-expr.cpp b/clang/test/SemaTemplate/dependent-expr.cpp<br>
> >> > index bb1e239c3490..e333ed927b9e 100644<br>
> >> > --- a/clang/test/SemaTemplate/dependent-expr.cpp<br>
> >> > +++ b/clang/test/SemaTemplate/dependent-expr.cpp<br>
> >> > @@ -1,5 +1,4 @@<br>
> >> >  // RUN: %clang_cc1 -fsyntax-only -verify %s<br>
> >> > -// expected-no-diagnostics<br>
> >> ><br>
> >> >  // PR5908<br>
> >> >  template <typename Iterator><br>
> >> > @@ -108,3 +107,22 @@ namespace PR18152 {<br>
> >> >    };<br>
> >> >    template struct A<0>;<br>
> >> >  }<br>
> >> > +<br>
> >> > +template<typename T> void stmt_expr_1() {<br>
> >> > +  static_assert( ({ false; }), "" );<br>
> >> > +}<br>
> >> > +void stmt_expr_2() {<br>
> >> > +  static_assert( ({ false; }), "" ); // expected-error {{failed}}<br>
> >> > +}<br>
> >> > +<br>
> >> > +namespace PR45083 {<br>
> >> > +  struct A { bool x; };<br>
> >> > +<br>
> >> > +  template<typename> struct B : A {<br>
> >> > +    void f() {<br>
> >> > +      const int n = ({ if (x) {} 0; });<br>
> >> > +    }<br>
> >> > +  };<br>
> >> > +<br>
> >> > +  template void B<int>::f();<br>
> >> > +}<br>
> >> ><br>
> >> ><br>
> >> ><br>
> >> > _______________________________________________<br>
> >> > cfe-commits mailing list<br>
> >> > <a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
> >> > <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
> >> _______________________________________________<br>
> >> cfe-commits mailing list<br>
> >> <a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
> >> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
> ><br>
> > _______________________________________________<br>
> > cfe-commits mailing list<br>
> > <a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
> > <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div>