[clang] fix uniquing of some TagTypes created from the injected class name #155347

mizvekov · 2025-08-26T02:54:22Z

This fixes a bug in the fast path for the creation of TagTypes from injected class names.

The creation of TagTypes has a fast path which, when there is no elaboration, uses storage in the declaration itself for memoizing the resuling type node, instead of using the folding set.

This memoizing would fail when the type was created from the injected class name, as we would look for the node in the injected declaration but store it in the non-injected one, so a different type would be created each time.

This regression was reported here: #147835 (comment)

Since this regression was never released, there are no release notes.

… name This fixes a bug in the fast path for the creation of TagTypes from injected class names. The creation of TagTypes has a fast path which, when there is no elaboration, uses storage in the declaration itself for memoizing the resuling type node, instead of using the folding set. This memoizing would fail when the type was created from the injected class name, as we would look for the node in the injected declaration but store it in the non-injected one, so a different type would be created each time. This regression was reported here: #147835 (comment) Since this regression was never released, there are no release notes.

llvmbot · 2025-08-26T02:54:54Z

@llvm/pr-subscribers-clang

Author: Matheus Izvekov (mizvekov)

Changes

This fixes a bug in the fast path for the creation of TagTypes from injected class names.

The creation of TagTypes has a fast path which, when there is no elaboration, uses storage in the declaration itself for memoizing the resuling type node, instead of using the folding set.

This memoizing would fail when the type was created from the injected class name, as we would look for the node in the injected declaration but store it in the non-injected one, so a different type would be created each time.

This regression was reported here: #147835 (comment)

Since this regression was never released, there are no release notes.

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

2 Files Affected:

(modified) clang/lib/AST/ASTContext.cpp (+20-21)
(modified) clang/test/AST/ast-dump-decl.cpp (+16-1)

diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 036df53063568..edd512fa78e46 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -5474,18 +5474,15 @@ TagType *ASTContext::getTagTypeInternal(ElaboratedTypeKeyword Keyword,
   return T;
 }
 
-static bool getNonInjectedClassName(const TagDecl *&TD) {
+static const TagDecl *getNonInjectedClassName(const TagDecl *TD) {
   if (const auto *RD = dyn_cast<CXXRecordDecl>(TD);
-      RD && RD->isInjectedClassName()) {
-    TD = cast<TagDecl>(RD->getDeclContext());
-    return true;
-  }
-  return false;
+      RD && RD->isInjectedClassName())
+    return cast<TagDecl>(RD->getDeclContext());
+  return TD;
 }
 
 CanQualType ASTContext::getCanonicalTagType(const TagDecl *TD) const {
-  ::getNonInjectedClassName(TD);
-  TD = TD->getCanonicalDecl();
+  TD = ::getNonInjectedClassName(TD)->getCanonicalDecl();
   if (TD->TypeForDecl)
     return TD->TypeForDecl->getCanonicalTypeUnqualified();
 
@@ -5501,40 +5498,42 @@ CanQualType ASTContext::getCanonicalTagType(const TagDecl *TD) const {
 QualType ASTContext::getTagType(ElaboratedTypeKeyword Keyword,
                                 NestedNameSpecifier Qualifier,
                                 const TagDecl *TD, bool OwnsTag) const {
+
+  const TagDecl *NonInjectedTD = ::getNonInjectedClassName(TD);
+  bool IsInjected = TD != NonInjectedTD;
+
   ElaboratedTypeKeyword PreferredKeyword =
-      getLangOpts().CPlusPlus
-          ? ElaboratedTypeKeyword::None
-          : KeywordHelpers::getKeywordForTagTypeKind(TD->getTagKind());
+      getLangOpts().CPlusPlus ? ElaboratedTypeKeyword::None
+                              : KeywordHelpers::getKeywordForTagTypeKind(
+                                    NonInjectedTD->getTagKind());
 
   if (Keyword == PreferredKeyword && !Qualifier && !OwnsTag) {
     if (const Type *T = TD->TypeForDecl; T && !T->isCanonicalUnqualified())
       return QualType(T, 0);
 
-    bool IsInjected = ::getNonInjectedClassName(TD);
-    const Type *CanonicalType = getCanonicalTagType(TD).getTypePtr();
+    const Type *CanonicalType = getCanonicalTagType(NonInjectedTD).getTypePtr();
     const Type *T =
         getTagTypeInternal(Keyword,
-                           /*Qualifier=*/std::nullopt, TD,
+                           /*Qualifier=*/std::nullopt, NonInjectedTD,
                            /*OwnsTag=*/false, IsInjected, CanonicalType,
                            /*WithFoldingSetNode=*/false);
     TD->TypeForDecl = T;
     return QualType(T, 0);
   }
 
-  bool IsInjected = ::getNonInjectedClassName(TD);
-
   llvm::FoldingSetNodeID ID;
-  TagTypeFoldingSetPlaceholder::Profile(ID, Keyword, Qualifier, TD, OwnsTag,
-                                        IsInjected);
+  TagTypeFoldingSetPlaceholder::Profile(ID, Keyword, Qualifier, NonInjectedTD,
+                                        OwnsTag, IsInjected);
 
   void *InsertPos = nullptr;
   if (TagTypeFoldingSetPlaceholder *T =
           TagTypes.FindNodeOrInsertPos(ID, InsertPos))
     return QualType(T->getTagType(), 0);
 
-  const Type *CanonicalType = getCanonicalTagType(TD).getTypePtr();
-  TagType *T = getTagTypeInternal(Keyword, Qualifier, TD, OwnsTag, IsInjected,
-                                  CanonicalType, /*WithFoldingSetNode=*/true);
+  const Type *CanonicalType = getCanonicalTagType(NonInjectedTD).getTypePtr();
+  TagType *T =
+      getTagTypeInternal(Keyword, Qualifier, NonInjectedTD, OwnsTag, IsInjected,
+                         CanonicalType, /*WithFoldingSetNode=*/true);
   TagTypes.InsertNode(TagTypeFoldingSetPlaceholder::fromTagType(T), InsertPos);
   return QualType(T, 0);
 }
diff --git a/clang/test/AST/ast-dump-decl.cpp b/clang/test/AST/ast-dump-decl.cpp
index dca0e9b9ca10a..6cada93c8b93b 100644
--- a/clang/test/AST/ast-dump-decl.cpp
+++ b/clang/test/AST/ast-dump-decl.cpp
@@ -973,5 +973,20 @@ namespace TestConstexprVariableTemplateWithInitializer {
   // CHECK-NEXT: `-VarDecl 0x{{.+}} <col:25, col:48> col:37 call_init 'const T' constexpr callinit{{$}}
   // CHECK-NEXT:  `-ParenListExpr 0x{{.+}} <col:46, col:48> 'NULL TYPE'{{$}}
   // CHECK-NEXT:   `-IntegerLiteral 0x{{.+}} <col:47> 'int' 0{{$}}
-
 }
+
+namespace TestInjectedClassName {
+  struct A {
+    using T1 = A;
+    using T2 = A;
+  };
+  // CHECK-LABEL: Dumping TestInjectedClassName:
+  // CHECK:       CXXRecordDecl [[TestInjectedClassName_RD:0x[^ ]+]] {{.*}} struct A definition
+  // CHECK:       CXXRecordDecl {{.*}} implicit referenced struct A
+  // CHECK-NEXT:  |-TypeAliasDecl {{.*}} T1 'A'
+  // CHECK-NEXT:  | `-RecordType [[TestInjectedClassName_RT:0x[^ ]+]] 'A' injected
+  // CHECK-NEXT:  |   `-CXXRecord [[TestInjectedClassName_RD]] 'A'
+  // CHECK-NEXT:  `-TypeAliasDecl {{.*}} T2 'A'
+  // CHECK-NEXT:    `-RecordType [[TestInjectedClassName_RT]] 'A' injected
+  // CHECK-NEXT:      `-CXXRecord [[TestInjectedClassName_RD]] 'A'
+} // namespace InjectedClassName

pranavk · 2025-08-28T18:23:29Z

This commit breaks some of our internal tests that were working fine with the original #147835

I don't have the repro yet and I am not sure of an easy way to extract it from internal code. Pasting logs that I have right now in case you could figure anything out from this:

RAW: AST crash frame 0 in TraverseConstructorInitializer under node:
<CXXCtorInitializer>
RAW: AST crash frame 1 in TraverseFunctionDeclHelper under node:
CXXConstructorDecl 0x12413d9e08f8 <./base/foobar.h:33:3, col:36> col:13 used constexpr foobar 'void () noexcept' default implicit-inline
|-CXXCtorInitializer Field 0x12413d420ac8 'entry_' 'std::atomic<EntryBase *>':'std::atomic<foobar::EntryBase *>'
| `-CXXDefaultInitExpr 0x1241383b40b8 <col:13> 'std::atomic<EntryBase *>':'std::atomic<foobar::EntryBase *>' has rewritten init
|   `-CXXConstructExpr 0x12413d421fd0 <line:180:34, col:44> 'std::atomic<EntryBase *>':'std::atomic<foobar::EntryBase *>' 'void (foobar::EntryBase *) noexcept' list
|     `-ImplicitCastExpr 0x12413d4215d0 <col:37> 'foobar::EntryBase *' <NullToPointer>
|       `-CXXNullPtrLiteralExpr 0x12413d421578 <col:37> 'std::nullptr_t'
|-CompoundStmt 0x1241383b4108 <line:33:36>
`-FullComment 0x1241145e4600 <line:26:5, line:31:44>
  `-ParagraphComment 0x1241145e45d0 <line:26:5, line:31:44>
    |-TextComment 0x1241145e4480 <line:26:5, col:75> Text=" xxx "
    |-TextComment 0x1241145e44b0 <line:27:5, col:78> Text=" xxx "
    |-TextComment 0x1241145e44e0 <line:28:5, col:79> Text=" xxx "
    |-TextComment 0x1241145e4510 <line:29:5, col:79> Text=" xxx "
    |-TextComment 0x1241145e4540 <line:30:5, col:74> Text=" xxx "
    `-TextComment 0x1241145e4570 <line:31:5, col:44> Text=" xxx ()`."
RAW: AST crash frame 2 in TraverseCXXConstructorDecl under the above node
RAW: AST crash frame 3 in BuildFunctionImplTree under the above node
RAW: Raising signal 11 with default behavior

mizvekov · 2025-08-28T19:17:40Z

I don't think that helps, however the patch is a bit simple, so you could maybe sort of reverse engineer what could have gone wrong from it.

For example, before this patch we would have produced non-identical CXXRecordTypes from basically the same information.
There is probably somewhere you are comparing non-canonical RecordTypes and now after this patch they are equal.

mizvekov self-assigned this Aug 26, 2025

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Aug 26, 2025

mizvekov mentioned this pull request Aug 26, 2025

[clang] Improve nested name specifier AST representation #147835

Merged

mizvekov requested a review from zyn0217 August 26, 2025 03:12

mizvekov merged commit 0a675f5 into main Aug 26, 2025
12 checks passed

mizvekov deleted the users/mizvekov/GH147835-regression-4 branch August 26, 2025 03:26

zwuis added the skip-precommit-approval PR for CI feedback, not intended for review label Aug 26, 2025

bradking mentioned this pull request Aug 28, 2025

Port to LLVM/Clang Git main as of 2025-08-26 CastXML/CastXML#287

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[clang] fix uniquing of some TagTypes created from the injected class name #155347

[clang] fix uniquing of some TagTypes created from the injected class name #155347

Uh oh!

mizvekov commented Aug 26, 2025 •

edited

Loading

Uh oh!

llvmbot commented Aug 26, 2025

Uh oh!

Uh oh!

pranavk commented Aug 28, 2025

Uh oh!

mizvekov commented Aug 28, 2025

Uh oh!

Uh oh!

[clang] fix uniquing of some TagTypes created from the injected class name #155347

[clang] fix uniquing of some TagTypes created from the injected class name #155347

Uh oh!

Conversation

mizvekov commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Aug 26, 2025

Uh oh!

Uh oh!

pranavk commented Aug 28, 2025

Uh oh!

mizvekov commented Aug 28, 2025

Uh oh!

Uh oh!

mizvekov commented Aug 26, 2025 •

edited

Loading