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

Merged definitions of member functions distinguished by non-functionally-equivalent constraints and by deduced return type #61273

Closed
ecatmur opened this issue Mar 8, 2023 · 4 comments
Assignees
Labels
ABI Application Binary Interface clang:frontend Language frontend issues, e.g. anything involving "Sema"

Comments

@ecatmur
Copy link

ecatmur commented Mar 8, 2023

template<int I> concept C = true;
template<typename T> struct A {
    auto f() requires C<42> { return 1; }  // #1
    auto f() requires true { return 2u; }  // #2
};
int main() {
    int (A<int>::*p)() = &A<int>::f;
    unsigned (A<int>::*q)() = &A<int>::f;
    return (A<int>().*p)() + (A<int>().*q)();
}

The above program is accepted and returns 4; the expected result would be 3. Inspection of the code (by O0 or by volatile) determines that a single A<int>::f() is emitted, corresponding to #.2.

Very strangely, replacing 2u by 2l and unsigned by long causes clang to reject: "definition with same mangled name '_ZN1AIiE1fEv' as another definition". This seems more sensible, but is apparently contra the Standard says: cplusplus/CWG#256. I have pointed out that following the Standard would require mangling constraints, which I view as a non starter since it would break ABI now and in future.

@llvmbot
Copy link
Collaborator

llvmbot commented Mar 9, 2023

@llvm/issue-subscribers-clang-frontend

@shafik
Copy link
Collaborator

shafik commented Mar 9, 2023

CC @erichkeane

I believe this is #49884 and maybe #48216

@erichkeane
Copy link
Collaborator

Definitely looks like it. We don't have guidance from the Itanium ABI folks yet as far as I know (and this would be something for one of the codegen code owners to take a look at: @rjmccall @efriedma-quic @asl).

@thesamesam thesamesam added the ABI Application Binary Interface label Mar 14, 2023
@zygoloid zygoloid self-assigned this Apr 3, 2023
zygoloid added a commit that referenced this issue Sep 20, 2023
This implements proposals from:

- itanium-cxx-abi/cxx-abi#24: mangling for
  constraints, requires-clauses, requires-expressions.
- itanium-cxx-abi/cxx-abi#31: requires-clauses and
  template parameters in a lambda expression are mangled into the <lambda-sig>.
- itanium-cxx-abi/cxx-abi#47 (STEP 3): mangling for
  template argument is prefixed by mangling of template parameter declaration
  if it's not "obvious", for example because the template parameter is
  constrained (we already implemented STEP 1 and STEP 2).

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

Reviewed By: erichkeane, #libc_abi

Differential Revision: https://reviews.llvm.org/D147655
@zygoloid
Copy link
Collaborator

Fixed in 4b163e3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ABI Application Binary Interface clang:frontend Language frontend issues, e.g. anything involving "Sema"
Projects
None yet
Development

No branches or pull requests

7 participants