-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
externally visible lambda in a static data member of a class is given internal linkage and wrong mangled name #58819
Comments
@llvm/issue-subscribers-clang-codegen |
@llvm/issue-subscribers-c-17 |
First pass at this. llvm-project/clang/lib/Parse/ParseDeclCXX.cpp Line 3187 in 55e8998
But then llvm-project/clang/lib/Sema/SemaLambda.cpp Line 1282 in 55e8998
Not sure why the latter doesn't happen for other adjacent cases (like lambdas in default argument initializers in a class, or anywhere else for that matter)... yet to be looked into. |
Hmm, nope, that seems to happen in the good cases too - so that's a slight red herring... |
Ah: llvm-project/clang/lib/Sema/SemaLambda.cpp Lines 323 to 333 in 55e8998
Only mentions two out of the four cases listed in the Itanium spec:
|
oh, it lists other cases later/elsewhere in the switch, but I guess doesn't fully/correctly implement the cases.. |
Huh, does GCC have a bug here too? I managed to fix the mangling, and I think the linkage too, but the symbols still differ from GCC and it looks like GCC's using internal linkage?
|
if I'm reading this correctly, that is. |
yeah, I have a small test case that changes behavior under optimizations with GCC (due to inlining) and not with Clang. Guess I could simplify it further and get it to have the wrong behavior even without optimization... |
lambda.h:
a.cpp:
b.cpp:
Returns 1 in a non-conforming implementation (GCC and Clang ToT) and 0 when conforming (with a prototype patch I have for clang), I think? |
Should we be using the variable context mangling, though? GCC isn't - it's mangling without the variable name, using the class scope:
But it looks like from the example in the itanium spec, we should use the type name:
Hmm, GCC and Clang both get this ^ case right... oh, wait, no, GCC gets the mangling right (though it differs from GCC's handling of the non-template case) using the 'x' member, but gets the linkage different from Clang at least
Are those ABI compatible linkages? ( |
GCC bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107741 |
Fixed in e9c9db3 |
@llvm/issue-subscribers-clang-frontend |
Testcase:
A::x()
is required to return the same address in every translation unit. But Clang emits the lambda and its static local with internal linkage and with a vendor-specific$_n
mangling:See this live on compiler explorer.
See also the relevant ABI rule; the case we're in here is "the initializers of inline or templated variables".
The text was updated successfully, but these errors were encountered: