[clang] Fix elaborated keyword canonicalization (#135916)

Author: mizvekov

clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp

@@ -54,8 +54,10 @@ static std::optional<TemplateSpecializationTypeLoc>
 matchEnableIfSpecializationImplTypename(TypeLoc TheType) {
   if (const auto Dep = TheType.getAs<DependentNameTypeLoc>()) {
     const IdentifierInfo *Identifier = Dep.getTypePtr()->getIdentifier();
+    ElaboratedTypeKeyword Keyword = Dep.getTypePtr()->getKeyword();
     if (!Identifier || Identifier->getName() != "type" ||
-        Dep.getTypePtr()->getKeyword() != ElaboratedTypeKeyword::Typename) {
+        (Keyword != ElaboratedTypeKeyword::Typename &&
+         Keyword != ElaboratedTypeKeyword::None)) {
       return std::nullopt;
     }
     TheType = Dep.getQualifierLoc().getTypeLoc();
@@ -108,8 +110,10 @@ matchEnableIfSpecializationImplTrait(TypeLoc TheType) {
 
     if (const auto *AliasedType =
             dyn_cast<DependentNameType>(Specialization->getAliasedType())) {
+      ElaboratedTypeKeyword Keyword = AliasedType->getKeyword();
       if (AliasedType->getIdentifier()->getName() != "type" ||
-          AliasedType->getKeyword() != ElaboratedTypeKeyword::Typename) {
+          (Keyword != ElaboratedTypeKeyword::Typename &&
+           Keyword != ElaboratedTypeKeyword::None)) {
         return std::nullopt;
       }
     } else {

clang/docs/ReleaseNotes.rst

@@ -486,6 +486,10 @@ Bug Fixes to C++ Support
 - Fixes matching of nested template template parameters. (#GH130362)
 - Correctly diagnoses template template paramters which have a pack parameter
   not in the last position.
+- Disallow overloading on struct vs class on dependent types, which is IFNDR, as
+  this makes the problem diagnosable.
+- Improved preservation of the presence or abscence of typename specifier when
+  printing types in diagnostics.
 - Clang now correctly parses ``if constexpr`` expressions in immediate function context. (#GH123524)
 - Fixed an assertion failure affecting code that uses C++23 "deducing this". (#GH130272)
 - Clang now properly instantiates destructors for initialized members within non-delegating constructors. (#GH93251)

clang/include/clang/AST/Type.h

@@ -2838,6 +2838,20 @@ class alignas(TypeAlignment) Type : public ExtQualsTypeCommonBase {
   /// immediately following this class.
   template <typename T> const T *getAs() const;
 
+  /// Look through sugar for an instance of TemplateSpecializationType which
+  /// is not a type alias, or null if there is no such type.
+  /// This is used when you want as-written template arguments or the template
+  /// name for a class template specialization.
+  const TemplateSpecializationType *
+  getAsNonAliasTemplateSpecializationType() const;
+
+  const TemplateSpecializationType *
+  castAsNonAliasTemplateSpecializationType() const {
+    const auto *TST = getAsNonAliasTemplateSpecializationType();
+    assert(TST && "not a TemplateSpecializationType");
+    return TST;
+  }
+
   /// Member-template getAsAdjusted<specific type>. Look through specific kinds
   /// of sugar (parens, attributes, etc) for an instance of \<specific type>.
   /// This is used when you need to walk over sugar nodes that represent some

clang/lib/AST/ASTContext.cpp

@@ -5747,6 +5747,30 @@ ASTContext::getMacroQualifiedType(QualType UnderlyingTy,
   return QualType(newType, 0);
 }
 
+static ElaboratedTypeKeyword
+getCanonicalElaboratedTypeKeyword(ElaboratedTypeKeyword Keyword) {
+  switch (Keyword) {
+  // These are just themselves.
+  case ElaboratedTypeKeyword::None:
+  case ElaboratedTypeKeyword::Struct:
+  case ElaboratedTypeKeyword::Union:
+  case ElaboratedTypeKeyword::Enum:
+  case ElaboratedTypeKeyword::Interface:
+    return Keyword;
+
+  // These are equivalent.
+  case ElaboratedTypeKeyword::Typename:
+    return ElaboratedTypeKeyword::None;
+
+  // These are functionally equivalent, so relying on their equivalence is
+  // IFNDR. By making them equivalent, we disallow overloading, which at least
+  // can produce a diagnostic.
+  case ElaboratedTypeKeyword::Class:
+    return ElaboratedTypeKeyword::Struct;
+  }
+  llvm_unreachable("unexpected keyword kind");
+}
+
 QualType ASTContext::getDependentNameType(ElaboratedTypeKeyword Keyword,
                                           NestedNameSpecifier *NNS,
                                           const IdentifierInfo *Name) const {
@@ -5758,10 +5782,13 @@ QualType ASTContext::getDependentNameType(ElaboratedTypeKeyword Keyword,
           DependentNameTypes.FindNodeOrInsertPos(ID, InsertPos))
     return QualType(T, 0);
 
+  ElaboratedTypeKeyword CanonKeyword =
+      getCanonicalElaboratedTypeKeyword(Keyword);
+  NestedNameSpecifier *CanonNNS = getCanonicalNestedNameSpecifier(NNS);
+
   QualType Canon;
-  if (NestedNameSpecifier *CanonNNS = getCanonicalNestedNameSpecifier(NNS);
-      CanonNNS != NNS) {
-    Canon = getDependentNameType(Keyword, CanonNNS, Name);
+  if (CanonKeyword != Keyword || CanonNNS != NNS) {
+    Canon = getDependentNameType(CanonKeyword, CanonNNS, Name);
     [[maybe_unused]] DependentNameType *T =
         DependentNameTypes.FindNodeOrInsertPos(ID, InsertPos);
     assert(!T && "broken canonicalization");
@@ -5800,27 +5827,27 @@ QualType ASTContext::getDependentTemplateSpecializationType(
 
   QualType Canon;
   if (!IsCanonical) {
-    ElaboratedTypeKeyword CanonKeyword = Keyword != ElaboratedTypeKeyword::None
-                                             ? Keyword
-                                             : ElaboratedTypeKeyword::Typename;
+    ElaboratedTypeKeyword CanonKeyword =
+        getCanonicalElaboratedTypeKeyword(Keyword);
     NestedNameSpecifier *CanonNNS = getCanonicalNestedNameSpecifier(NNS);
     bool AnyNonCanonArgs = false;
     auto CanonArgs =
         ::getCanonicalTemplateArguments(*this, Args, AnyNonCanonArgs);
 
-    if (AnyNonCanonArgs || CanonNNS != NNS || !Name.hasTemplateKeyword() ||
-        CanonKeyword != Keyword) {
+    if (CanonKeyword != Keyword || AnyNonCanonArgs || CanonNNS != NNS ||
+        !Name.hasTemplateKeyword()) {
       Canon = getDependentTemplateSpecializationType(
           CanonKeyword, {CanonNNS, Name.getName(), /*HasTemplateKeyword=*/true},
-          CanonArgs, /*IsCanonical=*/true);
+          CanonArgs,
+          /*IsCanonical=*/true);
       // Find the insert position again.
       [[maybe_unused]] auto *Nothing =
           DependentTemplateSpecializationTypes.FindNodeOrInsertPos(ID,
                                                                    InsertPos);
       assert(!Nothing && "canonical type broken");
     }
   } else {
-    assert(Keyword != ElaboratedTypeKeyword::None);
+    assert(Keyword == getCanonicalElaboratedTypeKeyword(Keyword));
     assert(Name.hasTemplateKeyword());
     assert(NNS == getCanonicalNestedNameSpecifier(NNS));
 #ifndef NDEBUG
@@ -7657,7 +7684,7 @@ ASTContext::getCanonicalNestedNameSpecifier(NestedNameSpecifier *NNS) const {
     if (const auto *DTST = T->getAs<DependentTemplateSpecializationType>()) {
       const DependentTemplateStorage &DTN = DTST->getDependentTemplateName();
       QualType NewT = getDependentTemplateSpecializationType(
-          ElaboratedTypeKeyword::Typename,
+          ElaboratedTypeKeyword::None,
           {/*NNS=*/nullptr, DTN.getName(), /*HasTemplateKeyword=*/true},
           DTST->template_arguments(), /*IsCanonical=*/true);
       assert(NewT.isCanonical());

clang/lib/AST/Type.cpp

@@ -1938,6 +1938,14 @@ TagDecl *Type::getAsTagDecl() const {
   return nullptr;
 }
 
+const TemplateSpecializationType *
+Type::getAsNonAliasTemplateSpecializationType() const {
+  const auto *TST = getAs<TemplateSpecializationType>();
+  while (TST && TST->isTypeAlias())
+    TST = TST->desugar()->getAs<TemplateSpecializationType>();
+  return TST;
+}
+
 bool Type::hasAttr(attr::Kind AK) const {
   const Type *Cur = this;
   while (const auto *AT = Cur->getAs<AttributedType>()) {

clang/lib/AST/TypeLoc.cpp

@@ -546,37 +546,47 @@ void UnaryTransformTypeLoc::initializeLocal(ASTContext &Context,
         Context.getTrivialTypeSourceInfo(getTypePtr()->getBaseType(), Loc));
 }
 
+template <class TL>
+static void initializeElaboratedKeyword(TL T, SourceLocation Loc) {
+  T.setElaboratedKeywordLoc(T.getTypePtr()->getKeyword() !=
+                                    ElaboratedTypeKeyword::None
+                                ? Loc
+                                : SourceLocation());
+}
+
+static NestedNameSpecifierLoc
+initializeQualifier(ASTContext &Context, NestedNameSpecifier *Qualifier,
+                    SourceLocation Loc) {
+  if (!Qualifier)
+    return NestedNameSpecifierLoc();
+  NestedNameSpecifierLocBuilder Builder;
+  Builder.MakeTrivial(Context, Qualifier, Loc);
+  return Builder.getWithLocInContext(Context);
+}
+
 void ElaboratedTypeLoc::initializeLocal(ASTContext &Context,
                                         SourceLocation Loc) {
   if (isEmpty())
     return;
-  setElaboratedKeywordLoc(Loc);
-  NestedNameSpecifierLocBuilder Builder;
-  Builder.MakeTrivial(Context, getTypePtr()->getQualifier(), Loc);
-  setQualifierLoc(Builder.getWithLocInContext(Context));
+  initializeElaboratedKeyword(*this, Loc);
+  setQualifierLoc(
+      initializeQualifier(Context, getTypePtr()->getQualifier(), Loc));
 }
 
 void DependentNameTypeLoc::initializeLocal(ASTContext &Context,
                                            SourceLocation Loc) {
-  setElaboratedKeywordLoc(Loc);
-  NestedNameSpecifierLocBuilder Builder;
-  Builder.MakeTrivial(Context, getTypePtr()->getQualifier(), Loc);
-  setQualifierLoc(Builder.getWithLocInContext(Context));
+  initializeElaboratedKeyword(*this, Loc);
+  setQualifierLoc(
+      initializeQualifier(Context, getTypePtr()->getQualifier(), Loc));
   setNameLoc(Loc);
 }
 
 void
 DependentTemplateSpecializationTypeLoc::initializeLocal(ASTContext &Context,
                                                         SourceLocation Loc) {
-  setElaboratedKeywordLoc(Loc);
-  if (NestedNameSpecifier *Qualifier =
-          getTypePtr()->getDependentTemplateName().getQualifier()) {
-    NestedNameSpecifierLocBuilder Builder;
-    Builder.MakeTrivial(Context, Qualifier, Loc);
-    setQualifierLoc(Builder.getWithLocInContext(Context));
-  } else {
-    setQualifierLoc(NestedNameSpecifierLoc());
-  }
+  initializeElaboratedKeyword(*this, Loc);
+  setQualifierLoc(initializeQualifier(
+      Context, getTypePtr()->getDependentTemplateName().getQualifier(), Loc));
   setTemplateKeywordLoc(Loc);
   setTemplateNameLoc(Loc);
   setLAngleLoc(Loc);

clang/lib/Sema/SemaDecl.cpp

@@ -247,15 +247,15 @@ static ParsedType recoverFromTypeInKnownDependentBase(Sema &S,
     return nullptr;
 
   // We found some types in dependent base classes.  Recover as if the user
-  // wrote 'typename MyClass::II' instead of 'II'.  We'll fully resolve the
-  // lookup during template instantiation.
+  // wrote 'MyClass::II' instead of 'II', and this implicit typename was
+  // allowed.  We'll fully resolve the lookup during template instantiation.
   S.Diag(NameLoc, diag::ext_found_in_dependent_base) << &II;
 
   ASTContext &Context = S.Context;
   auto *NNS = NestedNameSpecifier::Create(
       Context, nullptr, cast<Type>(Context.getRecordType(RD)));
   QualType T =
-      Context.getDependentNameType(ElaboratedTypeKeyword::Typename, NNS, &II);
+      Context.getDependentNameType(ElaboratedTypeKeyword::None, NNS, &II);
 
   CXXScopeSpec SS;
   SS.MakeTrivial(Context, NNS, SourceRange(NameLoc));

clang/lib/Sema/SemaDeclCXX.cpp

@@ -12154,33 +12154,31 @@ static bool isStdClassTemplate(Sema &S, QualType SugaredType, QualType *TypeArg,
   };
 
   ClassTemplateDecl *Template = nullptr;
-  const TemplateArgument *Arguments = nullptr;
-
-  QualType Ty = S.Context.getCanonicalType(SugaredType);
-  if (const RecordType *RT = Ty->getAs<RecordType>()) {
-    ClassTemplateSpecializationDecl *Specialization =
-        dyn_cast<ClassTemplateSpecializationDecl>(RT->getDecl());
-    if (!Specialization) {
-      ReportMatchingNameAsMalformed(RT->getDecl());
-      return false;
-    }
-
-    Template = Specialization->getSpecializedTemplate();
-    Arguments = Specialization->getTemplateArgs().data();
-  } else {
-    const TemplateSpecializationType *TST = nullptr;
-    if (auto *ICN = Ty->getAs<InjectedClassNameType>())
-      TST = ICN->getInjectedTST();
-    else
-      TST = Ty->getAs<TemplateSpecializationType>();
+  ArrayRef<TemplateArgument> Arguments;
+  {
+    const TemplateSpecializationType *TST =
+        SugaredType->getAsNonAliasTemplateSpecializationType();
+    if (!TST)
+      if (const auto *ICN = SugaredType->getAs<InjectedClassNameType>())
+        TST = ICN->getInjectedTST();
     if (TST) {
       Template = dyn_cast_or_null<ClassTemplateDecl>(
           TST->getTemplateName().getAsTemplateDecl());
-      Arguments = TST->template_arguments().begin();
+      Arguments = TST->template_arguments();
+    } else if (const RecordType *RT = SugaredType->getAs<RecordType>()) {
+      ClassTemplateSpecializationDecl *Specialization =
+          dyn_cast<ClassTemplateSpecializationDecl>(RT->getDecl());
+      if (!Specialization) {
+        ReportMatchingNameAsMalformed(RT->getDecl());
+        return false;
+      }
+      Template = Specialization->getSpecializedTemplate();
+      Arguments = Specialization->getTemplateArgs().asArray();
     }
   }
+
   if (!Template) {
-    ReportMatchingNameAsMalformed(Ty->getAsTagDecl());
+    ReportMatchingNameAsMalformed(SugaredType->getAsTagDecl());
     return false;
   }
 
@@ -12200,7 +12198,8 @@ static bool isStdClassTemplate(Sema &S, QualType SugaredType, QualType *TypeArg,
     // template?
     TemplateParameterList *Params = Template->getTemplateParameters();
     if (Params->getMinRequiredArguments() != 1 ||
-        !isa<TemplateTypeParmDecl>(Params->getParam(0))) {
+        !isa<TemplateTypeParmDecl>(Params->getParam(0)) ||
+        Params->getParam(0)->isTemplateParameterPack()) {
       if (MalformedDecl)
         *MalformedDecl = TemplateClass;
       return false;
@@ -12214,8 +12213,21 @@ static bool isStdClassTemplate(Sema &S, QualType SugaredType, QualType *TypeArg,
     return false;
 
   // This is an instance of std::{ClassName}. Find the argument type.
-  if (TypeArg)
-    *TypeArg = Arguments[0].getAsType();
+  if (TypeArg) {
+    QualType ArgType = Arguments[0].getAsType();
+    // FIXME: Since TST only has as-written arguments, we have to perform the
+    // only kind of conversion applicable to type arguments; in Objective-C ARC:
+    // - If an explicitly-specified template argument type is a lifetime type
+    //   with no lifetime qualifier, the __strong lifetime qualifier is
+    //   inferred.
+    if (S.getLangOpts().ObjCAutoRefCount && ArgType->isObjCLifetimeType() &&
+        !ArgType.getObjCLifetime()) {
+      Qualifiers Qs;
+      Qs.setObjCLifetime(Qualifiers::OCL_Strong);
+      ArgType = S.Context.getQualifiedType(ArgType, Qs);
+    }
+    *TypeArg = ArgType;
+  }
 
   return true;
 }

clang/lib/Sema/SemaInit.cpp

@@ -9897,7 +9897,7 @@ QualType Sema::DeduceTemplateSpecializationFromInitializer(
 
   auto TemplateName = DeducedTST->getTemplateName();
   if (TemplateName.isDependent())
-    return SubstAutoTypeDependent(TSInfo->getType());
+    return SubstAutoTypeSourceInfoDependent(TSInfo)->getType();
 
   // We can only perform deduction for class templates or alias templates.
   auto *Template =
@@ -9942,7 +9942,7 @@ QualType Sema::DeduceTemplateSpecializationFromInitializer(
     Diag(TSInfo->getTypeLoc().getBeginLoc(),
          diag::warn_cxx14_compat_class_template_argument_deduction)
         << TSInfo->getTypeLoc().getSourceRange() << 0;
-    return SubstAutoTypeDependent(TSInfo->getType());
+    return SubstAutoTypeSourceInfoDependent(TSInfo)->getType();
   }
 
   // FIXME: Perform "exact type" matching first, per CWG discussion?
@@ -10253,7 +10253,8 @@ QualType Sema::DeduceTemplateSpecializationFromInitializer(
   //  The placeholder is replaced by the return type of the function selected
   //  by overload resolution for class template deduction.
   QualType DeducedType =
-      SubstAutoType(TSInfo->getType(), Best->Function->getReturnType());
+      SubstAutoTypeSourceInfo(TSInfo, Best->Function->getReturnType())
+          ->getType();
   Diag(TSInfo->getTypeLoc().getBeginLoc(),
        diag::warn_cxx14_compat_class_template_argument_deduction)
       << TSInfo->getTypeLoc().getSourceRange() << 1 << DeducedType;