Skip to content

[clang] ItaniumMangle: fix mangling for unresolved types #135312

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 11, 2025

Conversation

mizvekov
Copy link
Contributor

This fixes a regression introduced here: #132748
which was originally reported here: #132748 (comment)

Some time in the clang-20 time frame, we had removed subst* nodes produced from template type alias instantiation.

This ended up accidentally fixing an issue where the result from an alias template substitution is mistaken for the substitution of an outer non-alias template, incorrectly applying the unresolved-type production to it.

This fixes it by ignoring subst* nodes produced from type aliases. Though builtin templates currently don't place subst* nodes, if we ever decide to place them, this fix should cover them as well.

No release notes since this regression was never released.

This fixes a regression introduced here: #132748
which was originally reported here: #132748 (comment)

Some time in the clang-20 time frame, we had removed subst* nodes produced
from template type alias instantiation.

This ended up accidentally fixing an issue where the result from
an alias template substitution is mistaken for the susbtitution
of an outer non-alias template, incorrectly applying the unresolved-type
production to it.

This fixes it by ignoring subst* nodes produced from type aliases.
Though builtin templates currently don't place subst* nodes, if
we ever decide to place them, this fix should cover them as well.

No release notes since this regression was never released.
@mizvekov mizvekov self-assigned this Apr 11, 2025
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Apr 11, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 11, 2025

@llvm/pr-subscribers-clang

Author: Matheus Izvekov (mizvekov)

Changes

This fixes a regression introduced here: #132748
which was originally reported here: #132748 (comment)

Some time in the clang-20 time frame, we had removed subst* nodes produced from template type alias instantiation.

This ended up accidentally fixing an issue where the result from an alias template substitution is mistaken for the substitution of an outer non-alias template, incorrectly applying the unresolved-type production to it.

This fixes it by ignoring subst* nodes produced from type aliases. Though builtin templates currently don't place subst* nodes, if we ever decide to place them, this fix should cover them as well.

No release notes since this regression was never released.


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

2 Files Affected:

  • (modified) clang/lib/AST/ItaniumMangle.cpp (+10-1)
  • (modified) clang/test/CodeGenCXX/mangle.cpp (+27)
diff --git a/clang/lib/AST/ItaniumMangle.cpp b/clang/lib/AST/ItaniumMangle.cpp
index 8790a5282a96b..c73f040ba1cb7 100644
--- a/clang/lib/AST/ItaniumMangle.cpp
+++ b/clang/lib/AST/ItaniumMangle.cpp
@@ -2511,7 +2511,6 @@ bool CXXNameMangler::mangleUnresolvedTypeOrSimpleId(QualType Ty,
   case Type::PackIndexing:
   case Type::TemplateTypeParm:
   case Type::UnaryTransform:
-  case Type::SubstTemplateTypeParm:
   unresolvedType:
     // Some callers want a prefix before the mangled type.
     Out << Prefix;
@@ -2524,6 +2523,16 @@ bool CXXNameMangler::mangleUnresolvedTypeOrSimpleId(QualType Ty,
     // so we return directly.
     return true;
 
+  case Type::SubstTemplateTypeParm: {
+    auto *ST = cast<SubstTemplateTypeParmType>(Ty);
+    // If this was replaced from a type alias, this is not substituted
+    // from an outer template parameter, so it's not an unresolved-type.
+    if (auto *TD = dyn_cast<TemplateDecl>(ST->getAssociatedDecl());
+        TD && TD->isTypeAlias())
+      return mangleUnresolvedTypeOrSimpleId(ST->getReplacementType(), Prefix);
+    goto unresolvedType;
+  }
+
   case Type::Typedef:
     mangleSourceNameWithAbiTags(cast<TypedefType>(Ty)->getDecl());
     break;
diff --git a/clang/test/CodeGenCXX/mangle.cpp b/clang/test/CodeGenCXX/mangle.cpp
index cf506aff92f0e..f4dc17bc4561e 100644
--- a/clang/test/CodeGenCXX/mangle.cpp
+++ b/clang/test/CodeGenCXX/mangle.cpp
@@ -1220,3 +1220,30 @@ namespace test61 {
   // CHECK-LABEL: @_ZN6test611fINS_1XEEEvNT_1Y1aENS3_1bE
   template void f<X>(int, int);
 }
+
+namespace test62 {
+  template <class> struct integral_constant {
+    static const int value = true;
+  };
+  template <int> struct _OrImpl {};
+  template <class _Args> using _Or = _OrImpl<_Args::value>;
+  template <class _Up>
+  void f(_Or<integral_constant<_Up>>) {}
+  // CHECK-LABEL: @_ZN6test621fIiEEvNS_7_OrImplIXsr17integral_constantIT_EE5valueEEE
+  template void f<int>(_OrImpl<1>);
+} // namespace test62
+
+namespace test63 {
+  namespace {
+    template <class, class> struct integral_constant {
+      static const int value = true;
+    };
+    template <class, class> struct _And {};
+    template <int> struct _OrImpl {};
+    template <class _First> using _Or = _OrImpl<_First::value>;
+    template <class _Up>
+    void f(_And<integral_constant<int, void>, _Or<integral_constant<_Up, int>>>);
+  } // namespace
+  // CHECK-LABEL: @_ZN6test6312_GLOBAL__N_11fIiEEvNS0_4_AndINS0_17integral_constantIivEENS0_7_OrImplIXsr17integral_constantIT_iEE5valueEEEEE
+  void g() { f<int>({}); }
+} // namespace test63

Copy link
Contributor

@zyn0217 zyn0217 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mizvekov mizvekov merged commit 7113aec into main Apr 11, 2025
14 checks passed
@mizvekov mizvekov deleted the users/mizvekov/subst-fix-mangling branch April 11, 2025 14:31
var-const pushed a commit to ldionne/llvm-project that referenced this pull request Apr 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants