[clang] [clang] fix broken canonicalization of DeducedTemplateSpecializationType (PR #95202)

Matheus Izvekov via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 12 00:18:57 PDT 2024


https://github.com/mizvekov created https://github.com/llvm/llvm-project/pull/95202

This reverts the functional elements of commit 3e78fa860235431323aaf08c8fa922d75a7cfffa and redoes it, by fixing the true root cause of #61317.

A TemplateName can be non-canonical; profiling it based on the canonical name would result in inconsistent preservation of as-written information in the AST.

The true problem in #61317 is that we would not consider the methods with requirements expression which contain DTSTs as the same in relation to merging of declarations when importing modules.

The expressions would never match because they contained DTSTs pointing to different redeclarations of the same class template, but since canonicalization for them was broken, their canonical types would not match either.

---

No changelog entry because #61317 was already claimed as fixed in previous release.

>From 71a827e8ff6dcc47f1fd434a19070e6f841ee27a Mon Sep 17 00:00:00 2001
From: Matheus Izvekov <mizvekov at gmail.com>
Date: Wed, 12 Jun 2024 00:42:48 -0300
Subject: [PATCH] [clang] fix broken canonicalization of
 DeducedTemplateSpecializationType

This reverts the functional elements of commit 3e78fa860235431323aaf08c8fa922d75a7cfffa
and redoes it, by fixing the true root cause of #61317.

A TemplateName can be non-canonical; profiling it based on the canonical
name would result in inconsistent preservation of as-written information
in the AST.

The true problem in #61317 is that we would not consider the methods
with requirements expression which contain DTSTs as the same
in relation to merging of declarations when importing modules.

The expressions would never match because they contained DTSTs pointing
to different redeclarations of the same class template, but since
canonicalization for them was broken, their canonical types would not
match either.

---

No changelog entry because #61317 was already claimed as fixed in
previous release.
---
 clang/include/clang/AST/ASTContext.h   |  4 ++++
 clang/include/clang/AST/TemplateName.h |  4 +++-
 clang/include/clang/AST/Type.h         |  9 +++------
 clang/lib/AST/ASTContext.cpp           | 25 +++++++++++++++++++------
 clang/lib/AST/TemplateName.cpp         |  9 ---------
 5 files changed, 29 insertions(+), 22 deletions(-)

diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index a1d1d1c51cd41..5985887000d44 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1770,6 +1770,10 @@ class ASTContext : public RefCountedBase<ASTContext> {
   QualType getDeducedTemplateSpecializationType(TemplateName Template,
                                                 QualType DeducedType,
                                                 bool IsDependent) const;
+  QualType getDeducedTemplateSpecializationTypeInternal(TemplateName Template,
+                                                        QualType DeducedType,
+                                                        bool IsDependent,
+                                                        QualType Canon) const;
 
   /// Return the unique reference to the type for the specified TagDecl
   /// (struct/union/class/enum) decl.
diff --git a/clang/include/clang/AST/TemplateName.h b/clang/include/clang/AST/TemplateName.h
index 988a55acd2252..24a7fde76195d 100644
--- a/clang/include/clang/AST/TemplateName.h
+++ b/clang/include/clang/AST/TemplateName.h
@@ -346,7 +346,9 @@ class TemplateName {
   /// error.
   void dump() const;
 
-  void Profile(llvm::FoldingSetNodeID &ID);
+  void Profile(llvm::FoldingSetNodeID &ID) {
+    ID.AddPointer(Storage.getOpaqueValue());
+  }
 
   /// 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 9eb3f6c09e3d3..f557697da74c6 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -6050,14 +6050,13 @@ class DeducedTemplateSpecializationType : public DeducedType,
 
   DeducedTemplateSpecializationType(TemplateName Template,
                                     QualType DeducedAsType,
-                                    bool IsDeducedAsDependent)
+                                    bool IsDeducedAsDependent, QualType Canon)
       : DeducedType(DeducedTemplateSpecialization, DeducedAsType,
                     toTypeDependence(Template.getDependence()) |
                         (IsDeducedAsDependent
                              ? TypeDependence::DependentInstantiation
                              : TypeDependence::None),
-                    DeducedAsType.isNull() ? QualType(this, 0)
-                                           : DeducedAsType.getCanonicalType()),
+                    Canon),
         Template(Template) {}
 
 public:
@@ -6071,9 +6070,7 @@ class DeducedTemplateSpecializationType : public DeducedType,
   static void Profile(llvm::FoldingSetNodeID &ID, TemplateName Template,
                       QualType Deduced, bool IsDependent) {
     Template.Profile(ID);
-    QualType CanonicalType =
-        Deduced.isNull() ? Deduced : Deduced.getCanonicalType();
-    ID.AddPointer(CanonicalType.getAsOpaquePtr());
+    Deduced.Profile(ID);
     ID.AddBoolean(IsDependent || Template.isDependent());
   }
 
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index bf74e56a14799..34aa399fda2f8 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -5925,11 +5925,9 @@ QualType ASTContext::getUnconstrainedType(QualType T) const {
   return T;
 }
 
-/// 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 ASTContext::getDeducedTemplateSpecializationTypeInternal(
+    TemplateName Template, QualType DeducedType, bool IsDependent,
+    QualType Canon) const {
   // Look in the folding set for an existing type.
   void *InsertPos = nullptr;
   llvm::FoldingSetNodeID ID;
@@ -5940,7 +5938,8 @@ QualType ASTContext::getDeducedTemplateSpecializationType(
     return QualType(DTST, 0);
 
   auto *DTST = new (*this, alignof(DeducedTemplateSpecializationType))
-      DeducedTemplateSpecializationType(Template, DeducedType, IsDependent);
+      DeducedTemplateSpecializationType(Template, DeducedType, IsDependent,
+                                        Canon);
   llvm::FoldingSetNodeID TempID;
   DTST->Profile(TempID);
   assert(ID == TempID && "ID does not match");
@@ -5949,6 +5948,20 @@ QualType ASTContext::getDeducedTemplateSpecializationType(
   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 11544dbb56e31..d4e8a8971a971 100644
--- a/clang/lib/AST/TemplateName.cpp
+++ b/clang/lib/AST/TemplateName.cpp
@@ -264,15 +264,6 @@ 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) {



More information about the cfe-commits mailing list