release/19.x: Revert "[clang] fix broken canonicalization of DeducedTemplateSpeciatizationType (#95202)"

[cor3ntin]

This reverts commit 2e1ad93.

Reverting #95202 in the 19.x branch

Fixes #106182

The change in #95202 causes code to crash and there is no good way to backport a fix for that as there are ABI-impacting changes at play.
Instead we revert #95202 in the 19x branch, fixing the regression and preserving the 18.x behavior (which is GCC's behavior)

#106335 (comment)

[llvmbot]

@llvm/pr-subscribers-clang

Author: cor3ntin (cor3ntin)

Changes

This reverts commit 2e1ad93.

Reverting #95202 in the 19.x branch

Fixes #106182

The change in #95202 causes code to crash and there is no good way to backport a fix for that as there are ABI-impacting changes at play.
Instead we revert #95202 in the 19x branch, fixing the regression and preserving the 18.x behavior (which is GCC's behavior)

#106335 (comment)

---

Full diff: https://github.com/llvm/llvm-project/pull/106516.diff

7 Files Affected:

• (modified) clang/include/clang/AST/ASTContext.h (-7)
• (modified) clang/include/clang/AST/TemplateName.h (+1-3)
• (modified) clang/include/clang/AST/Type.h (+7-4)
• (modified) clang/lib/AST/ASTContext.cpp (+6-19)
• (modified) clang/lib/AST/TemplateName.cpp (+9)
• (modified) clang/unittests/AST/CMakeLists.txt (-1)
• (removed) clang/unittests/AST/ProfilingTest.cpp (-75)

diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 6d1c8ca8a2f961..16a19645d7f367 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1805,13 +1805,6 @@ class ASTContext : public RefCountedBase<ASTContext> {
                                                 QualType DeducedType,
                                                 bool IsDependent) const;
 
-private:
-  QualType getDeducedTemplateSpecializationTypeInternal(TemplateName Template,
-                                                        QualType DeducedType,
-                                                        bool IsDependent,
-                                                        QualType Canon) const;
-
-public:
   /// Return the unique reference to the type for the specified TagDecl
   /// (struct/union/class/enum) decl.
   QualType getTagDeclType(const TagDecl *Decl) const;
diff --git a/clang/include/clang/AST/TemplateName.h b/clang/include/clang/AST/TemplateName.h
index e3b7dd261535d6..e7313dee012813 100644
--- a/clang/include/clang/AST/TemplateName.h
+++ b/clang/include/clang/AST/TemplateName.h
@@ -347,9 +347,7 @@ class TemplateName {
   /// error.
   void dump() const;
 
-  void Profile(llvm::FoldingSetNodeID &ID) {
-    ID.AddPointer(Storage.getOpaqueValue());
-  }
+  void Profile(llvm::FoldingSetNodeID &ID);
 
   /// Retrieve the template name as a void pointer.
   void *getAsVoidPointer() const { return Storage.getOpaqueValue(); }
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 25defea58c2dc2..9a711030cff9ca 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -6421,27 +6421,30 @@ class DeducedTemplateSpecializationType : public DeducedType,
 
   DeducedTemplateSpecializationType(TemplateName Template,
                                     QualType DeducedAsType,
-                                    bool IsDeducedAsDependent, QualType Canon)
+                                    bool IsDeducedAsDependent)
       : DeducedType(DeducedTemplateSpecialization, DeducedAsType,
                     toTypeDependence(Template.getDependence()) |
                         (IsDeducedAsDependent
                              ? TypeDependence::DependentInstantiation
                              : TypeDependence::None),
-                    Canon),
+                    DeducedAsType.isNull() ? QualType(this, 0)
+                                           : DeducedAsType.getCanonicalType()),
         Template(Template) {}
 
 public:
   /// Retrieve the name of the template that we are deducing.
   TemplateName getTemplateName() const { return Template;}
 
-  void Profile(llvm::FoldingSetNodeID &ID) const {
+  void Profile(llvm::FoldingSetNodeID &ID) {
     Profile(ID, getTemplateName(), getDeducedType(), isDependentType());
   }
 
   static void Profile(llvm::FoldingSetNodeID &ID, TemplateName Template,
                       QualType Deduced, bool IsDependent) {
     Template.Profile(ID);
-    Deduced.Profile(ID);
+    QualType CanonicalType =
+        Deduced.isNull() ? Deduced : Deduced.getCanonicalType();
+    ID.AddPointer(CanonicalType.getAsOpaquePtr());
     ID.AddBoolean(IsDependent || Template.isDependent());
   }
 
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 3da5e888f25175..1064507f34616a 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -6269,9 +6269,11 @@ QualType ASTContext::getUnconstrainedType(QualType T) const {
   return T;
 }
 
-QualType ASTContext::getDeducedTemplateSpecializationTypeInternal(
-    TemplateName Template, QualType DeducedType, bool IsDependent,
-    QualType Canon) const {
+/// Return the uniqued reference to the deduced template specialization type
+/// which has been deduced to the given type, or to the canonical undeduced
+/// such type, or the canonical deduced-but-dependent such type.
+QualType ASTContext::getDeducedTemplateSpecializationType(
+    TemplateName Template, QualType DeducedType, bool IsDependent) const {
   // Look in the folding set for an existing type.
   void *InsertPos = nullptr;
   llvm::FoldingSetNodeID ID;
@@ -6282,8 +6284,7 @@ QualType ASTContext::getDeducedTemplateSpecializationTypeInternal(
     return QualType(DTST, 0);
 
   auto *DTST = new (*this, alignof(DeducedTemplateSpecializationType))
-      DeducedTemplateSpecializationType(Template, DeducedType, IsDependent,
-                                        Canon);
+      DeducedTemplateSpecializationType(Template, DeducedType, IsDependent);
   llvm::FoldingSetNodeID TempID;
   DTST->Profile(TempID);
   assert(ID == TempID && "ID does not match");
@@ -6292,20 +6293,6 @@ QualType ASTContext::getDeducedTemplateSpecializationTypeInternal(
   return QualType(DTST, 0);
 }
 
-/// Return the uniqued reference to the deduced template specialization type
-/// which has been deduced to the given type, or to the canonical undeduced
-/// such type, or the canonical deduced-but-dependent such type.
-QualType ASTContext::getDeducedTemplateSpecializationType(
-    TemplateName Template, QualType DeducedType, bool IsDependent) const {
-  QualType Canon = DeducedType.isNull()
-                       ? getDeducedTemplateSpecializationTypeInternal(
-                             getCanonicalTemplateName(Template), QualType(),
-                             IsDependent, QualType())
-                       : DeducedType.getCanonicalType();
-  return getDeducedTemplateSpecializationTypeInternal(Template, DeducedType,
-                                                      IsDependent, Canon);
-}
-
 /// getAtomicType - Return the uniqued reference to the atomic type for
 /// the given value type.
 QualType ASTContext::getAtomicType(QualType T) const {
diff --git a/clang/lib/AST/TemplateName.cpp b/clang/lib/AST/TemplateName.cpp
index d4e8a8971a971a..11544dbb56e31d 100644
--- a/clang/lib/AST/TemplateName.cpp
+++ b/clang/lib/AST/TemplateName.cpp
@@ -264,6 +264,15 @@ bool TemplateName::containsUnexpandedParameterPack() const {
   return getDependence() & TemplateNameDependence::UnexpandedPack;
 }
 
+void TemplateName::Profile(llvm::FoldingSetNodeID &ID) {
+  if (const auto* USD = getAsUsingShadowDecl())
+    ID.AddPointer(USD->getCanonicalDecl());
+  else if (const auto *TD = getAsTemplateDecl())
+    ID.AddPointer(TD->getCanonicalDecl());
+  else
+    ID.AddPointer(Storage.getOpaqueValue());
+}
+
 void TemplateName::print(raw_ostream &OS, const PrintingPolicy &Policy,
                          Qualified Qual) const {
   auto handleAnonymousTTP = [](TemplateDecl *TD, raw_ostream &OS) {
diff --git a/clang/unittests/AST/CMakeLists.txt b/clang/unittests/AST/CMakeLists.txt
index dcc9bc0f39ac2c..29d2b39cff8b15 100644
--- a/clang/unittests/AST/CMakeLists.txt
+++ b/clang/unittests/AST/CMakeLists.txt
@@ -31,7 +31,6 @@ add_clang_unittest(ASTTests
   EvaluateAsRValueTest.cpp
   ExternalASTSourceTest.cpp
   NamedDeclPrinterTest.cpp
-  ProfilingTest.cpp
   RandstructTest.cpp
   RecursiveASTVisitorTest.cpp
   SizelessTypesTest.cpp
diff --git a/clang/unittests/AST/ProfilingTest.cpp b/clang/unittests/AST/ProfilingTest.cpp
deleted file mode 100644
index 27a4a197f1cbfa..00000000000000
--- a/clang/unittests/AST/ProfilingTest.cpp
+++ /dev/null
@@ -1,75 +0,0 @@
-//===- unittests/AST/ProfilingTest.cpp --- Tests for Profiling ------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "clang/AST/ASTContext.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "clang/ASTMatchers/ASTMatchers.h"
-#include "clang/Tooling/Tooling.h"
-#include "gtest/gtest.h"
-#include <utility>
-
-namespace clang {
-namespace {
-using namespace ast_matchers;
-
-static auto getClassTemplateRedecls() {
-  std::string Code = R"cpp(
-    template <class> struct A;
-    template <class> struct A;
-    template <class> struct A;
-  )cpp";
-  auto AST = tooling::buildASTFromCode(Code);
-  ASTContext &Ctx = AST->getASTContext();
-
-  auto MatchResults = match(classTemplateDecl().bind("id"), Ctx);
-  SmallVector<ClassTemplateDecl *, 3> Res;
-  for (BoundNodes &N : MatchResults) {
-    if (auto *CTD = const_cast<ClassTemplateDecl *>(
-            N.getNodeAs<ClassTemplateDecl>("id")))
-      Res.push_back(CTD);
-  }
-  assert(Res.size() == 3);
-#ifndef NDEBUG
-  for (auto &&I : Res)
-    assert(I->getCanonicalDecl() == Res[0]);
-#endif
-  return std::make_tuple(std::move(AST), Res[1], Res[2]);
-}
-
-template <class T> static void testTypeNode(const T *T1, const T *T2) {
-  {
-    llvm::FoldingSetNodeID ID1, ID2;
-    T1->Profile(ID1);
-    T2->Profile(ID2);
-    ASSERT_NE(ID1, ID2);
-  }
-  auto *CT1 = cast<T>(T1->getCanonicalTypeInternal());
-  auto *CT2 = cast<T>(T2->getCanonicalTypeInternal());
-  {
-    llvm::FoldingSetNodeID ID1, ID2;
-    CT1->Profile(ID1);
-    CT2->Profile(ID2);
-    ASSERT_EQ(ID1, ID2);
-  }
-}
-
-TEST(Profiling, DeducedTemplateSpecializationType_Name) {
-  auto [AST, CTD1, CTD2] = getClassTemplateRedecls();
-  ASTContext &Ctx = AST->getASTContext();
-
-  auto *T1 = cast<DeducedTemplateSpecializationType>(
-      Ctx.getDeducedTemplateSpecializationType(TemplateName(CTD1), QualType(),
-                                               false));
-  auto *T2 = cast<DeducedTemplateSpecializationType>(
-      Ctx.getDeducedTemplateSpecializationType(TemplateName(CTD2), QualType(),
-                                               false));
-  testTypeNode(T1, T2);
-}
-
-} // namespace
-} // namespace clang

[AaronBallman]

LGTM! Do we have a release note we need to remove as well?

[cor3ntin]

we did not add one (because there was already a PR that partially but not fully addressed this bug)

[github-actions]

@cor3ntin (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

[cor3ntin]

no need for a release note here