[abi][ItaniumMangle] Inconsistent mangling of friend function templates.

[VitaNuo]

a.cc:

template <class>
concept True = true;

namespace test2 {
  template<typename T> struct A {
    template<typename U>
    friend void h(...) requires True<U> {}

    template<typename S>
    friend void g(...) requires True<S>;
  };

template<typename S>
void g(...) requires True<S> {}

void call() {
    A<int> ai;
    h<void>(ai);
    g<void>(ai);
}
}

In the above snippet:

• the call h<void>(ai) is mangled as _ZN5test21hIvEEvzQ4TrueITL0__E. Note that the constrained parameter U is mangled as TL0_ (4TrueITL0__E), i.e., a second-level template parameter (presumably the next level after the template parameters of the enclosing class A). Note that the enclosing class template specialization A<int> is not part of the mangling.
• the call g<void>(ai) is mangled as _ZN5test21gIvEEvzQ4TrueIT_E. Note that the constrained parameter U is mangled as T_ (4TrueIT_E), i.e., a first-level template parameter.
• the difference between h and g is that the definition of friend template function h is inside of struct A, and the definition of g is outside. Otherwise, their signatures are identical.

The mangling of h<void>(ai) makes it impossible to implement demangling of template parameter substitutions, since the substitution TL0_ cannot be demangled.

Options:

• Include the enclosing class template specialization A<int> into the mangling of h<void>(ai), i.e., in cases when the friend function definition is part of the enclosing class.
• Mangle the constraint True<U> in the definition of h as TrueIT_E, i.e., as a first-level template parameter (ignoring the enclosing class), since as per https://eel.is/c++draft/temp.friend#9 and mangling for constrained templates itanium-cxx-abi/cxx-abi#24 (comment), h is not a member-like constrained friend.

The latter option seems to fit better into the proposal in itanium-cxx-abi/cxx-abi#24 (comment).

[llvmbot]

@llvm/issue-subscribers-tools-llvm-cxxfilt

Author: Viktoriia Bakalova (VitaNuo)

a.cc: ``` template <class> concept True = true;

namespace test2 {
template<typename T> struct A {
template<typename U>
friend void h(...) requires True<U> {}

template<typename S>
friend void g(...) requires True<S>;

};

template<typename S>
void g(...) requires True<S> {}

void call() {
A<int> ai;
h<void>(ai);
g<void>(ai);
}
}

In the above snippet:
- the call `h<void>(ai)` is mangled as `_ZN5test21hIvEEvzQ4TrueITL0__E`. Note that the constrained parameter `U` is mangled as  `TL0_` (`4TrueITL0__E`), i.e., a second-level template parameter (presumably the next level after the template parameters of the enclosing class `A`). Note that the enclosing class template specialization `A<int>` is not part of the mangling.
- the call `g<void>(ai)` is mangled as `_ZN5test21gIvEEvzQ4TrueIT_E`. Note that the constrained parameter `U` is mangled as  `T_` (`4TrueIT_E`), i.e., a first-level template parameter. 
- the difference between `h` and `g` is that the definition of friend template function `h` is inside of `struct A`, and the definition of `g` is outside. Otherwise, their signatures are identical.

The mangling of `h<void>(ai)` makes it impossible to implement [demangling of template parameter substitutions](https://github.com/VitaNuo/llvm-project/blob/1557e3b9d672bb1f2fce745d664c039d578e5d0c/llvm/include/llvm/Demangle/ItaniumDemangle.h#L5795), since the substitution `TL0_` cannot be demangled.

Options:
- Include the enclosing class template specialization `A<int>` into the mangling of `h<void>(ai)`, i.e., in cases when the friend function definition is part of the enclosing class.
- Mangle the constraint `True&lt;U&gt;` in the definition of `h` as `TrueIT_E`, i.e., as a first-level template parameter (ignoring the enclosing class), since as per https://eel.is/c++draft/temp.friend#<!-- -->9 and https://github.com/itanium-cxx-abi/cxx-abi/issues/24#issuecomment-1491130332, `h` is not a member-like constrained friend.

The latter option seems to fit better into the proposal in https://github.com/itanium-cxx-abi/cxx-abi/issues/24#issuecomment-1491130332.

</details>

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: Viktoriia Bakalova (VitaNuo)

a.cc: ``` template <class> concept True = true;

namespace test2 {
template<typename T> struct A {
template<typename U>
friend void h(...) requires True<U> {}

template<typename S>
friend void g(...) requires True<S>;

};

template<typename S>
void g(...) requires True<S> {}

void call() {
A<int> ai;
h<void>(ai);
g<void>(ai);
}
}

In the above snippet:
- the call `h<void>(ai)` is mangled as `_ZN5test21hIvEEvzQ4TrueITL0__E`. Note that the constrained parameter `U` is mangled as  `TL0_` (`4TrueITL0__E`), i.e., a second-level template parameter (presumably the next level after the template parameters of the enclosing class `A`). Note that the enclosing class template specialization `A<int>` is not part of the mangling.
- the call `g<void>(ai)` is mangled as `_ZN5test21gIvEEvzQ4TrueIT_E`. Note that the constrained parameter `U` is mangled as  `T_` (`4TrueIT_E`), i.e., a first-level template parameter. 
- the difference between `h` and `g` is that the definition of friend template function `h` is inside of `struct A`, and the definition of `g` is outside. Otherwise, their signatures are identical.

The mangling of `h<void>(ai)` makes it impossible to implement [demangling of template parameter substitutions](https://github.com/VitaNuo/llvm-project/blob/1557e3b9d672bb1f2fce745d664c039d578e5d0c/llvm/include/llvm/Demangle/ItaniumDemangle.h#L5795), since the substitution `TL0_` cannot be demangled.

Options:
- Include the enclosing class template specialization `A<int>` into the mangling of `h<void>(ai)`, i.e., in cases when the friend function definition is part of the enclosing class.
- Mangle the constraint `True&lt;U&gt;` in the definition of `h` as `TrueIT_E`, i.e., as a first-level template parameter (ignoring the enclosing class), since as per https://eel.is/c++draft/temp.friend#<!-- -->9 and https://github.com/itanium-cxx-abi/cxx-abi/issues/24#issuecomment-1491130332, `h` is not a member-like constrained friend.

The latter option seems to fit better into the proposal in https://github.com/itanium-cxx-abi/cxx-abi/issues/24#issuecomment-1491130332.

</details>

[zygoloid]

Yes, the current mangling is wrong. The second option is the intended one -- we should mangle a constraint on a non-member-like constrained friend the same way that we'd mangle the constraint if it appears outside of the enclosing templates. I think that's going to be prohibitively hard to do directly from the mangler without changes to the AST: the mangler relies on having canonical representations for things like types in order to form proper substitutions, and those won't work correctly if we use the constraint expression from the AST because it'll refer to template parameters at the wrong depth. For example:

template<typename T> concept C = true;

template<typename T> struct A {
  template<typename U> requires C<U> friend U f(...) {}
};

void g(A<int> a) { f<float>(a); }

mangles as _Z1fIfQ1CITL0__EET_z. If we naively implement the depth renumbering in the mangler we'd instead get _Z1fIfQ1CIT_EET_z (with no substitution because the two T_s are different types), but we should actually mangle this as _Z1fIfQ1CIT_EES0_z. In order to get the correct result reliably we will need to "substitute" into the constraints to remove the unused outer levels.

So: I think we should perform an eager substitution into the constraints of a non-member-like constrained friend while we're type-checking its declaration in Sema, in order to reduce the depth of the template parameters so that they match a declaration in the semantic DeclContext, and store the result somewhere on the friend function for use in mangling.

We should also think about using that pre-substituted form for redeclaration matching and satisfaction checks, and storing only a single form for the constraints, always relative to the effective parent (that is, the lexical parent for a member-like constrained friend and the semantic parent for anything else). That would likely be a cleaner and better representation in general, because all redeclarations of an entity would then have the same constraints -- but would probably increase the number of places that need to special-case member-like constrained friends.

@mizvekov This seems relevant to your interests.

[zygoloid]

itanium-cxx-abi/cxx-abi#106 (comment) is also relevant: the demangler is likely to get confused about substitutions from later in the mangling of a constrained member-like friend template referring back to things in the constraints that they match according to Clang's canonical types.