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

function signature constraints are not a part of mangled name #49884

Closed
llvmbot opened this issue May 31, 2021 · 7 comments
Closed

function signature constraints are not a part of mangled name #49884

llvmbot opened this issue May 31, 2021 · 7 comments
Assignees
Labels
bugzilla Issues migrated from bugzilla c++20 clang:codegen

Comments

@llvmbot
Copy link
Collaborator

llvmbot commented May 31, 2021

Bugzilla Link 50540
Version 11.0
OS All
Reporter LLVM Bugzilla Contributor
CC @Dushistov,@zygoloid,@saarraz

Extended Description

$ cat b.cpp && echo EOFFFFFFFFFF

template <class> void foo() {} //1
void useFirst()
{
    foo<void>();// call 1 - instantiate "void foo<int>()"
}

template <class> void foo() requires true {} //2
void useSecond()
{
    foo<void>();// call 2 - instantiate "void foo<int>() requires true"
}

EOFFFFFFFFFF

$ clang++ -v -std=c++20 -c b.cpp
clang version 11.1.0
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/lib/llvm/11/bin
Selected GCC installation: /usr/lib/gcc/x86_64-pc-linux-gnu/10.2.0
Candidate multilib: .;@m64
Candidate multilib: 32;@m32
Selected multilib: .;@m64
 (in-process)
 "/usr/lib/llvm/11/bin/clang-11" -cc1 -triple x86_64-pc-linux-gnu -emit-obj -mrelax-all -disable-free -disable-llvm-verifier -discard-value-names -main-file-name b.cpp -mrelocation-model static -mframe-pointer=all -fmath-errno -fno-rounding-math -mconstructor-aliases -munwind-tables -target-cpu x86-64 -fno-split-dwarf-inlining -debugger-tuning=gdb -v -resource-dir /usr/lib/llvm/11/bin/../../../../lib/clang/11.1.0 -internal-isystem /usr/lib/gcc/x86_64-pc-linux-gnu/10.2.0/include/g++-v10 -internal-isystem /usr/lib/gcc/x86_64-pc-linux-gnu/10.2.0/include/g++-v10/x86_64-pc-linux-gnu -internal-isystem /usr/lib/gcc/x86_64-pc-linux-gnu/10.2.0/include/g++-v10/backward -internal-isystem /usr/local/include -internal-isystem /usr/lib/llvm/11/bin/../../../../lib/clang/11.1.0/include -internal-externc-isystem /include -internal-externc-isystem /usr/include -std=c++20 -fdeprecated-macro -fdebug-compilation-dir /home/vopl/tmp/15 -ferror-limit 19 -fgnuc-version=4.2.1 -fcxx-exceptions -fexceptions -fcolor-diagnostics -o b.o -x c++ b.cpp
clang -cc1 version 11.1.0 based upon LLVM 11.1.0 default target x86_64-pc-linux-gnu
ignoring nonexistent directory "/usr/local/include"
ignoring nonexistent directory "/include"
#include "..." search starts here:
#include <...> search starts here:
 /usr/lib/gcc/x86_64-pc-linux-gnu/10.2.0/include/g++-v10
 /usr/lib/gcc/x86_64-pc-linux-gnu/10.2.0/include/g++-v10/x86_64-pc-linux-gnu
 /usr/lib/gcc/x86_64-pc-linux-gnu/10.2.0/include/g++-v10/backward
 /usr/lib/llvm/11/bin/../../../../lib/clang/11.1.0/include
 /usr/include
End of search list.
b.cpp:7:23: error: definition with same mangled name '_Z3fooIvEvv' as another definition
template <class> void foo() requires true {} //2
                      ^
b.cpp:1:23: note: previous definition is here
template <class> void foo() {} //1
                      ^
1 error generated.

My expectations: mangled names must be different, because mangled name is based on the signature and a signature includes the requires-clause as a part

@llvmbot
Copy link
Collaborator Author

llvmbot commented May 31, 2021

@llvmbot
Copy link
Collaborator Author

llvmbot commented Jun 4, 2021

@shafik
Copy link
Collaborator

shafik commented Sep 20, 2023

@zygoloid it looks like the discussion on the ABI issue died out but on the gcc bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100825

you mentioned that you had an implementation ready, is that still the case?

@llvmbot
Copy link
Collaborator Author

llvmbot commented Sep 20, 2023

@llvm/issue-subscribers-clang-codegen

| | | | --- | --- | | Bugzilla Link | [50540](https://llvm.org/bz50540) | | Version | 11.0 | | OS | All | | Reporter | LLVM Bugzilla Contributor | | CC | @dushistov,@zygoloid,@saarraz |

Extended Description

$ cat b.cpp && echo EOFFFFFFFFFF

template &lt;class&gt; void foo() {} //1
void useFirst()
{
    foo&lt;void&gt;();// call 1 - instantiate "void foo&lt;int&gt;()"
}

template &lt;class&gt; void foo() requires true {} //2
void useSecond()
{
    foo&lt;void&gt;();// call 2 - instantiate "void foo&lt;int&gt;() requires true"
}

EOFFFFFFFFFF

$ clang++ -v -std=c++20 -c b.cpp
clang version 11.1.0
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/lib/llvm/11/bin
Selected GCC installation: /usr/lib/gcc/x86_64-pc-linux-gnu/10.2.0
Candidate multilib: .;@<!-- -->m64
Candidate multilib: 32;@<!-- -->m32
Selected multilib: .;@<!-- -->m64
 (in-process)
 "/usr/lib/llvm/11/bin/clang-11" -cc1 -triple x86_64-pc-linux-gnu -emit-obj -mrelax-all -disable-free -disable-llvm-verifier -discard-value-names -main-file-name b.cpp -mrelocation-model static -mframe-pointer=all -fmath-errno -fno-rounding-math -mconstructor-aliases -munwind-tables -target-cpu x86-64 -fno-split-dwarf-inlining -debugger-tuning=gdb -v -resource-dir /usr/lib/llvm/11/bin/../../../../lib/clang/11.1.0 -internal-isystem /usr/lib/gcc/x86_64-pc-linux-gnu/10.2.0/include/g++-v10 -internal-isystem /usr/lib/gcc/x86_64-pc-linux-gnu/10.2.0/include/g++-v10/x86_64-pc-linux-gnu -internal-isystem /usr/lib/gcc/x86_64-pc-linux-gnu/10.2.0/include/g++-v10/backward -internal-isystem /usr/local/include -internal-isystem /usr/lib/llvm/11/bin/../../../../lib/clang/11.1.0/include -internal-externc-isystem /include -internal-externc-isystem /usr/include -std=c++20 -fdeprecated-macro -fdebug-compilation-dir /home/vopl/tmp/15 -ferror-limit 19 -fgnuc-version=4.2.1 -fcxx-exceptions -fexceptions -fcolor-diagnostics -o b.o -x c++ b.cpp
clang -cc1 version 11.1.0 based upon LLVM 11.1.0 default target x86_64-pc-linux-gnu
ignoring nonexistent directory "/usr/local/include"
ignoring nonexistent directory "/include"
#include "..." search starts here:
#include &lt;...&gt; search starts here:
 /usr/lib/gcc/x86_64-pc-linux-gnu/10.2.0/include/g++-v10
 /usr/lib/gcc/x86_64-pc-linux-gnu/10.2.0/include/g++-v10/x86_64-pc-linux-gnu
 /usr/lib/gcc/x86_64-pc-linux-gnu/10.2.0/include/g++-v10/backward
 /usr/lib/llvm/11/bin/../../../../lib/clang/11.1.0/include
 /usr/include
End of search list.
b.cpp:7:23: error: definition with same mangled name '_Z3fooIvEvv' as another definition
template &lt;class&gt; void foo() requires true {} //2
                      ^
b.cpp:1:23: note: previous definition is here
template &lt;class&gt; void foo() {} //1
                      ^
1 error generated.

My expectations: mangled names must be different, because mangled name is based on the signature and a signature includes the requires-clause as a part

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

@zygoloid it looks like the discussion on the ABI issue died out but on the gcc bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100825

you mentioned that you had an implementation ready, is that still the case?

Yes, I've merged https://reviews.llvm.org/D147655 as 4b163e3, which fixes this issue. We now produce these symbols:

  • _Z3fooIvEvv for unconstrained foo<void>()
  • _Z3fooIvEvvQLb1E for foo<void>() requires true

@shafik
Copy link
Collaborator

shafik commented Sep 20, 2023

Yes, I've merged https://reviews.llvm.org/D147655 as 4b163e3, which fixes this issue. We now produce these symbols:

  • _Z3fooIvEvv for unconstrained foo<void>()
  • _Z3fooIvEvvQLb1E for foo<void>() requires true

Excellent, does this also fix: #52951

@zygoloid
Copy link
Collaborator

Excellent, does this also fix: #52951

It does!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla c++20 clang:codegen
Projects
Status: Done
Development

No branches or pull requests

4 participants