This is an archive of the discontinued LLVM Phabricator instance.

Implement mangling rules for C++20 concepts and requires-expressions.
ClosedPublic

Authored by rsmith on Apr 5 2023, 12:33 PM.

Details

Summary

This implements proposals from:

This changes the manglings for a few cases:

  • Functions and function templates with constraints.
  • Function templates with template parameters with deduced types: typename<auto N> void f();
  • Function templates with template template parameters where the argument has a different template-head: template<template<typename...T>> void f(); f<std::vector>();

In each case where a mangling changed, the change fixes a mangling collision.

Note that only function templates are affected, not class templates or variable
templates, and only new constructs (template parameters with deduced types,
constrained templates) and esoteric constructs (templates with template
template parameters with non-matching template template arguments, most of
which Clang still does not accept by default due to
-frelaxed-template-template-args not being enabled by default), so the risk
to ABI stability from this change is relatively low. Nonetheless,
-fclang-abi-compat=17 can be used to restore the old manglings for cases
which we could successfully but incorrectly mangle before.

Fixes #48216, #49884, #61273

Diff Detail

Event Timeline

rsmith created this revision.Apr 5 2023, 12:33 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 5 2023, 12:33 PM
rsmith requested review of this revision.Apr 5 2023, 12:33 PM
Herald added projects: Restricted Project, Restricted Project, Restricted Project. · View Herald TranscriptApr 5 2023, 12:33 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript

Here's the diff to test_demangle.pass.cpp:

--- a/libcxxabi/test/test_demangle.pass.cpp
+++ b/libcxxabi/test/test_demangle.pass.cpp
@@ -29995,10 +29995,10 @@ const char* cases[][2] =
     {"_Z15test_uuidofTypeI10TestStructEvDTu8__uuidofT_EE", "void test_uuidofType<TestStruct>(decltype(__uuidof(TestStruct)))"},
     {"_Z15test_uuidofExprI9HasMemberEvDTu8__uuidofXsrT_6memberEEE", "void test_uuidofExpr<HasMember>(decltype(__uuidof(HasMember::member)))"},
 
-    // C++2a char8_t:
+    // C++20 char8_t:
     {"_ZTSPDu", "typeinfo name for char8_t*"},
 
-    // C++2a lambda-expressions:
+    // C++20 lambda-expressions:
     {"_ZNK1xMUlTyT_E_clIiEEDaS_", "auto x::'lambda'<typename $T>($T)::operator()<int>(x) const"},
     {"_ZNK1xMUlTnPA3_ivE_clILS0_0EEEDav", "auto x::'lambda'<int (*$N) [3]>()::operator()<(int [3])0>() const"},
     {"_ZNK1xMUlTyTtTyTnT_TpTnPA3_TL0__ETpTyvE_clIi1XJfEEEDav", "auto x::'lambda'<typename $T, template<typename $T0, $T $N, $T0 (*...$N0) [3]> typename $TT, typename ...$T1>()::operator()<int, X, float>() const"},
@@ -30015,8 +30015,10 @@ const char* cases[][2] =
     // See https://github.com/itanium-cxx-abi/cxx-abi/issues/106.
     {"_ZN1XIZ1fIiEvOT_EUlS2_DpT0_E_EclIJEEEvDpT_", "void X<void f<int>(int&&)::'lambda'(int&&, auto...)>::operator()<>()"},
     {"_ZZZZN6abcdef9abcdefghi29abcdefabcdefabcdefabcefabcdef27xxxxxxxxxxxxxxxxxxxxxxxxxxxEN4absl8DurationERKNSt3__u12basic_stringIcNS4_11char_traitsIcEENS4_9allocatorIcEEEEPNS1_19yyyyyyyyyyyyyyyyyyyEENK3$_5clEvENKUlvE_clEvE6zzzzzz", "abcdef::abcdefghi::abcdefabcdefabcdefabcefabcdef::xxxxxxxxxxxxxxxxxxxxxxxxxxx(absl::Duration, std::__u::basic_string<char, std::__u::char_traits<char>, std::__u::allocator<char>> const&, abcdef::abcdefghi::abcdefabcdefabcdefabcefabcdef::yyyyyyyyyyyyyyyyyyy*)::$_5::operator()() const::'lambda'()::operator()() const::zzzzzz"},
+    // See https://github.com/itanium-cxx-abi/cxx-abi/issues/165.
+    {"_ZN1C1fIiEEvDTtlNS_UlT_TL0__E_EEE", "void C::f<int>(decltype(C::'lambda'(int, auto){}))"},
 
-    // C++2a class type non-type template parameters:
+    // C++20 class type non-type template parameters:
     {"_Z1fIXtl1BLPi0ELi1EEEEvv", "void f<B{(int*)0, 1}>()"},
     {"_Z1fIXtl1BLPi32EEEEvv", "void f<B{(int*)32}>()"},
     {"_Z1fIXtl1BrcPiLi0EEEEvv", "void f<B{reinterpret_cast<int*>(0)}>()"},
@@ -30089,6 +30091,51 @@ const char* cases[][2] =
     {"_ZW1ML4Oink", "Oink@M"},
     {"_ZW1ML1fi", "f@M(int)"},
 
+    // 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_>"},
+    {"_ZN5test21hIvEEvzQ4TrueITL0__E", "void test2::h<void>(...) requires True<TL0_>"},
+    {"_ZN5test2F1iIvQaa4TrueIT_E4TrueITL0__EEEvz", "void test2::friend i<void>(...)"},
+    {"_ZN5test21jIvQ4TrueITL0__EEEvz", "void test2::j<void>(...)"},
+    {"_ZN5test2F1kITk4TruevQ4TrueIT_EEEvz", "void test2::friend k<void>(...)"},
+    {"_ZN5test21lITk4TruevEEvz", "void test2::l<void>(...)"},
+    {"_ZN5test31dITnDaLi0EEEvv", "void test3::d<0>()"},
+    {"_ZN5test31eITnDcLi0EEEvv", "void test3::e<0>()"},
+    {"_ZN5test31fITnDk1CLi0EEEvv", "void test3::f<0>()"},
+    {"_ZN5test31gITnDk1DIiELi0EEEvv", "void test3::g<0>()"},
+    {"_ZN5test31hIiTnDk1DIT_ELi0EEEvv", "void test3::h<int, 0>()"},
+    {"_ZN5test31iIiEEvDTnw_Dk1CpicvT__EEE", "void test3::i<int>(decltype(new C auto((int)())))"},
+    {"_ZN5test31jIiEEvDTnw_DK1CpicvT__EEE", "void test3::j<int>(decltype(new C decltype(auto)((int)())))"},
+    {"_ZN5test41fITk1CiEEvv", "void test4::f<int>()"},
+    {"_ZN5test41gITk1DIiEiEEvv", "void test4::g<int>()"},
+    {"_ZN5test51fINS_1XEEEvv", "void test5::f<test5::X>()"},
+    {"_ZN5test51fITtTyTnTL0__ENS_1YEEEvv", "void test5::f<test5::Y>()"},
+    {"_ZN5test51fITtTyTnTL0__ENS_1ZEEEvv", "void test5::f<test5::Z>()"},
+    {"_ZN5test51gITtTyTnTL0__Q1CIS1_EENS_1XEEEvv", "void test5::g<test5::X>()"},
+    {"_ZN5test51gINS_1YEEEvv", "void test5::g<test5::Y>()"},
+    {"_ZN5test51gITtTyTnTL0__Q1CIS1_EENS_1ZEEEvv", "void test5::g<test5::Z>()"},
+    {"_ZN5test51hITtTk1CTnTL0__ENS_1XEEEvv", "void test5::h<test5::X>()"},
+    {"_ZN5test51hITtTk1CTnTL0__ENS_1YEEEvv", "void test5::h<test5::Y>()"},
+    {"_ZN5test51hINS_1ZEEEvv", "void test5::h<test5::Z>()"},
+    {"_ZN5test51iITpTtTk1CTnTL0__EJNS_1XENS_1YENS_1ZEEEEvv", "void test5::i<test5::X, test5::Y, test5::Z>()"},
+    {"_ZN5test51iITpTtTk1CTnTL0__EJNS_1YENS_1ZENS_1XEEEEvv", "void test5::i<test5::Y, test5::Z, test5::X>()"},
+    {"_ZN5test51iIJNS_1ZENS_1XENS_1YEEEEvv", "void test5::i<test5::Z, test5::X, test5::Y>()"},
+    {"_ZN5test51pINS_1AEEEvv", "void test5::p<test5::A>()"},
+    {"_ZN5test51pITtTpTyENS_1BEEEvv", "void test5::p<test5::B>()"},
+    {"_ZN5test51qITtTyTyENS_1AEEEvv", "void test5::q<test5::A>()"},
+    {"_ZN5test51qINS_1BEEEvv", "void test5::q<test5::B>()"},
+    {"_ZN5test61fITk1CiEEvT_", "void test6::f<int>(int)"},
+    {"_ZN5test61gIiTk1DIT_EiEEvT0_", "void test6::g<int, int>(int)"},
+    {"_ZZN5test71fIiEEvvENKUlTyQaa1CIT_E1CITL0__ET0_E_clIiiEEDaS3_Q1CIDtfp_EE", "auto void test7::f<int>()::'lambda'<typename $T> requires C<T> && C<TL0_> (auto)::operator()<int, int>(auto) const requires C<decltype(fp)>"},
+    {"_ZZN5test71fIiEEvvENKUlTyQaa1CIT_E1CITL0__ET0_E0_clIiiEEDaS3_Qaa1CIDtfp_EELb1E", "auto void test7::f<int>()::'lambda0'<typename $T> requires C<T> && C<TL0_> (auto)::operator()<int, int>(auto) const requires C<decltype(fp)> && true"},
+    {"_ZZN5test71fIiEEvvENKUlTyQaa1CIT_E1CITL0__ET0_E1_clIiiEEDaS3_Q1CIDtfp_EE", "auto void test7::f<int>()::'lambda1'<typename $T> requires C<T> && C<TL0_> (auto)::operator()<int, int>(auto) const requires C<decltype(fp)>"},
+    {"_ZZN5test71fIiEEvvENKUlTyT0_E_clIiiEEDaS1_", "auto void test7::f<int>()::'lambda'<typename $T>(auto)::operator()<int, int>(auto) const"},
+
+    // C++20 requires expressions, see https://github.com/itanium-cxx-abi/cxx-abi/issues/24.
+    {"_Z1fIiEviQrqXcvT__EXfp_Xeqfp_cvS0__EXplcvS0__ELi1ER5SmallXmicvS0__ELi1ENXmlcvS0__ELi2ENR11SmallerThanILi1234EETS0_T1XIS0_ETNS3_4typeETS2_IiEQ11SmallerThanIS0_Li256EEE", "void f<int>(int) requires requires { (T)(); fp; fp == (T)(); {(T)() + 1} -> Small; {(T)() - 1} noexcept; {(T)() * 2} noexcept -> SmallerThan<1234>; typename T; typename X<T>; typename X<T>::type; typename X<int>; requires SmallerThan<T, 256>; }"},
+    {"_Z1gIiEviQrQT__XplfL0p_fp_E", "void g<int>(int) requires requires (T) { fp + fp; }"},
+
     // Special Substs a, b, d, i, o, s (not including std::)
     {"_Z1fSaIiE", "f(std::allocator<int>)"},
     {"_Z1fSbIiE", "f(std::basic_string<int>)"},

Unfortunately, Arcanist both insists on uploading "full context" (that is, the entire 5.2MB file, almost all of which is unchanged), and refuses to upload "very large" diffs, so it's not able to upload this small change to Phabricator for review.

I can't really review the libcxxabi parts, or the llvm-demangler parts, but everything looks right to me. I've got a pair of quick questions, otherwise I think this is going to be fine for me.

Note the extra paren changes are something I think are valuable, but I'm trying to figure out their meaning in thsi patch.

clang/include/clang/AST/ExprConcepts.h
529

Is this an unrelated change? Are paren locations required for mangling?

clang/lib/AST/ItaniumMangle.cpp
5271

What is happening here? What are we deriving from the validity of the paren? Or is this differentiating:

requires (something) vs requires C<T> sorta thing?

rsmith added inline comments.Apr 5 2023, 2:54 PM
clang/include/clang/AST/ExprConcepts.h
529

requires (params) { blah; } has a different mangling from requires { blah; } -- in particular, the former introduces a new level of function parameters, so references to enclosing function parameters are numbered differently in the two. This applies even if params is empty or void (requires () { ... } and requires (void) { ... } are both valid).

We previously generated the same AST for requires { ... } and requires () { ... }, which meant we couldn't mangle this correctly. Tracking the parens (or at least their existence) fixes that (and also improves the AST fidelity for tooling).

clang/lib/AST/ItaniumMangle.cpp
5271

Yes, the rq form is for requires expressions without a parameter list, and the rQ form is for requires expressions with a parameter list.

erichkeane accepted this revision.Apr 6 2023, 6:09 AM
erichkeane added inline comments.
clang/include/clang/AST/ExprConcepts.h
529

Tracking the parens (or at least their existence) fixes tha

it seems you test that via the LParenLoc, which is why I'm surprised about the addition of the RParenLoc? I have no problem with the addition of the RParenLoc for the below reason, but was trying to figure out the significance toward the mangling.

and also improves the AST fidelity for tooling

This I definitely get, was just trying to figure out why it was in a patch for mangling.

I agree it doesn't affect too much code, but people do instantiate templates manually sometimes. IIUC, linking against shared libraries would break if that library does explicit instantiations of constrained functions. That concerns me a bit.

Also, do you know if GCC developers are planning on implementing this change as well?

rsmith added a comment.Apr 6 2023, 4:05 PM

I agree it doesn't affect too much code, but people do instantiate templates manually sometimes. IIUC, linking against shared libraries would break if that library does explicit instantiations of constrained functions. That concerns me a bit.

There has not been any stable ABI from any compiler targeting the Itanium C++ ABI for constrained templates prior to this change. I don't think we need to worry too much about people using unfinished compiler features being broken when those features are finished.

From an ABI perspective, I think the other cases that I called out are more interesting than the constrained templates side of things: this changes the mangling for non-type template parameters with deduced types, non-type template parameters with instantiation-dependent types, and template template parameters that don't match their template template arguments, in the template parameter list of a function template. The latter two of those changes can in principle affect code dating back to C++98. There might be a case for putting those changes in particular behind a flag. @rjmccall I would particularly appreciate your feedback on that concern.

Also, do you know if GCC developers are planning on implementing this change as well?

I have no reason to think they would not follow the cross-vendor ABI specification, though I'll ping them explicitly. The ABI proposals haven't been accepted yet; I'm not intending to land this change until the proposals have reached consensus in the Itanium C++ ABI group.

clang/include/clang/AST/ExprConcepts.h
529

Yeah, we don't really need to track the RParenLoc, or the LParenLoc for that matter, as part of this mangling change, we just need any way to get the "were there parens?" information.

There has not been any stable ABI from any compiler targeting the Itanium C++ ABI for constrained templates prior to this change. I don't think we need to worry too much about people using unfinished compiler features being broken when those features are finished.

The ABI has been stable for quite some time and people have been using concepts with it for almost 3 years now. I'm not sure we can still break ABI this easily. But then, I also have no data to say we can't.

The ABI proposals haven't been accepted yet; I'm not intending to land this change until the proposals have reached consensus in the Itanium C++ ABI group.

Thanks for clarifying!

There has not been any stable ABI from any compiler targeting the Itanium C++ ABI for constrained templates prior to this change. I don't think we need to worry too much about people using unfinished compiler features being broken when those features are finished.

The ABI has been stable for quite some time and people have been using concepts with it for almost 3 years now. I'm not sure we can still break ABI this easily. But then, I also have no data to say we can't.

We've never claimed full support for concepts, so those folks would be relying on an unstable ABI. However, if it turns out this causes significant pain in practice, perhaps we could use ABI tags to give folks the older ABI? I'd prefer to avoid that in this case given that the feature isn't yet fully supported (I don't like the idea of setting a precedent for relying on the ABI of incomplete features in general), but concepts is a sufficiently important use case that I could imagine doing it as a one-off if needed.

The ABI proposals haven't been accepted yet; I'm not intending to land this change until the proposals have reached consensus in the Itanium C++ ABI group.

Thanks for clarifying!

There has not been any stable ABI from any compiler targeting the Itanium C++ ABI for constrained templates prior to this change. I don't think we need to worry too much about people using unfinished compiler features being broken when those features are finished.

The ABI has been stable for quite some time and people have been using concepts with it for almost 3 years now. I'm not sure we can still break ABI this easily. But then, I also have no data to say we can't.

We've never claimed full support for concepts, so those folks would be relying on an unstable ABI. However, if it turns out this causes significant pain in practice, perhaps we could use ABI tags to give folks the older ABI? I'd prefer to avoid that in this case given that the feature isn't yet fully supported (I don't like the idea of setting a precedent for relying on the ABI of incomplete features in general), but concepts is a sufficiently important use case that I could imagine doing it as a one-off if needed.

This patch already extends -fclang-abi-compat to retain the old manglings so that users can stay on an old (broken) ABI if they need to. I don't think we need to do more than that for the concepts mangling changes.

The ABI proposals haven't been accepted yet; I'm not intending to land this change until the proposals have reached consensus in the Itanium C++ ABI group.

The corresponding (confirmed) GCC bug report that they don't implement these mangling rules yet is: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100825

This patch supersedes https://reviews.llvm.org/D126818 (apologies for the duplicated work, @erichkeane -- I didn't find that before I started working on this).

rsmith edited the summary of this revision. (Show Details)Apr 7 2023, 12:55 PM
rsmith updated this revision to Diff 511787.Apr 7 2023, 1:51 PM
  • Add release notes.

There has not been any stable ABI from any compiler targeting the Itanium C++ ABI for constrained templates prior to this change. I don't think we need to worry too much about people using unfinished compiler features being broken when those features are finished.

The ABI has been stable for quite some time and people have been using concepts with it for almost 3 years now. I'm not sure we can still break ABI this easily. But then, I also have no data to say we can't.

We've never claimed full support for concepts, so those folks would be relying on an unstable ABI. However, if it turns out this causes significant pain in practice, perhaps we could use ABI tags to give folks the older ABI? I'd prefer to avoid that in this case given that the feature isn't yet fully supported (I don't like the idea of setting a precedent for relying on the ABI of incomplete features in general), but concepts is a sufficiently important use case that I could imagine doing it as a one-off if needed.

This patch already extends -fclang-abi-compat to retain the old manglings so that users can stay on an old (broken) ABI if they need to. I don't think we need to do more than that for the concepts mangling changes.

The ABI proposals haven't been accepted yet; I'm not intending to land this change until the proposals have reached consensus in the Itanium C++ ABI group.

The corresponding (confirmed) GCC bug report that they don't implement these mangling rules yet is: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100825

This patch supersedes https://reviews.llvm.org/D126818 (apologies for the duplicated work, @erichkeane -- I didn't find that before I started working on this).

No problem! This is a much more complete solution, and mine was held up on waiting for the Itanium ABI group to come to a consensus anyway (and fell by the wayside).

Matt added a subscriber: Matt.Apr 11 2023, 10:13 PM
rsmith added a comment.Sep 6 2023, 4:12 PM

Ping. Are there any further concerns here? (This obviously needs to be merged with trunk, and the -fclang-abi-compat= checks and release notes need to be updated to match the latest release version.)

ldionne accepted this revision as: Restricted Project.Sep 7 2023, 10:41 AM
This revision is now accepted and ready to land.Sep 7 2023, 10:41 AM
rsmith edited the summary of this revision. (Show Details)Sep 20 2023, 12:36 PM
This revision was landed with ongoing or failed builds.Sep 20 2023, 12:38 PM
This revision was automatically updated to reflect the committed changes.
dyung added a subscriber: dyung.Sep 21 2023, 6:36 PM

Hi @rsmith, we have an internal test where your change seems to have changed the mangling in C++17 mode and wanted to check if that was intentional.

Consider the following code:

// Literals in templates
#include <cstddef>
template <typename T, T I> T returnit() {return I;};
enum colour { RED = -3, GREEN, BLUE};
// use long type for enumeration
enum bigcolour { YELLOW = (1l << 32), CYAN, MAGENTA};
void callreturnit() {
    auto a = returnit<int, 4>();
    auto b = returnit<unsigned int, 4>();
    auto c = returnit<long, 4>();
    auto d = returnit<unsigned long, 4>();
    auto e = returnit<long long, -456789>();
    auto f = returnit<bool, true>();
    auto g = returnit<bool, false>();
    auto n = returnit<std::nullptr_t, nullptr>();
    auto cg = returnit<colour, GREEN>();
    auto cy = returnit<bigcolour, YELLOW>();
}

The compiler in C++17 mode now seems to differently mangle each instance:

functionbeforeafter
returnit<int, 4>();_Z8returnitIiLi4EET_v_Z8returnitIiTnT_Li4EES0_v
returnit<unsigned int, 4>();_Z8returnitIjLj4EET_v_Z8returnitIjTnT_Lj4EES0_v
returnit<long, 4>();_Z8returnitIlLl4EET_v_Z8returnitIlTnT_Ll4EES0_v
returnit<unsigned long, 4>();_Z8returnitImLm4EET_v_Z8returnitImTnT_Lm4EES0_v
returnit<long long, -456789>();_Z8returnitIxLxn456789EET_v_Z8returnitIxTnT_Lxn456789EES0_v
returnit<bool, true>();_Z8returnitIbLb1EET_v_Z8returnitIbTnT_Lb1EES0_v
returnit<bool, false>();_Z8returnitIbLb0EET_v_Z8returnitIbTnT_Lb0EES0_v
returnit<std::nullptr_t, nullptr>();_Z8returnitIDnLDn0EET_v_Z8returnitIDnTnT_LDn0EES0_v
returnit<colour, GREEN>();_Z8returnitI6colourLS0_n2EET_v_Z8returnitI6colourTnT_LS0_n2EES1_v
returnit<bigcolour, YELLOW>();_Z8returnitI9bigcolourLS0_4294967296EET_v_Z8returnitI9bigcolourTnT_LS0_4294967296EES1_v

Are these changes in non-C++20 mode intentional?

Hi @rsmith, we have an internal test where your change seems to have changed the mangling in C++17 mode and wanted to check if that was intentional.

[...]

Are these changes in non-C++20 mode intentional?

Yes, they're intentional. Unfortunately we could have mangling collisions here when using the old ABI rule; these two different templates would have the same mangling:

template <typename T, T I> T returnit() {return I;};
template <typename T, int I> T returnit() { return I; }

But... it looks like this case is missing from the list of affected cases in the change description and the release notes. Sorry about that. Release notes updated in rGaaa79a59317f859485d701d1eb68ac4cb213e1d1.

Hey, Richard. It looks like your new tests are failing on Apple's buildbots, I'm guessing because the default ABI compatibility mode is older.

I've been informed that Apple's change to the default ABI compatibility mode is in our private fork, so this is not your problem; sorry for the noise.

I'm not sure why we decided to do that privately; I'll see if we can fix that.

I've been informed that Apple's change to the default ABI compatibility mode is in our private fork, so this is not your problem; sorry for the noise.

I'm not sure why we decided to do that privately; I'll see if we can fix that.

We (Sony) also use an older ABI and I am working on a patch to be committed upstream to fix this for our target.

Yeah, the more I think about this, the more I think that while (1) Apple should upstream its use of an older default, regardless (2) the existence of any targets at all with an older default means that tests like this always need to be using -fclang-abi-compat=latest.

Yeah, the more I think about this, the more I think that while (1) Apple should upstream its use of an older default, regardless (2) the existence of any targets at all with an older default means that tests like this always need to be using -fclang-abi-compat=latest.

Seems reasonable. I went ahead and made that change in rG940850066290a484144db80f09e6c19709f5fe49.

I think we could probably limit this to just the test using %itanium_abi_triple, but there's also an affected test specifying -triple=x86_64-apple-darwin9, so that might not be enough for your needs. Also, I suppose some vendors might want to carry a patch that changes the default globally rather than on a per-target basis. So I just changed all the tests that touch the new mangling.

Hi @rsmith,

these two different templates would have the same mangling:

template <typename T, T I> T returnit() {return I;};
template <typename T, int I> T returnit() { return I; }

I tried compiling long foo() { return returnit<long, 4>(); } with these two templates, and got different manglings.
_Z8returnitIlLl4EET_v and
_Z8returnitIlLi4EET_v

Am I misunderstanding something about the problem with the old mangling rules?

Hi @rsmith,

these two different templates would have the same mangling:

template <typename T, T I> T returnit() {return I;};
template <typename T, int I> T returnit() { return I; }

I tried compiling long foo() { return returnit<long, 4>(); } with these two templates, and got different manglings.
_Z8returnitIlLl4EET_v and
_Z8returnitIlLi4EET_v

Am I misunderstanding something about the problem with the old mangling rules?

You need to do returnit<int, 4>.

You need to do returnit<int, 4>.

Doh! Thanks. Those two instantiations could have different function bodies, but would have the same mangled name. Got it.