From 71886c56f336667969be4cac0b6a17a3f75b7555 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Wed, 15 Jul 2020 19:38:46 -0700 Subject: [PATCH] Where possible, don't try to ask whether a template argument is dependent until it's been converted to match its parameter. The type of a non-type template parameter can in general affect whether the template argument is dependent. Note that this is not always possible. For template arguments that name static local variables in templates, the type of the template parameter affects whether the argument is dependent, so the query is imprecise until we know the parameter type. For example, in: template void f() { static const int n = 5; typename T::template X x; } ... we don't know whether 'n' is dependent until we know whether the corresponding template parameter is of type 'int' or 'const int&'. --- clang/include/clang/AST/Type.h | 23 +++++++++--- clang/lib/AST/Type.cpp | 26 ++++++------- clang/lib/Sema/SemaConcept.cpp | 23 +++++------- clang/lib/Sema/SemaDecl.cpp | 6 +-- clang/lib/Sema/SemaTemplate.cpp | 43 ++++++++-------------- .../test/SemaTemplate/instantiate-static-local.cpp | 27 ++++++++++++++ 6 files changed, 85 insertions(+), 63 deletions(-) create mode 100644 clang/test/SemaTemplate/instantiate-static-local.cpp diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h index a3059b8..21c8bf7 100644 --- a/clang/include/clang/AST/Type.h +++ b/clang/include/clang/AST/Type.h @@ -5112,11 +5112,24 @@ class alignas(8) TemplateSpecializationType public: /// Determine whether any of the given template arguments are dependent. - static bool anyDependentTemplateArguments(ArrayRef Args, - bool &InstantiationDependent); - - static bool anyDependentTemplateArguments(const TemplateArgumentListInfo &, - bool &InstantiationDependent); + /// + /// The converted arguments should be supplied when known; whether an + /// argument is dependent can depend on the conversions performed on it + /// (for example, a 'const int' passed as a template argument might be + /// dependent if the parameter is a reference but non-dependent if the + /// parameter is an int). + /// + /// Note that the \p Args parameter is unused: this is intentional, to remind + /// the caller that they need to pass in the converted arguments, not the + /// specified arguments. + static bool + anyDependentTemplateArguments(ArrayRef Args, + ArrayRef Converted); + static bool + anyDependentTemplateArguments(const TemplateArgumentListInfo &, + ArrayRef Converted); + static bool anyInstantiationDependentTemplateArguments( + ArrayRef Args); /// True if this template specialization type matches a current /// instantiation in the context in which it is found. diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp index e5811db..5dec80b 100644 --- a/clang/lib/AST/Type.cpp +++ b/clang/lib/AST/Type.cpp @@ -3590,24 +3590,24 @@ void SubstTemplateTypeParmPackType::Profile(llvm::FoldingSetNodeID &ID, ID.AddPointer(P.getAsType().getAsOpaquePtr()); } -bool TemplateSpecializationType:: -anyDependentTemplateArguments(const TemplateArgumentListInfo &Args, - bool &InstantiationDependent) { - return anyDependentTemplateArguments(Args.arguments(), - InstantiationDependent); +bool TemplateSpecializationType::anyDependentTemplateArguments( + const TemplateArgumentListInfo &Args, ArrayRef Converted) { + return anyDependentTemplateArguments(Args.arguments(), Converted); } -bool TemplateSpecializationType:: -anyDependentTemplateArguments(ArrayRef Args, - bool &InstantiationDependent) { - for (const TemplateArgumentLoc &ArgLoc : Args) { - if (ArgLoc.getArgument().isDependent()) { - InstantiationDependent = true; +bool TemplateSpecializationType::anyDependentTemplateArguments( + ArrayRef Args, ArrayRef Converted) { + for (const TemplateArgument &Arg : Converted) + if (Arg.isDependent()) return true; - } + return false; +} +bool TemplateSpecializationType::anyInstantiationDependentTemplateArguments( + ArrayRef Args) { + for (const TemplateArgumentLoc &ArgLoc : Args) { if (ArgLoc.getArgument().isInstantiationDependent()) - InstantiationDependent = true; + return true; } return false; } diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp index ddd95fa..1ff7b1c 100644 --- a/clang/lib/Sema/SemaConcept.cpp +++ b/clang/lib/Sema/SemaConcept.cpp @@ -1053,25 +1053,20 @@ ReturnTypeRequirement(TemplateParameterList *TPL) : auto *Constraint = cast_or_null( TC->getImmediatelyDeclaredConstraint()); - bool Dependent = false; - if (Constraint->getTemplateArgsAsWritten()) { - for (auto &ArgLoc : - Constraint->getTemplateArgsAsWritten()->arguments().drop_front(1)) { - if (ArgLoc.getArgument().isDependent()) { - Dependent = true; - break; - } - } - } + bool Dependent = + Constraint->getTemplateArgsAsWritten() && + TemplateSpecializationType::anyInstantiationDependentTemplateArguments( + Constraint->getTemplateArgsAsWritten()->arguments().drop_front(1)); TypeConstraintInfo.setInt(Dependent ? 1 : 0); } concepts::TypeRequirement::TypeRequirement(TypeSourceInfo *T) : - Requirement(RK_Type, T->getType()->isDependentType(), + Requirement(RK_Type, T->getType()->isInstantiationDependentType(), T->getType()->containsUnexpandedParameterPack(), // We reach this ctor with either dependent types (in which // IsSatisfied doesn't matter) or with non-dependent type in // which the existence of the type indicates satisfaction. - /*IsSatisfied=*/true - ), Value(T), - Status(T->getType()->isDependentType() ? SS_Dependent : SS_Satisfied) {} + /*IsSatisfied=*/true), + Value(T), + Status(T->getType()->isInstantiationDependentType() ? SS_Dependent + : SS_Satisfied) {} diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index f100550..2b6eb39 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -9516,12 +9516,10 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC, // that either the specialized function type or the specialized // template is dependent, and therefore matching will fail. In // this case, don't check the specialization yet. - bool InstantiationDependent = false; if (isFunctionTemplateSpecialization && isFriend && (NewFD->getType()->isDependentType() || DC->isDependentContext() || - TemplateSpecializationType::anyDependentTemplateArguments( - TemplateArgs, - InstantiationDependent))) { + TemplateSpecializationType::anyInstantiationDependentTemplateArguments( + TemplateArgs.arguments()))) { assert(HasExplicitTemplateArgs && "friend function specialization without template args"); if (CheckDependentFunctionTemplateSpecialization(NewFD, TemplateArgs, diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp index d5d11db..dd361ec 100644 --- a/clang/lib/Sema/SemaTemplate.cpp +++ b/clang/lib/Sema/SemaTemplate.cpp @@ -3661,7 +3661,6 @@ QualType Sema::CheckTemplateIdType(TemplateName Name, QualType CanonType; - bool InstantiationDependent = false; if (TypeAliasTemplateDecl *AliasTemplate = dyn_cast(Template)) { @@ -3724,7 +3723,7 @@ QualType Sema::CheckTemplateIdType(TemplateName Name, } } else if (Name.isDependent() || TemplateSpecializationType::anyDependentTemplateArguments( - TemplateArgs, InstantiationDependent)) { + TemplateArgs, Converted)) { // This class template specialization is a dependent // type. Therefore, its canonical type is another class template // specialization type that contains all of the converted @@ -4315,11 +4314,9 @@ DeclResult Sema::ActOnVarTemplateSpecialization( // FIXME: Move these checks to CheckTemplatePartialSpecializationArgs so we // also do them during instantiation. - bool InstantiationDependent; if (!Name.isDependent() && - !TemplateSpecializationType::anyDependentTemplateArguments( - TemplateArgs.arguments(), - InstantiationDependent)) { + !TemplateSpecializationType::anyDependentTemplateArguments(TemplateArgs, + Converted)) { Diag(TemplateNameLoc, diag::err_partial_spec_fully_specialized) << VarTemplate->getDeclName(); IsPartialSpecialization = false; @@ -4480,10 +4477,9 @@ Sema::CheckVarTemplateId(VarTemplateDecl *Template, SourceLocation TemplateLoc, return true; // Produce a placeholder value if the specialization is dependent. - bool InstantiationDependent = false; if (Template->getDeclContext()->isDependentContext() || - TemplateSpecializationType::anyDependentTemplateArguments( - TemplateArgs, InstantiationDependent)) + TemplateSpecializationType::anyDependentTemplateArguments(TemplateArgs, + Converted)) return DeclResult(); // Find the variable template specialization declaration that @@ -4669,22 +4665,16 @@ Sema::CheckConceptTemplateId(const CXXScopeSpec &SS, return ExprError(); ConstraintSatisfaction Satisfaction; - bool AreArgsDependent = false; - for (TemplateArgument &Arg : Converted) { - if (Arg.isDependent()) { - AreArgsDependent = true; - break; - } - } + bool AreArgsDependent = + TemplateSpecializationType::anyDependentTemplateArguments(*TemplateArgs, + Converted); if (!AreArgsDependent && - CheckConstraintSatisfaction(NamedConcept, - {NamedConcept->getConstraintExpr()}, - Converted, - SourceRange(SS.isSet() ? SS.getBeginLoc() : - ConceptNameInfo.getLoc(), - TemplateArgs->getRAngleLoc()), - Satisfaction)) - return ExprError(); + CheckConstraintSatisfaction( + NamedConcept, {NamedConcept->getConstraintExpr()}, Converted, + SourceRange(SS.isSet() ? SS.getBeginLoc() : ConceptNameInfo.getLoc(), + TemplateArgs->getRAngleLoc()), + Satisfaction)) + return ExprError(); return ConceptSpecializationExpr::Create(Context, SS.isSet() ? SS.getWithLocInContext(Context) : NestedNameSpecifierLoc{}, @@ -8345,10 +8335,9 @@ DeclResult Sema::ActOnClassTemplateSpecialization( // FIXME: Move this to CheckTemplatePartialSpecializationArgs so we // also do it during instantiation. - bool InstantiationDependent; if (!Name.isDependent() && - !TemplateSpecializationType::anyDependentTemplateArguments( - TemplateArgs.arguments(), InstantiationDependent)) { + !TemplateSpecializationType::anyDependentTemplateArguments(TemplateArgs, + Converted)) { Diag(TemplateNameLoc, diag::err_partial_spec_fully_specialized) << ClassTemplate->getDeclName(); isPartialSpecialization = false; diff --git a/clang/test/SemaTemplate/instantiate-static-local.cpp b/clang/test/SemaTemplate/instantiate-static-local.cpp new file mode 100644 index 0000000..dd13901 --- /dev/null +++ b/clang/test/SemaTemplate/instantiate-static-local.cpp @@ -0,0 +1,27 @@ +// RUN: %clang_cc1 -std=c++2a -x c++ %s -verify + +namespace use_after_instantiation { + template struct A { static constexpr int &value = R; }; + + template auto S() { + static int s; + return A{}; + } + + auto &s = decltype(S())::value; + + // This is ill-formed, but it should not crash. + // FIXME: Right now, it does crash. + // expected-no-diagnostics +#if 0 + template auto T() { + static int s; + struct A { + static constexpr int &value = s; // expected-error {{static}} + }; + return A{}; + } + + auto &t = decltype(T())::value; +#endif +} -- 2.7.4