Skip to content
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

[clang][ItaniumMangle] Mangle friend function templates with a constr… #110247

Merged
merged 5 commits into from
Sep 30, 2024

Conversation

VitaNuo
Copy link
Contributor

@VitaNuo VitaNuo commented Sep 27, 2024

…aint that depends on a template parameter from an enclosing template as members of the enclosing class.

Such function templates should be considered member-like constrained friends per [temp.friend]p9 and itanium-cxx-abi/cxx-abi#24 (comment)).

…aint that depends on a template parameter from an enclosing template as members of the enclosing class.

Such function templates should be considered member-like constrained friends per [temp.friend]p9 and itanium-cxx-abi/cxx-abi#24 (comment)).
@VitaNuo VitaNuo requested a review from a team as a code owner September 27, 2024 11:36
@llvmbot llvmbot added clang Clang issues not falling into any other category libc++abi libc++abi C++ Runtime Library. Not libc++. clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Sep 27, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 27, 2024

@llvm/pr-subscribers-libcxxabi

@llvm/pr-subscribers-clang

Author: Viktoriia Bakalova (VitaNuo)

Changes

…aint that depends on a template parameter from an enclosing template as members of the enclosing class.

Such function templates should be considered member-like constrained friends per [temp.friend]p9 and itanium-cxx-abi/cxx-abi#24 (comment)).


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

3 Files Affected:

  • (modified) clang/lib/AST/ItaniumMangle.cpp (+8)
  • (modified) clang/test/CodeGenCXX/mangle-concept.cpp (+3-3)
  • (modified) libcxxabi/test/test_demangle.pass.cpp (+3-3)
diff --git a/clang/lib/AST/ItaniumMangle.cpp b/clang/lib/AST/ItaniumMangle.cpp
index b6e1da0c3192da..172561f73b9a74 100644
--- a/clang/lib/AST/ItaniumMangle.cpp
+++ b/clang/lib/AST/ItaniumMangle.cpp
@@ -2270,6 +2270,14 @@ void CXXNameMangler::mangleTemplatePrefix(GlobalDecl GD,
     mangleTemplateParameter(TTP->getDepth(), TTP->getIndex());
   } else {
     const DeclContext *DC = Context.getEffectiveDeclContext(ND);
+    if (const auto *FD = dyn_cast<FunctionTemplateDecl>(GD.getDecl())) {
+      // Member-like constrained friends are mangled as if they were members of
+      // the enclosing class.
+      if (FD->getTemplatedDecl()->isMemberLikeConstrainedFriend() &&
+          getASTContext().getLangOpts().getClangABICompat() >
+              LangOptions::ClangABI::Ver17)
+      DC = GD.getDecl()->getLexicalDeclContext()->getRedeclContext();
+    }
     manglePrefix(DC, NoFunction);
     if (isa<BuiltinTemplateDecl>(ND) || isa<ConceptDecl>(ND))
       mangleUnqualifiedName(GD, DC, nullptr);
diff --git a/clang/test/CodeGenCXX/mangle-concept.cpp b/clang/test/CodeGenCXX/mangle-concept.cpp
index 91dc1b0e688e0d..6053511c00a7b5 100644
--- a/clang/test/CodeGenCXX/mangle-concept.cpp
+++ b/clang/test/CodeGenCXX/mangle-concept.cpp
@@ -58,19 +58,19 @@ namespace test2 {
     // CHECK: call {{.*}}@_ZN5test21AIiEF1fEzQ4TrueIT_E(
     // CLANG17: call {{.*}}@_ZN5test21fEz(
     f(ai);
-    // CHECK: call {{.*}}@_ZN5test2F1gIvEEvzQaa4TrueIT_E4TrueITL0__E(
+    // CHECK: call {{.*}}@_ZN5test21AIiEF1gIvEEvzQaa4TrueIT_E4TrueITL0__E(
     // CLANG17: call {{.*}}@_ZN5test21gIvEEvz(
     g(ai);
     // CHECK: call {{.*}}@_ZN5test21hIvEEvzQ4TrueITL0__E(
     // CLANG17: call {{.*}}@_ZN5test21hIvEEvz(
     h(ai);
-    // CHECK: call {{.*}}@_ZN5test2F1iIvQaa4TrueIT_E4TrueITL0__EEEvz(
+    // CHECK: call {{.*}}@_ZN5test21AIiEF1iIvQaa4TrueIT_E4TrueITL0__EEEvz(
     // CLANG17: call {{.*}}@_ZN5test21iIvEEvz(
     i(ai);
     // CHECK: call {{.*}}@_ZN5test21jIvQ4TrueITL0__EEEvz(
     // CLANG17: call {{.*}}@_ZN5test21jIvEEvz(
     j(ai);
-    // CHECK: call {{.*}}@_ZN5test2F1kITk4TruevQ4TrueIT_EEEvz(
+    // CHECK: call {{.*}}@_ZN5test21AIiEF1kITk4TruevQ4TrueIT_EEEvz(
     // CLANG17: call {{.*}}@_ZN5test21kIvEEvz(
     k(ai);
     // CHECK: call {{.*}}@_ZN5test21lITk4TruevEEvz(
diff --git a/libcxxabi/test/test_demangle.pass.cpp b/libcxxabi/test/test_demangle.pass.cpp
index 17786a3a486fcd..efe482aad1b76c 100644
--- a/libcxxabi/test/test_demangle.pass.cpp
+++ b/libcxxabi/test/test_demangle.pass.cpp
@@ -30128,11 +30128,11 @@ const char* cases[][2] =
     // C++20 concepts, see https://github.com/itanium-cxx-abi/cxx-abi/issues/24.
     {"_Z2f0IiE1SIX1CIT_EEEv", "S<C<int>> f0<int>()"},
     {"_ZN5test21AIiEF1fEzQ4TrueIT_E", "test2::A<int>::friend f(...) requires True<T>"},
-    {"_ZN5test2F1gIvEEvzQaa4TrueIT_E4TrueITL0__E", "void test2::friend g<void>(...) requires True<T> && True<TL0_>"},
+    {"_ZN5test21AIiEF1gIvEEvzQaa4TrueIT_E4TrueITL0__E", "void test2::A<int>::friend g<void>(...) requires True<T> && True<TL0_>"},
     {"_ZN5test21hIvEEvzQ4TrueITL0__E", "void test2::h<void>(...) requires True<TL0_>"},
-    {"_ZN5test2F1iIvQaa4TrueIT_E4TrueITL0__EEEvz", "void test2::friend i<void>(...)"},
+    {"_ZN5test21AIiEF1iIvQaa4TrueIT_E4TrueITL0__EEEvz", "void test2::A<int>::friend i<void>(...)"},
     {"_ZN5test21jIvQ4TrueITL0__EEEvz", "void test2::j<void>(...)"},
-    {"_ZN5test2F1kITk4TruevQ4TrueIT_EEEvz", "void test2::friend k<void>(...)"},
+    {"_ZN5test21AIiEF1kITk4TruevQ4TrueIT_EEEvz", "void test2::A<int>::friend k<void>(...)"},
     {"_ZN5test21lITk4TruevEEvz", "void test2::l<void>(...)"},
     {"_ZN5test31dITnDaLi0EEEvv", "void test3::d<0>()"},
     {"_ZN5test31eITnDcLi0EEEvv", "void test3::e<0>()"},

Copy link

github-actions bot commented Sep 27, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@VitaNuo VitaNuo linked an issue Sep 27, 2024 that may be closed by this pull request
… constraint that depends on a template parameter from an enclosing template as members of the enclosing class.
Copy link
Contributor

@usx95 usx95 left a comment

Choose a reason for hiding this comment

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

Thanks for catching and fixing this.

Please also add release notes in clang/docs/ReleaseNotes.rst

clang/lib/AST/ItaniumMangle.cpp Outdated Show resolved Hide resolved
clang/test/CodeGenCXX/mangle-concept.cpp Show resolved Hide resolved
… with a constraint that depends on a template parameter from an enclosing template as members of the enclosing class.
… with a constraint that depends on a template parameter from an enclosing template as members of the enclosing class.
Copy link
Contributor

@usx95 usx95 left a comment

Choose a reason for hiding this comment

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

Thanks. LGTM.

Left a comment to simplify the change.

@@ -704,6 +704,15 @@ ItaniumMangleContextImpl::getEffectiveDeclContext(const Decl *D) {
return D->getLexicalDeclContext()->getRedeclContext();
}

if (const auto *FTD = dyn_cast<FunctionTemplateDecl>(D)) {
// Member-like constrained friends are mangled as if they were members of
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's reuse the function decl above.

We could use the Decl::getAsFunction() to do both of these checks for us.

if (const auto *FD = D->getAsFunction())

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great idea!

…mplates with a constraint that depends on a template parameter from an enclosing template as members of the enclosing class.
@VitaNuo VitaNuo merged commit 147558e into llvm:main Sep 30, 2024
10 of 13 checks passed
@zygoloid
Copy link
Collaborator

Should this have -fclang-abi-compat support?

@hokein
Copy link
Collaborator

hokein commented Sep 30, 2024

Should this have -fclang-abi-compat support?

(It seems like we should, although I don’t have direct experience with it.)

This is a bug fix for 4b163e3, which is released in Clang 18 and Clang 19. Therefore, we should likely add -fclang-abi-compat for Ver19?

@llvm llvm deleted a comment from llvm-ci Sep 30, 2024
@llvm llvm deleted a comment from llvm-ci Sep 30, 2024
@llvm llvm deleted a comment from llvm-ci Sep 30, 2024
@llvm llvm deleted a comment from llvm-ci Sep 30, 2024
@llvm llvm deleted a comment from llvm-ci Sep 30, 2024
@llvm llvm deleted a comment from llvm-ci Sep 30, 2024
@llvm llvm deleted a comment from llvm-ci Sep 30, 2024
@llvm llvm deleted a comment from llvm-ci Sep 30, 2024
@llvm llvm deleted a comment from llvm-ci Sep 30, 2024
@llvm llvm deleted a comment from llvm-ci Sep 30, 2024
@llvm llvm deleted a comment from llvm-ci Sep 30, 2024
@llvm llvm deleted a comment from llvm-ci Sep 30, 2024
@llvm llvm deleted a comment from llvm-ci Sep 30, 2024
@llvm llvm deleted a comment from llvm-ci Sep 30, 2024
@llvm llvm deleted a comment from llvm-ci Sep 30, 2024
@llvm llvm deleted a comment from llvm-ci Sep 30, 2024
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 libc++abi libc++abi C++ Runtime Library. Not libc++.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[abi] Declaring class of a member-like friend method is not mangled.
5 participants