[clang] Preserve Qualifiers and type sugar in TemplateNames

This patch improves the preservation of qualifiers
and loss of type sugar in TemplateNames.

This problem is analogous to https://reviews.llvm.org/D112374
and this patch takes a very similar approach to that patch,
except the impact here is much lesser.

When a TemplateName was written bare, without qualifications,
we wouldn't produce a QualifiedTemplate which could be used
to disambiguate it from a Canonical TemplateName. This had
effects in the TemplateName printer, which had workarounds
to deal with this, and wouldn't print the TemplateName
as-written in most situations.

There are also some related fixes to help preserve this type
sugar along the way into diagnostics, so that this patch can
be properly tested.

- Fix dropping the template keyword.
- Fix type deduction to preserve sugar in TST TemplateNames.

clang/docs/ReleaseNotes.rst

@@ -804,6 +804,8 @@ Bug Fixes to AST Handling
 - Clang now properly preserves ``FoundDecls`` within a ``ConceptReference``. (#GH82628)
 - The presence of the ``typename`` keyword is now stored in ``TemplateTemplateParmDecl``.
 - Fixed malformed AST generated for anonymous union access in templates. (#GH90842)
+- Improved preservation of qualifiers and sugar in TemplateNames, including
+  template keyword.
 
 Miscellaneous Bug Fixes
 ^^^^^^^^^^^^^^^^^^^^^^^

clang/include/clang/AST/TemplateName.h

@@ -332,7 +332,7 @@ class TemplateName {
   /// unexpanded parameter pack (for C++0x variadic templates).
   bool containsUnexpandedParameterPack() const;
 
-  enum class Qualified { None, AsWritten, Fully };
+  enum class Qualified { None, AsWritten };
   /// Print the template name.
   ///
   /// \param OS the output stream to which the template name will be

clang/include/clang/Sema/Sema.h

@@ -8988,6 +8988,9 @@ class Sema final : public SemaBase {
                          const TemplateArgumentListInfo *TemplateArgs);
 
   void diagnoseMissingTemplateArguments(TemplateName Name, SourceLocation Loc);
+  void diagnoseMissingTemplateArguments(const CXXScopeSpec &SS,
+                                        bool TemplateKeyword, TemplateDecl *TD,
+                                        SourceLocation Loc);
 
   ExprResult BuildTemplateIdExpr(const CXXScopeSpec &SS,
                                  SourceLocation TemplateKWLoc, LookupResult &R,

clang/lib/AST/ASTContext.cpp

@@ -5006,9 +5006,6 @@ ASTContext::getTemplateSpecializationType(TemplateName Template,
                                           QualType Underlying) const {
   assert(!Template.getAsDependentTemplateName() &&
          "No dependent template names here!");
-  // Look through qualified template names.
-  if (QualifiedTemplateName *QTN = Template.getAsQualifiedTemplateName())
-    Template = QTN->getUnderlyingTemplate();
 
   const auto *TD = Template.getAsTemplateDecl();
   bool IsTypeAlias = TD && TD->isTypeAlias();
@@ -5044,10 +5041,6 @@ QualType ASTContext::getCanonicalTemplateSpecializationType(
   assert(!Template.getAsDependentTemplateName() &&
          "No dependent template names here!");
 
-  // Look through qualified template names.
-  if (QualifiedTemplateName *QTN = Template.getAsQualifiedTemplateName())
-    Template = TemplateName(QTN->getUnderlyingTemplate());
-
   // Build the canonical template specialization type.
   TemplateName CanonTemplate = getCanonicalTemplateName(Template);
   bool AnyNonCanonArgs = false;
@@ -5262,10 +5255,12 @@ TemplateArgument ASTContext::getInjectedTemplateArg(NamedDecl *Param) {
     Arg = TemplateArgument(E);
   } else {
     auto *TTP = cast<TemplateTemplateParmDecl>(Param);
+    TemplateName Name = getQualifiedTemplateName(
+        nullptr, /*TemplateKeyword=*/false, TemplateName(TTP));
     if (TTP->isParameterPack())
-      Arg = TemplateArgument(TemplateName(TTP), std::optional<unsigned>());
+      Arg = TemplateArgument(Name, std::optional<unsigned>());
     else
-      Arg = TemplateArgument(TemplateName(TTP));
+      Arg = TemplateArgument(Name);
   }
 
   if (Param->isTemplateParameterPack())
@@ -9304,7 +9299,8 @@ TemplateName ASTContext::getAssumedTemplateName(DeclarationName Name) const {
 TemplateName ASTContext::getQualifiedTemplateName(NestedNameSpecifier *NNS,
                                                   bool TemplateKeyword,
                                                   TemplateName Template) const {
-  assert(NNS && "Missing nested-name-specifier in qualified template name");
+  assert(Template.getKind() == TemplateName::Template ||
+         Template.getKind() == TemplateName::UsingTemplate);
 
   // FIXME: Canonicalization?
   llvm::FoldingSetNodeID ID;

clang/lib/AST/DeclTemplate.cpp

@@ -627,9 +627,10 @@ ClassTemplateDecl::getInjectedClassNameSpecialization() {
   TemplateParameterList *Params = getTemplateParameters();
   SmallVector<TemplateArgument, 16> TemplateArgs;
   Context.getInjectedTemplateArgs(Params, TemplateArgs);
-  CommonPtr->InjectedClassNameType
-    = Context.getTemplateSpecializationType(TemplateName(this),
-                                            TemplateArgs);
+  TemplateName Name = Context.getQualifiedTemplateName(
+      /*NNS=*/nullptr, /*TemplateKeyword=*/false, TemplateName(this));
+  CommonPtr->InjectedClassNameType =
+      Context.getTemplateSpecializationType(Name, TemplateArgs);
   return CommonPtr->InjectedClassNameType;
 }
 

clang/lib/AST/ODRHash.cpp

@@ -146,10 +146,17 @@ void ODRHash::AddTemplateName(TemplateName Name) {
   case TemplateName::Template:
     AddDecl(Name.getAsTemplateDecl());
     break;
+  case TemplateName::QualifiedTemplate: {
+    QualifiedTemplateName *QTN = Name.getAsQualifiedTemplateName();
+    if (NestedNameSpecifier *NNS = QTN->getQualifier())
+      AddNestedNameSpecifier(NNS);
+    AddBoolean(QTN->hasTemplateKeyword());
+    AddTemplateName(QTN->getUnderlyingTemplate());
+    break;
+  }
   // TODO: Support these cases.
   case TemplateName::OverloadedTemplate:
   case TemplateName::AssumedTemplate:
-  case TemplateName::QualifiedTemplate:
   case TemplateName::DependentTemplate:
   case TemplateName::SubstTemplateTemplateParm:
   case TemplateName::SubstTemplateTemplateParmPack:

clang/lib/AST/TemplateBase.cpp

@@ -547,7 +547,7 @@ void TemplateArgument::print(const PrintingPolicy &Policy, raw_ostream &Out,
       const auto *TTP = cast<TemplateTemplateParmDecl>(TD);
       Out << "template-parameter-" << TTP->getDepth() << "-" << TTP->getIndex();
     } else {
-      TN.print(Out, Policy, TemplateName::Qualified::Fully);
+      TN.print(Out, Policy);
     }
     break;
   }

clang/lib/AST/TemplateName.cpp

@@ -235,8 +235,8 @@ TemplateNameDependence TemplateName::getDependence() const {
   auto D = TemplateNameDependence::None;
   switch (getKind()) {
   case TemplateName::NameKind::QualifiedTemplate:
-    D |= toTemplateNameDependence(
-        getAsQualifiedTemplateName()->getQualifier()->getDependence());
+    if (NestedNameSpecifier *NNS = getAsQualifiedTemplateName()->getQualifier())
+      D |= toTemplateNameDependence(NNS->getDependence());
     break;
   case TemplateName::NameKind::DependentTemplate:
     D |= toTemplateNameDependence(
@@ -292,9 +292,8 @@ void TemplateName::Profile(llvm::FoldingSetNodeID &ID) {
 
 void TemplateName::print(raw_ostream &OS, const PrintingPolicy &Policy,
                          Qualified Qual) const {
-  auto Kind = getKind();
-  TemplateDecl *Template = nullptr;
-  if (Kind == TemplateName::Template || Kind == TemplateName::UsingTemplate) {
+  if (NameKind Kind = getKind();
+      Kind == TemplateName::Template || Kind == TemplateName::UsingTemplate) {
     // After `namespace ns { using std::vector }`, what is the fully-qualified
     // name of the UsingTemplateName `vector` within ns?
     //
@@ -304,46 +303,43 @@ void TemplateName::print(raw_ostream &OS, const PrintingPolicy &Policy,
     // Similar to the UsingType behavior, using declarations are used to import
     // names more often than to export them, thus using the original name is
     // most useful in this case.
-    Template = getAsTemplateDecl();
-  }
-
-  if (Template)
-    if (Policy.CleanUglifiedParameters &&
-        isa<TemplateTemplateParmDecl>(Template) && Template->getIdentifier())
-      OS << Template->getIdentifier()->deuglifiedName();
-    else if (Qual == Qualified::Fully &&
-             getDependence() !=
-                 TemplateNameDependenceScope::DependentInstantiation)
-      Template->printQualifiedName(OS, Policy);
-    else
+    TemplateDecl *Template = getAsTemplateDecl();
+    if (Qual == Qualified::None)
       OS << *Template;
-  else if (QualifiedTemplateName *QTN = getAsQualifiedTemplateName()) {
-    if (Qual == Qualified::Fully &&
-        getDependence() !=
-            TemplateNameDependenceScope::DependentInstantiation) {
-      QTN->getUnderlyingTemplate().getAsTemplateDecl()->printQualifiedName(
-          OS, Policy);
-      return;
-    }
-    if (Qual == Qualified::AsWritten)
-      QTN->getQualifier()->print(OS, Policy);
+    else
+      Template->printQualifiedName(OS, Policy);
+  } else if (QualifiedTemplateName *QTN = getAsQualifiedTemplateName()) {
+    if (NestedNameSpecifier *NNS = QTN->getQualifier();
+        Qual != Qualified::None && NNS)
+      NNS->print(OS, Policy);
     if (QTN->hasTemplateKeyword())
       OS << "template ";
-    OS << *QTN->getUnderlyingTemplate().getAsTemplateDecl();
+
+    TemplateName Underlying = QTN->getUnderlyingTemplate();
+    assert(Underlying.getKind() == TemplateName::Template ||
+           Underlying.getKind() == TemplateName::UsingTemplate);
+
+    TemplateDecl *UTD = Underlying.getAsTemplateDecl();
+    if (IdentifierInfo *II = UTD->getIdentifier();
+        Policy.CleanUglifiedParameters && II &&
+        isa<TemplateTemplateParmDecl>(UTD))
+      OS << II->deuglifiedName();
+    else
+      OS << *UTD;
   } else if (DependentTemplateName *DTN = getAsDependentTemplateName()) {
-    if (Qual == Qualified::AsWritten && DTN->getQualifier())
-      DTN->getQualifier()->print(OS, Policy);
+    if (NestedNameSpecifier *NNS = DTN->getQualifier())
+      NNS->print(OS, Policy);
     OS << "template ";
 
     if (DTN->isIdentifier())
       OS << DTN->getIdentifier()->getName();
     else
       OS << "operator " << getOperatorSpelling(DTN->getOperator());
-  } else if (SubstTemplateTemplateParmStorage *subst
-               = getAsSubstTemplateTemplateParm()) {
+  } else if (SubstTemplateTemplateParmStorage *subst =
+                 getAsSubstTemplateTemplateParm()) {
     subst->getReplacement().print(OS, Policy, Qual);
-  } else if (SubstTemplateTemplateParmPackStorage *SubstPack
-                                        = getAsSubstTemplateTemplateParmPack())
+  } else if (SubstTemplateTemplateParmPackStorage *SubstPack =
+                 getAsSubstTemplateTemplateParmPack())
     OS << *SubstPack->getParameterPack();
   else if (AssumedTemplateStorage *Assumed = getAsAssumedTemplateName()) {
     Assumed->getDeclName().print(OS, Policy);

clang/lib/AST/TextNodeDumper.cpp

@@ -1988,15 +1988,15 @@ void TextNodeDumper::VisitAutoType(const AutoType *T) {
 
 void TextNodeDumper::VisitDeducedTemplateSpecializationType(
     const DeducedTemplateSpecializationType *T) {
-  if (T->getTemplateName().getKind() == TemplateName::UsingTemplate)
+  if (T->getTemplateName().getAsUsingShadowDecl())
     OS << " using";
 }
 
 void TextNodeDumper::VisitTemplateSpecializationType(
     const TemplateSpecializationType *T) {
   if (T->isTypeAlias())
     OS << " alias";
-  if (T->getTemplateName().getKind() == TemplateName::UsingTemplate)
+  if (T->getTemplateName().getAsUsingShadowDecl())
     OS << " using";
   OS << " ";
   T->getTemplateName().dump(OS);

clang/lib/AST/Type.cpp

@@ -4251,7 +4251,8 @@ TemplateSpecializationType::TemplateSpecializationType(
   assert((T.getKind() == TemplateName::Template ||
           T.getKind() == TemplateName::SubstTemplateTemplateParm ||
           T.getKind() == TemplateName::SubstTemplateTemplateParmPack ||
-          T.getKind() == TemplateName::UsingTemplate) &&
+          T.getKind() == TemplateName::UsingTemplate ||
+          T.getKind() == TemplateName::QualifiedTemplate) &&
          "Unexpected template name for TemplateSpecializationType");
 
   auto *TemplateArgs = reinterpret_cast<TemplateArgument *>(this + 1);

clang/lib/AST/TypePrinter.cpp

@@ -1586,14 +1586,14 @@ void TypePrinter::printTemplateId(const TemplateSpecializationType *T,
   IncludeStrongLifetimeRAII Strong(Policy);
 
   TemplateDecl *TD = T->getTemplateName().getAsTemplateDecl();
-  // FIXME: Null TD never excercised in test suite.
+  // FIXME: Null TD never exercised in test suite.
   if (FullyQualify && TD) {
     if (!Policy.SuppressScope)
       AppendScope(TD->getDeclContext(), OS, TD->getDeclName());
 
     OS << TD->getName();
   } else {
-    T->getTemplateName().print(OS, Policy);
+    T->getTemplateName().print(OS, Policy, TemplateName::Qualified::None);
   }
 
   DefaultTemplateArgsPolicyRAII TemplateArgs(Policy);

clang/lib/Sema/SemaDecl.cpp

@@ -538,8 +538,9 @@ ParsedType Sema::getTypeName(const IdentifierInfo &II, SourceLocation NameLoc,
   } else if (AllowDeducedTemplate) {
     if (auto *TD = getAsTypeTemplateDecl(IIDecl)) {
       assert(!FoundUsingShadow || FoundUsingShadow->getTargetDecl() == TD);
-      TemplateName Template =
-          FoundUsingShadow ? TemplateName(FoundUsingShadow) : TemplateName(TD);
+      TemplateName Template = Context.getQualifiedTemplateName(
+          SS ? SS->getScopeRep() : nullptr, /*TemplateKeyword=*/false,
+          FoundUsingShadow ? TemplateName(FoundUsingShadow) : TemplateName(TD));
       T = Context.getDeducedTemplateSpecializationType(Template, QualType(),
                                                        false);
       // Don't wrap in a further UsingType.
@@ -1137,12 +1138,10 @@ Sema::NameClassification Sema::ClassifyName(Scope *S, CXXScopeSpec &SS,
           dyn_cast<UsingShadowDecl>(*Result.begin());
       assert(!FoundUsingShadow ||
              TD == cast<TemplateDecl>(FoundUsingShadow->getTargetDecl()));
-      Template =
-          FoundUsingShadow ? TemplateName(FoundUsingShadow) : TemplateName(TD);
-      if (SS.isNotEmpty())
-        Template = Context.getQualifiedTemplateName(SS.getScopeRep(),
-                                                    /*TemplateKeyword=*/false,
-                                                    Template);
+      Template = Context.getQualifiedTemplateName(
+          SS.getScopeRep(),
+          /*TemplateKeyword=*/false,
+          FoundUsingShadow ? TemplateName(FoundUsingShadow) : TemplateName(TD));
     } else {
       // All results were non-template functions. This is a function template
       // name.

clang/lib/Sema/SemaDeclCXX.cpp

@@ -11547,12 +11547,12 @@ bool Sema::CheckDeductionGuideDeclarator(Declarator &D, QualType &R,
       TemplateName SpecifiedName = RetTST.getTypePtr()->getTemplateName();
       bool TemplateMatches =
           Context.hasSameTemplateName(SpecifiedName, GuidedTemplate);
-      auto TKind = SpecifiedName.getKind();
-      // A Using TemplateName can't actually be valid (either it's qualified, or
-      // we're in the wrong scope). But we have diagnosed these problems
-      // already.
-      bool SimplyWritten = TKind == TemplateName::Template ||
-                           TKind == TemplateName::UsingTemplate;
+
+      const QualifiedTemplateName *Qualifiers =
+          SpecifiedName.getAsQualifiedTemplateName();
+      assert(Qualifiers && "expected QualifiedTemplate");
+      bool SimplyWritten = !Qualifiers->hasTemplateKeyword() &&
+                           Qualifiers->getQualifier() == nullptr;
       if (SimplyWritten && TemplateMatches)
         AcceptableReturnType = true;
       else {

clang/lib/Sema/SemaExpr.cpp

@@ -3284,10 +3284,10 @@ ExprResult Sema::BuildDeclarationNameExpr(
     return CreateRecoveryExpr(NameInfo.getBeginLoc(), NameInfo.getEndLoc(), {});
   }
 
-  if (TemplateDecl *Template = dyn_cast<TemplateDecl>(D)) {
+  if (TemplateDecl *TD = dyn_cast<TemplateDecl>(D)) {
     // Specifically diagnose references to class templates that are missing
     // a template argument list.
-    diagnoseMissingTemplateArguments(TemplateName(Template), Loc);
+    diagnoseMissingTemplateArguments(SS, /*TemplateKeyword=*/false, TD, Loc);
     return ExprError();
   }
 

clang/lib/Sema/SemaExprMember.cpp

@@ -1194,7 +1194,8 @@ Sema::BuildMemberReferenceExpr(Expr *BaseExpr, QualType BaseExprType,
 
   if (VarTemplateDecl *VarTempl = dyn_cast<VarTemplateDecl>(MemberDecl)) {
     if (!TemplateArgs) {
-      diagnoseMissingTemplateArguments(TemplateName(VarTempl), MemberLoc);
+      diagnoseMissingTemplateArguments(
+          SS, /*TemplateKeyword=*/TemplateKWLoc.isValid(), VarTempl, MemberLoc);
       return ExprError();
     }
 

clang/lib/Sema/SemaTemplate.cpp

@@ -292,7 +292,7 @@ TemplateNameKind Sema::isTemplateName(Scope *S,
     Template =
         FoundUsingShadow ? TemplateName(FoundUsingShadow) : TemplateName(TD);
     assert(!FoundUsingShadow || FoundUsingShadow->getTargetDecl() == TD);
-    if (SS.isSet() && !SS.isInvalid()) {
+    if (!SS.isInvalid()) {
       NestedNameSpecifier *Qualifier = SS.getScopeRep();
       Template = Context.getQualifiedTemplateName(Qualifier, hasTemplateKeyword,
                                                   Template);
@@ -342,8 +342,11 @@ bool Sema::isDeductionGuideName(Scope *S, const IdentifierInfo &Name,
   if (!TD || !getAsTypeTemplateDecl(TD))
     return false;
 
-  if (Template)
-    *Template = TemplateTy::make(TemplateName(TD));
+  if (Template) {
+    TemplateName Name = Context.getQualifiedTemplateName(
+        SS.getScopeRep(), /*TemplateKeyword=*/false, TemplateName(TD));
+    *Template = TemplateTy::make(Name);
+  }
   return true;
 }
 
@@ -983,10 +986,6 @@ ParsedTemplateArgument Sema::ActOnTemplateTypeArgument(TypeResult ParsedType) {
 
     if (auto DTST = TL.getAs<DeducedTemplateSpecializationTypeLoc>()) {
       TemplateName Name = DTST.getTypePtr()->getTemplateName();
-      if (SS.isSet())
-        Name = Context.getQualifiedTemplateName(SS.getScopeRep(),
-                                                /*HasTemplateKeyword=*/false,
-                                                Name);
       ParsedTemplateArgument Result(SS, TemplateTy::make(Name),
                                     DTST.getTemplateNameLoc());
       if (EllipsisLoc.isValid())
@@ -5621,6 +5620,15 @@ void Sema::diagnoseMissingTemplateArguments(TemplateName Name,
   }
 }
 
+void Sema::diagnoseMissingTemplateArguments(const CXXScopeSpec &SS,
+                                            bool TemplateKeyword,
+                                            TemplateDecl *TD,
+                                            SourceLocation Loc) {
+  TemplateName Name = Context.getQualifiedTemplateName(
+      SS.getScopeRep(), TemplateKeyword, TemplateName(TD));
+  diagnoseMissingTemplateArguments(Name, Loc);
+}
+
 ExprResult
 Sema::CheckConceptTemplateId(const CXXScopeSpec &SS,
                              SourceLocation TemplateKWLoc,
@@ -5691,7 +5699,8 @@ ExprResult Sema::BuildTemplateIdExpr(const CXXScopeSpec &SS,
   // Non-function templates require a template argument list.
   if (auto *TD = R.getAsSingle<TemplateDecl>()) {
     if (!TemplateArgs && !isa<FunctionTemplateDecl>(TD)) {
-      diagnoseMissingTemplateArguments(TemplateName(TD), R.getNameLoc());
+      diagnoseMissingTemplateArguments(
+          SS, /*TemplateKeyword=*/TemplateKWLoc.isValid(), TD, R.getNameLoc());
       return ExprError();
     }
   }