From 617007240cbfb97c8ccf6d61b0c4ca0bb62d43c9 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Wed, 15 Jul 2020 19:37:15 -0700 Subject: [PATCH] Improve modeling of variable template specializations with dependent arguments. Don't build a variable template specialization declaration until its scope and template arguments are non-dependent. No functionality change intended, but the AST representation is now more consistent with how we model other templates. --- .../clang/Basic/DiagnosticSemaKinds.td | 2 + clang/include/clang/Sema/Sema.h | 10 +- clang/lib/Sema/SemaExprMember.cpp | 73 ++++---- clang/lib/Sema/SemaTemplate.cpp | 161 +++++++++--------- .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 20 +-- 5 files changed, 122 insertions(+), 144 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 7fe9396dbfc3a..d0bddd80b8fe5 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -4957,6 +4957,8 @@ def err_mismatched_exception_spec_explicit_instantiation : Error< def ext_mismatched_exception_spec_explicit_instantiation : ExtWarn< err_mismatched_exception_spec_explicit_instantiation.Text>, InGroup; +def err_explicit_instantiation_dependent : Error< + "explicit instantiation has dependent template arguments">; // C++ typename-specifiers def err_typename_nested_not_found : Error<"no type named %0 in %1">; diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 09e4c6743efc9..6965b3326388d 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -7342,11 +7342,17 @@ class Sema final { SourceLocation TemplateKWLoc, TemplateParameterList *TemplateParams, StorageClass SC, bool IsPartialSpecialization); + /// Get the specialization of the given variable template corresponding to + /// the specified argument list, or a null-but-valid result if the arguments + /// are dependent. DeclResult CheckVarTemplateId(VarTemplateDecl *Template, SourceLocation TemplateLoc, SourceLocation TemplateNameLoc, const TemplateArgumentListInfo &TemplateArgs); + /// Form a reference to the specialization of the given variable template + /// corresponding to the specified argument list, or a null-but-valid result + /// if the arguments are dependent. ExprResult CheckVarTemplateId(const CXXScopeSpec &SS, const DeclarationNameInfo &NameInfo, VarTemplateDecl *Template, @@ -9169,10 +9175,6 @@ class Sema final { bool InstantiatingVarTemplate = false, VarTemplateSpecializationDecl *PrevVTSD = nullptr); - VarDecl *getVarTemplateSpecialization( - VarTemplateDecl *VarTempl, const TemplateArgumentListInfo *TemplateArgs, - const DeclarationNameInfo &MemberNameInfo, SourceLocation TemplateKWLoc); - void InstantiateVariableInitializer( VarDecl *Var, VarDecl *OldVar, const MultiLevelTemplateArgumentList &TemplateArgs); diff --git a/clang/lib/Sema/SemaExprMember.cpp b/clang/lib/Sema/SemaExprMember.cpp index 466d1fe59c715..93ed756e084bb 100644 --- a/clang/lib/Sema/SemaExprMember.cpp +++ b/clang/lib/Sema/SemaExprMember.cpp @@ -944,28 +944,6 @@ static bool IsInFnTryBlockHandler(const Scope *S) { return false; } -VarDecl * -Sema::getVarTemplateSpecialization(VarTemplateDecl *VarTempl, - const TemplateArgumentListInfo *TemplateArgs, - const DeclarationNameInfo &MemberNameInfo, - SourceLocation TemplateKWLoc) { - if (!TemplateArgs) { - diagnoseMissingTemplateArguments(TemplateName(VarTempl), - MemberNameInfo.getBeginLoc()); - return nullptr; - } - - DeclResult VDecl = CheckVarTemplateId(VarTempl, TemplateKWLoc, - MemberNameInfo.getLoc(), *TemplateArgs); - if (VDecl.isInvalid()) - return nullptr; - VarDecl *Var = cast(VDecl.get()); - if (!Var->getTemplateSpecializationKind()) - Var->setTemplateSpecializationKind(TSK_ImplicitInstantiation, - MemberNameInfo.getLoc()); - return Var; -} - ExprResult Sema::BuildMemberReferenceExpr(Expr *BaseExpr, QualType BaseExprType, SourceLocation OpLoc, bool IsArrow, @@ -1097,19 +1075,11 @@ Sema::BuildMemberReferenceExpr(Expr *BaseExpr, QualType BaseExprType, if (!BaseExpr) { // If this is not an instance member, convert to a non-member access. if (!MemberDecl->isCXXInstanceMember()) { - // If this is a variable template, get the instantiated variable - // declaration corresponding to the supplied template arguments - // (while emitting diagnostics as necessary) that will be referenced - // by this expression. - assert((!TemplateArgs || isa(MemberDecl)) && - "How did we get template arguments here sans a variable template"); - if (isa(MemberDecl)) { - MemberDecl = getVarTemplateSpecialization( - cast(MemberDecl), TemplateArgs, - R.getLookupNameInfo(), TemplateKWLoc); - if (!MemberDecl) - return ExprError(); - } + // We might have a variable template specialization (or maybe one day a + // member concept-id). + if (TemplateArgs || TemplateKWLoc.isValid()) + return BuildTemplateIdExpr(SS, TemplateKWLoc, R, /*ADL*/false, TemplateArgs); + return BuildDeclarationNameExpr(SS, R.getLookupNameInfo(), MemberDecl, FoundDecl, TemplateArgs); } @@ -1168,14 +1138,32 @@ Sema::BuildMemberReferenceExpr(Expr *BaseExpr, QualType BaseExprType, MemberNameInfo, Enum->getType(), VK_RValue, OK_Ordinary); } + if (VarTemplateDecl *VarTempl = dyn_cast(MemberDecl)) { - if (VarDecl *Var = getVarTemplateSpecialization( - VarTempl, TemplateArgs, MemberNameInfo, TemplateKWLoc)) - return BuildMemberExpr( - BaseExpr, IsArrow, OpLoc, &SS, TemplateKWLoc, Var, FoundDecl, - /*HadMultipleCandidates=*/false, MemberNameInfo, - Var->getType().getNonReferenceType(), VK_LValue, OK_Ordinary); - return ExprError(); + if (!TemplateArgs) { + diagnoseMissingTemplateArguments(TemplateName(VarTempl), MemberLoc); + return ExprError(); + } + + DeclResult VDecl = CheckVarTemplateId(VarTempl, TemplateKWLoc, + MemberNameInfo.getLoc(), *TemplateArgs); + if (VDecl.isInvalid()) + return ExprError(); + + // Non-dependent member, but dependent template arguments. + if (!VDecl.get()) + return ActOnDependentMemberExpr( + BaseExpr, BaseExpr->getType(), IsArrow, OpLoc, SS, TemplateKWLoc, + FirstQualifierInScope, MemberNameInfo, TemplateArgs); + + VarDecl *Var = cast(VDecl.get()); + if (!Var->getTemplateSpecializationKind()) + Var->setTemplateSpecializationKind(TSK_ImplicitInstantiation, MemberLoc); + + return BuildMemberExpr( + BaseExpr, IsArrow, OpLoc, &SS, TemplateKWLoc, Var, FoundDecl, + /*HadMultipleCandidates=*/false, MemberNameInfo, + Var->getType().getNonReferenceType(), VK_LValue, OK_Ordinary); } // We found something that we didn't expect. Complain. @@ -1852,7 +1840,6 @@ Sema::BuildImplicitMemberExpr(const CXXScopeSpec &SS, // If this is known to be an instance access, go ahead and build an // implicit 'this' expression now. - // 'this' expression now. QualType ThisTy = getCurrentThisType(); assert(!ThisTy.isNull() && "didn't correctly pre-flight capture of 'this'"); diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp index d63d307b9ab7c..53954ca6ec411 100644 --- a/clang/lib/Sema/SemaTemplate.cpp +++ b/clang/lib/Sema/SemaTemplate.cpp @@ -4362,6 +4362,13 @@ Sema::CheckVarTemplateId(VarTemplateDecl *Template, SourceLocation TemplateLoc, Converted, /*UpdateArgsWithConversion=*/true)) return true; + // Produce a placeholder value if the specialization is dependent. + bool InstantiationDependent = false; + if (Template->getDeclContext()->isDependentContext() || + TemplateSpecializationType::anyDependentTemplateArguments( + TemplateArgs, InstantiationDependent)) + return DeclResult(); + // Find the variable template specialization declaration that // corresponds to these arguments. void *InsertPos = nullptr; @@ -4389,84 +4396,75 @@ Sema::CheckVarTemplateId(VarTemplateDecl *Template, SourceLocation TemplateLoc, // 1. Attempt to find the closest partial specialization that this // specializes, if any. - // If any of the template arguments is dependent, then this is probably - // a placeholder for an incomplete declarative context; which must be - // complete by instantiation time. Thus, do not search through the partial - // specializations yet. // TODO: Unify with InstantiateClassTemplateSpecialization()? // Perhaps better after unification of DeduceTemplateArguments() and // getMoreSpecializedPartialSpecialization(). - bool InstantiationDependent = false; - if (!TemplateSpecializationType::anyDependentTemplateArguments( - TemplateArgs, InstantiationDependent)) { - - SmallVector PartialSpecs; - Template->getPartialSpecializations(PartialSpecs); + SmallVector PartialSpecs; + Template->getPartialSpecializations(PartialSpecs); - for (unsigned I = 0, N = PartialSpecs.size(); I != N; ++I) { - VarTemplatePartialSpecializationDecl *Partial = PartialSpecs[I]; - TemplateDeductionInfo Info(FailedCandidates.getLocation()); + for (unsigned I = 0, N = PartialSpecs.size(); I != N; ++I) { + VarTemplatePartialSpecializationDecl *Partial = PartialSpecs[I]; + TemplateDeductionInfo Info(FailedCandidates.getLocation()); - if (TemplateDeductionResult Result = - DeduceTemplateArguments(Partial, TemplateArgList, Info)) { - // Store the failed-deduction information for use in diagnostics, later. - // TODO: Actually use the failed-deduction info? - FailedCandidates.addCandidate().set( - DeclAccessPair::make(Template, AS_public), Partial, - MakeDeductionFailureInfo(Context, Result, Info)); - (void)Result; - } else { - Matched.push_back(PartialSpecMatchResult()); - Matched.back().Partial = Partial; - Matched.back().Args = Info.take(); - } + if (TemplateDeductionResult Result = + DeduceTemplateArguments(Partial, TemplateArgList, Info)) { + // Store the failed-deduction information for use in diagnostics, later. + // TODO: Actually use the failed-deduction info? + FailedCandidates.addCandidate().set( + DeclAccessPair::make(Template, AS_public), Partial, + MakeDeductionFailureInfo(Context, Result, Info)); + (void)Result; + } else { + Matched.push_back(PartialSpecMatchResult()); + Matched.back().Partial = Partial; + Matched.back().Args = Info.take(); } + } - if (Matched.size() >= 1) { - SmallVector::iterator Best = Matched.begin(); - if (Matched.size() == 1) { - // -- If exactly one matching specialization is found, the - // instantiation is generated from that specialization. - // We don't need to do anything for this. - } else { - // -- If more than one matching specialization is found, the - // partial order rules (14.5.4.2) are used to determine - // whether one of the specializations is more specialized - // than the others. If none of the specializations is more - // specialized than all of the other matching - // specializations, then the use of the variable template is - // ambiguous and the program is ill-formed. - for (SmallVector::iterator P = Best + 1, - PEnd = Matched.end(); - P != PEnd; ++P) { - if (getMoreSpecializedPartialSpecialization(P->Partial, Best->Partial, - PointOfInstantiation) == - P->Partial) - Best = P; - } + if (Matched.size() >= 1) { + SmallVector::iterator Best = Matched.begin(); + if (Matched.size() == 1) { + // -- If exactly one matching specialization is found, the + // instantiation is generated from that specialization. + // We don't need to do anything for this. + } else { + // -- If more than one matching specialization is found, the + // partial order rules (14.5.4.2) are used to determine + // whether one of the specializations is more specialized + // than the others. If none of the specializations is more + // specialized than all of the other matching + // specializations, then the use of the variable template is + // ambiguous and the program is ill-formed. + for (SmallVector::iterator P = Best + 1, + PEnd = Matched.end(); + P != PEnd; ++P) { + if (getMoreSpecializedPartialSpecialization(P->Partial, Best->Partial, + PointOfInstantiation) == + P->Partial) + Best = P; + } - // Determine if the best partial specialization is more specialized than - // the others. - for (SmallVector::iterator P = Matched.begin(), - PEnd = Matched.end(); - P != PEnd; ++P) { - if (P != Best && getMoreSpecializedPartialSpecialization( - P->Partial, Best->Partial, - PointOfInstantiation) != Best->Partial) { - AmbiguousPartialSpec = true; - break; - } + // Determine if the best partial specialization is more specialized than + // the others. + for (SmallVector::iterator P = Matched.begin(), + PEnd = Matched.end(); + P != PEnd; ++P) { + if (P != Best && getMoreSpecializedPartialSpecialization( + P->Partial, Best->Partial, + PointOfInstantiation) != Best->Partial) { + AmbiguousPartialSpec = true; + break; } } - - // Instantiate using the best variable template partial specialization. - InstantiationPattern = Best->Partial; - InstantiationArgs = Best->Args; - } else { - // -- If no match is found, the instantiation is generated - // from the primary template. - // InstantiationPattern = Template->getTemplatedDecl(); } + + // Instantiate using the best variable template partial specialization. + InstantiationPattern = Best->Partial; + InstantiationArgs = Best->Args; + } else { + // -- If no match is found, the instantiation is generated + // from the primary template. + // InstantiationPattern = Template->getTemplatedDecl(); } // 2. Create the canonical declaration. @@ -4514,6 +4512,9 @@ Sema::CheckVarTemplateId(const CXXScopeSpec &SS, if (Decl.isInvalid()) return ExprError(); + if (!Decl.get()) + return ExprResult(); + VarDecl *Var = cast(Decl.get()); if (!Var->getTemplateSpecializationKind()) Var->setTemplateSpecializationKind(TSK_ImplicitInstantiation, @@ -4601,18 +4602,14 @@ ExprResult Sema::BuildTemplateIdExpr(const CXXScopeSpec &SS, } } - auto AnyDependentArguments = [&]() -> bool { - bool InstantiationDependent; - return TemplateArgs && - TemplateSpecializationType::anyDependentTemplateArguments( - *TemplateArgs, InstantiationDependent); - }; - // In C++1y, check variable template ids. - if (R.getAsSingle() && !AnyDependentArguments()) { - return CheckVarTemplateId(SS, R.getLookupNameInfo(), - R.getAsSingle(), - TemplateKWLoc, TemplateArgs); + if (R.getAsSingle()) { + ExprResult Res = CheckVarTemplateId(SS, R.getLookupNameInfo(), + R.getAsSingle(), + TemplateKWLoc, TemplateArgs); + if (Res.isInvalid() || Res.isUsable()) + return Res; + // Result is dependent. Carry on to build an UnresolvedLookupEpxr. } if (R.getAsSingle()) { @@ -9921,6 +9918,14 @@ DeclResult Sema::ActOnExplicitInstantiation(Scope *S, if (Res.isInvalid()) return true; + if (!Res.isUsable()) { + // We somehow specified dependent template arguments in an explicit + // instantiation. This should probably only happen during error + // recovery. + Diag(D.getIdentifierLoc(), diag::err_explicit_instantiation_dependent); + return true; + } + // Ignore access control bits, we don't need them for redeclaration // checking. Prev = cast(Res.get()); diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index 81c53b06420f0..965f1626ff2e5 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -5134,15 +5134,6 @@ void Sema::InstantiateVariableDefinition(SourceLocation PointOfInstantiation, VarTemplateSpecializationDecl *VarSpec = dyn_cast(Var); if (VarSpec) { - // If this is a variable template specialization, make sure that it is - // non-dependent. - bool InstantiationDependent = false; - assert(!TemplateSpecializationType::anyDependentTemplateArguments( - VarSpec->getTemplateArgsInfo(), InstantiationDependent) && - "Only instantiate variable template specializations that are " - "not type-dependent"); - (void)InstantiationDependent; - // If this is a static data member template, there might be an // uninstantiated initializer on the declaration. If so, instantiate // it now. @@ -5963,16 +5954,7 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc, NamedDecl *D, return nullptr; DeclContext::lookup_result Found = ParentDC->lookup(Name); - if (auto *VTSD = dyn_cast(D)) { - VarTemplateDecl *Templ = cast_or_null( - findInstantiationOf(Context, VTSD->getSpecializedTemplate(), - Found.begin(), Found.end())); - if (!Templ) - return nullptr; - Result = getVarTemplateSpecialization( - Templ, &VTSD->getTemplateArgsInfo(), NewNameInfo, SourceLocation()); - } else - Result = findInstantiationOf(Context, D, Found.begin(), Found.end()); + Result = findInstantiationOf(Context, D, Found.begin(), Found.end()); } else { // Since we don't have a name for the entity we're looking for, // our only option is to walk through all of the declarations to