From 5c845c1c50ac89a6f12557d1571678f3d1432478 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Mon, 9 Mar 2020 17:34:33 -0700 Subject: [PATCH] PR45083: Mark statement expressions as being dependent if they appear in a dependent context. This matches the GCC behavior. We track the enclosing template depth when determining whether a statement expression is within a dependent context; there doesn't appear to be any other reliable way to determine this. We previously assumed they were neither value- nor instantiation-dependent under any circumstances, which would lead to crashes and other misbehavior. --- clang/include/clang/AST/Expr.h | 22 ++++--- clang/include/clang/AST/Stmt.h | 15 +++++ clang/include/clang/Sema/Sema.h | 43 +++++++++++++- clang/include/clang/Sema/Template.h | 10 ++++ clang/lib/AST/ASTImporter.cpp | 5 +- clang/lib/Parse/ParseExpr.cpp | 3 +- clang/lib/Sema/SemaExpr.cpp | 13 ++-- clang/lib/Sema/SemaExprCXX.cpp | 5 +- clang/lib/Sema/SemaTemplate.cpp | 42 +++++++++++++ clang/lib/Sema/SemaTemplateInstantiate.cpp | 4 ++ clang/lib/Sema/TreeTransform.h | 28 ++++++--- clang/lib/Serialization/ASTReaderStmt.cpp | 1 + clang/lib/Serialization/ASTWriterStmt.cpp | 1 + clang/test/SemaTemplate/dependent-expr.cpp | 69 +++++++++++++++++++++- 14 files changed, 231 insertions(+), 30 deletions(-) diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index 7271dbb830a28..b1f17631c8925 100644 --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -3959,14 +3959,18 @@ 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) : - Expr(StmtExprClass, T, VK_RValue, OK_Ordinary, - T->isDependentType(), false, false, false), - SubStmt(substmt), LParenLoc(lp), RParenLoc(rp) { } + StmtExpr(CompoundStmt *SubStmt, QualType T, SourceLocation LParenLoc, + SourceLocation RParenLoc, unsigned TemplateDepth) + : // 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(), + TemplateDepth != 0, TemplateDepth != 0, false), + SubStmt(SubStmt), LParenLoc(LParenLoc), RParenLoc(RParenLoc) { + // FIXME: A templated statement expression should have an associated + // DeclContext so that nested declarations always have a dependent context. + StmtExprBits.TemplateDepth = TemplateDepth; + } /// Build an empty statement expression. explicit StmtExpr(EmptyShell Empty) : Expr(StmtExprClass, Empty) { } @@ -3983,6 +3987,8 @@ class StmtExpr : public Expr { SourceLocation getRParenLoc() const { return RParenLoc; } void setRParenLoc(SourceLocation L) { RParenLoc = L; } + unsigned getTemplateDepth() const { return StmtExprBits.TemplateDepth; } + static bool classof(const Stmt *T) { return T->getStmtClass() == StmtExprClass; } diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h index 5191a8261f869..dd1084623e9c2 100644 --- a/clang/include/clang/AST/Stmt.h +++ b/clang/include/clang/AST/Stmt.h @@ -589,6 +589,18 @@ class alignas(void *) Stmt { unsigned Kind : 2; }; + class StmtExprBitfields { + friend class ASTStmtReader; + friend class StmtExpr; + + unsigned : NumExprBits; + + /// The number of levels of template parameters enclosing this statement + /// expression. Used to determine if a statement expression remains + /// dependent after instantiation. + unsigned TemplateDepth; + }; + //===--- C++ Expression bitfields classes ---===// class CXXOperatorCallExprBitfields { @@ -997,6 +1009,9 @@ class alignas(void *) Stmt { PseudoObjectExprBitfields PseudoObjectExprBits; SourceLocExprBitfields SourceLocExprBits; + // GNU Extensions. + StmtExprBitfields StmtExprBits; + // C++ Expressions CXXOperatorCallExprBitfields CXXOperatorCallExprBits; CXXRewrittenBinaryOperatorBitfields CXXRewrittenBinaryOperatorBits; diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index d24dfe563bd28..cf02362e86bad 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -630,6 +630,15 @@ class Sema final { /// function, block, and method scopes that are currently active. SmallVector FunctionScopes; + /// The index of the first FunctionScope that corresponds to the current + /// context. + unsigned FunctionScopesStart = 0; + + ArrayRef getFunctionScopes() const { + return llvm::makeArrayRef(FunctionScopes.begin() + FunctionScopesStart, + FunctionScopes.end()); + } + /// Stack containing information needed when in C++2a an 'auto' is encountered /// in a function declaration parameter type specifier in order to invent a /// corresponding template parameter in the enclosing abbreviated function @@ -637,6 +646,16 @@ class Sema final { /// the FunctionScopes stack. SmallVector InventedParameterInfos; + /// The index of the first InventedParameterInfo that refers to the current + /// context. + unsigned InventedParameterInfosStart = 0; + + ArrayRef getInventedParameterInfos() const { + return llvm::makeArrayRef(InventedParameterInfos.begin() + + InventedParameterInfosStart, + InventedParameterInfos.end()); + } + typedef LazyVector ExtVectorDeclsType; @@ -810,17 +829,24 @@ class Sema final { DeclContext *SavedContext; ProcessingContextState SavedContextState; QualType SavedCXXThisTypeOverride; + unsigned SavedFunctionScopesStart; + unsigned SavedInventedParameterInfosStart; public: ContextRAII(Sema &S, DeclContext *ContextToPush, bool NewThisContext = true) : S(S), SavedContext(S.CurContext), SavedContextState(S.DelayedDiagnostics.pushUndelayed()), - SavedCXXThisTypeOverride(S.CXXThisTypeOverride) + SavedCXXThisTypeOverride(S.CXXThisTypeOverride), + SavedFunctionScopesStart(S.FunctionScopesStart), + SavedInventedParameterInfosStart(S.InventedParameterInfosStart) { assert(ContextToPush && "pushing null context"); S.CurContext = ContextToPush; if (NewThisContext) S.CXXThisTypeOverride = QualType(); + // Any saved FunctionScopes do not refer to this context. + S.FunctionScopesStart = S.FunctionScopes.size(); + S.InventedParameterInfosStart = S.InventedParameterInfos.size(); } void pop() { @@ -828,6 +854,8 @@ class Sema final { S.CurContext = SavedContext; S.DelayedDiagnostics.popUndelayed(SavedContextState); S.CXXThisTypeOverride = SavedCXXThisTypeOverride; + S.FunctionScopesStart = SavedFunctionScopesStart; + S.InventedParameterInfosStart = SavedInventedParameterInfosStart; SavedContext = nullptr; } @@ -4963,8 +4991,10 @@ class Sema final { LabelDecl *TheDecl); void ActOnStartStmtExpr(); - ExprResult ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt, - SourceLocation RPLoc); // "({..})" + ExprResult ActOnStmtExpr(Scope *S, SourceLocation LPLoc, Stmt *SubStmt, + SourceLocation RPLoc); + ExprResult BuildStmtExpr(SourceLocation LPLoc, Stmt *SubStmt, + SourceLocation RPLoc, unsigned TemplateDepth); // Handle the final expression in a statement expression. ExprResult ActOnStmtExprResult(ExprResult E); void ActOnStmtExprError(); @@ -12020,6 +12050,13 @@ class Sema final { return DC; } + /// Determine the number of levels of enclosing template parameters. This is + /// only usable while parsing. Note that this does not include dependent + /// contexts in which no template parameters have yet been declared, such as + /// in a terse function template or generic lambda before the first 'auto' is + /// encountered. + unsigned getTemplateDepth(Scope *S) const; + /// To be used for checking whether the arguments being passed to /// function exceeds the number of parameters expected for it. static bool TooManyArguments(size_t NumParams, size_t NumArgs, diff --git a/clang/include/clang/Sema/Template.h b/clang/include/clang/Sema/Template.h index 4c1cfecd4de68..47d6143bdc9aa 100644 --- a/clang/include/clang/Sema/Template.h +++ b/clang/include/clang/Sema/Template.h @@ -95,6 +95,16 @@ class VarDecl; return TemplateArgumentLists.size(); } + /// Determine how many of the \p OldDepth outermost template parameter + /// lists would be removed by substituting these arguments. + unsigned getNewDepth(unsigned OldDepth) const { + if (OldDepth < NumRetainedOuterLevels) + return OldDepth; + if (OldDepth < getNumLevels()) + return NumRetainedOuterLevels; + return OldDepth - TemplateArgumentLists.size(); + } + /// Retrieve the template argument at a given depth and index. const TemplateArgument &operator()(unsigned Depth, unsigned Index) const { assert(NumRetainedOuterLevels <= Depth && Depth < getNumLevels()); diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 86be6fc582a7f..c135e95a3dd59 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -6635,8 +6635,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->getTemplateDepth()); } ExpectedStmt ASTNodeImporter::VisitUnaryOperator(UnaryOperator *E) { diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp index 584de6b87d90a..b038e6935d873 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 dc69b198a289f..29592456d7b17 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -13921,9 +13921,13 @@ void Sema::ActOnStmtExprError() { PopExpressionEvaluationContext(); } -ExprResult -Sema::ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt, - SourceLocation RPLoc) { // "({..})" +ExprResult Sema::ActOnStmtExpr(Scope *S, SourceLocation LPLoc, Stmt *SubStmt, + SourceLocation RPLoc) { + return BuildStmtExpr(LPLoc, SubStmt, RPLoc, getTemplateDepth(S)); +} + +ExprResult Sema::BuildStmtExpr(SourceLocation LPLoc, Stmt *SubStmt, + SourceLocation RPLoc, unsigned TemplateDepth) { assert(SubStmt && isa(SubStmt) && "Invalid action invocation!"); CompoundStmt *Compound = cast(SubStmt); @@ -13954,7 +13958,8 @@ Sema::ActOnStmtExpr(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); + Expr *ResStmtExpr = + new (Context) StmtExpr(Compound, Ty, LPLoc, RPLoc, TemplateDepth); if (StmtExprMayBindToTemp) return MaybeBindToTemporary(ResStmtExpr); return ResStmtExpr; diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index 3a4208c445309..f81aabb55e4c8 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -6946,8 +6946,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(), + /*FIXME TemplateDepth=*/0); return MaybeCreateExprWithCleanups(E); } diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp index 3fb39853d9b4b..f7568e3b62b7d 100755 --- a/clang/lib/Sema/SemaTemplate.cpp +++ b/clang/lib/Sema/SemaTemplate.cpp @@ -46,6 +46,48 @@ clang::getTemplateParamsRange(TemplateParameterList const * const *Ps, return SourceRange(Ps[0]->getTemplateLoc(), Ps[N-1]->getRAngleLoc()); } +unsigned Sema::getTemplateDepth(Scope *S) const { + unsigned Depth = 0; + + // Each template parameter scope represents one level of template parameter + // depth. + for (Scope *TempParamScope = S->getTemplateParamParent(); + TempParamScope && !Depth; + TempParamScope = TempParamScope->getParent()->getTemplateParamParent()) { + ++Depth; + } + + // Note that there are template parameters with the given depth. + auto ParamsAtDepth = [&](unsigned D) { Depth = std::max(Depth, D + 1); }; + + // Look for parameters of an enclosing generic lambda. We don't create a + // template parameter scope for these. + for (FunctionScopeInfo *FSI : getFunctionScopes()) { + if (auto *LSI = dyn_cast(FSI)) { + if (!LSI->TemplateParams.empty()) { + ParamsAtDepth(LSI->AutoTemplateParameterDepth); + break; + } + if (LSI->GLTemplateParameterList) { + ParamsAtDepth(LSI->GLTemplateParameterList->getDepth()); + break; + } + } + } + + // Look for parameters of an enclosing terse function template. We don't + // create a template parameter scope for these either. + for (const InventedTemplateParameterInfo &Info : + getInventedParameterInfos()) { + if (!Info.TemplateParams.empty()) { + ParamsAtDepth(Info.AutoTemplateParameterDepth); + break; + } + } + + return Depth; +} + /// \brief Determine whether the declaration found is acceptable as the name /// of a template and, if so, return that template declaration. Otherwise, /// returns null. diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp index b37b4bbba783a..d68440cc6fc49 100644 --- a/clang/lib/Sema/SemaTemplateInstantiate.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp @@ -922,6 +922,10 @@ namespace { this->Entity = Entity; } + unsigned TransformTemplateDepth(unsigned Depth) { + return TemplateArgs.getNewDepth(Depth); + } + bool TryExpandParameterPacks(SourceLocation EllipsisLoc, SourceRange PatternRange, ArrayRef Unexpanded, diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index 0497a0df149a9..97600dcc7a847 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -212,6 +212,14 @@ class TreeTransform { return T.isNull(); } + /// Transform a template parameter depth level. + /// + /// During a transformation that transforms template parameters, this maps + /// an old template parameter depth to a new depth. + unsigned TransformTemplateDepth(unsigned Depth) { + return Depth; + } + /// Determine whether the given call argument should be dropped, e.g., /// because it is a default argument. /// @@ -2549,10 +2557,10 @@ class TreeTransform { /// /// 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) { - return getSema().ActOnStmtExpr(LParenLoc, SubStmt, RParenLoc); + ExprResult RebuildStmtExpr(SourceLocation LParenLoc, Stmt *SubStmt, + SourceLocation RParenLoc, unsigned TemplateDepth) { + return getSema().BuildStmtExpr(LParenLoc, SubStmt, RParenLoc, + TemplateDepth); } /// Build a new __builtin_choose_expr expression. @@ -10433,16 +10441,18 @@ TreeTransform::TransformStmtExpr(StmtExpr *E) { return ExprError(); } - if (!getDerived().AlwaysRebuild() && + unsigned OldDepth = E->getTemplateDepth(); + unsigned NewDepth = getDerived().TransformTemplateDepth(OldDepth); + + if (!getDerived().AlwaysRebuild() && OldDepth == NewDepth && 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()); + return getDerived().RebuildStmtExpr(E->getLParenLoc(), SubStmt.get(), + E->getRParenLoc(), NewDepth); } template @@ -11888,6 +11898,8 @@ 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/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp index 4c1abde29a208..1a3e2995ead4f 100644 --- a/clang/lib/Serialization/ASTReaderStmt.cpp +++ b/clang/lib/Serialization/ASTReaderStmt.cpp @@ -1205,6 +1205,7 @@ void ASTStmtReader::VisitStmtExpr(StmtExpr *E) { E->setLParenLoc(readSourceLocation()); E->setRParenLoc(readSourceLocation()); E->setSubStmt(cast_or_null(Record.readSubStmt())); + E->StmtExprBits.TemplateDepth = Record.readInt(); } void ASTStmtReader::VisitChooseExpr(ChooseExpr *E) { diff --git a/clang/lib/Serialization/ASTWriterStmt.cpp b/clang/lib/Serialization/ASTWriterStmt.cpp index bf63ac300c6d8..cea0fc0476e02 100644 --- a/clang/lib/Serialization/ASTWriterStmt.cpp +++ b/clang/lib/Serialization/ASTWriterStmt.cpp @@ -1069,6 +1069,7 @@ void ASTStmtWriter::VisitStmtExpr(StmtExpr *E) { Record.AddStmt(E->getSubStmt()); Record.AddSourceLocation(E->getLParenLoc()); Record.AddSourceLocation(E->getRParenLoc()); + Record.push_back(E->getTemplateDepth()); Code = serialization::EXPR_STMT; } diff --git a/clang/test/SemaTemplate/dependent-expr.cpp b/clang/test/SemaTemplate/dependent-expr.cpp index bb1e239c3490c..0ce4cb39d3493 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 +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++2a %s // PR5908 template @@ -108,3 +107,69 @@ 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(); + + template void f() { + decltype(({})) x; // expected-error {{incomplete type}} + } + template void f(); // expected-note {{instantiation of}} + + template auto g() { + auto c = [](auto, int) -> decltype(({})) {}; + using T = decltype(c(0.0, 0)); + using T = void; + return c(0, 0); + } + using U = decltype(g()); // expected-note {{previous}} + using U = float; // expected-error {{different types ('float' vs 'decltype(g())' (aka 'void'))}} + + void h(auto a, decltype(g())*) {} // expected-note {{previous}} + void h(auto a, void*) {} // expected-error {{redefinition}} + + void i(auto a) { + [](auto a, int = ({decltype(a) i; i * 2;})){}(a); // expected-error {{no matching function}} expected-note {{substitution failure}} + } + void use_i() { + i(0); + i((void*)0); // expected-note {{instantiation of}} + } +} + +namespace BindingInStmtExpr { + template struct overload : Ts... { + overload(Ts ...ts) : Ts(decltype(ts)(ts))... {} + using Ts::operator()...; + }; + + template struct N {}; + + template auto num_bindings() { + auto f0 = [](auto t, unsigned) { return N<0>(); }; + auto f1 = [](auto t, int) -> decltype(({ auto [_1] = t; N<1>(); })) { return {}; }; + auto f2 = [](auto t, int) -> decltype(({ auto [_1, _2] = t; N<2>(); })) { return {}; }; + auto f3 = [](auto t, int) -> decltype(({ auto [_1, _2, _3] = t; N<3>(); })) { return {}; }; + return decltype(overload(f0, f1, f2, f3)(T(), 0))(); + } + + struct T { int a; int b; }; + // Make sure we get a correct, non-dependent type back. + using U = decltype(num_bindings()); // expected-note {{previous}} + using U = N<3>; // expected-error-re {{type alias redefinition with different types ('N<3>' vs {{.*}}N<2>}} +}