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

mangling of lambdas for inline variables #94

Closed
urnathan opened this issue Mar 11, 2020 · 6 comments · Fixed by #126
Closed

mangling of lambdas for inline variables #94

urnathan opened this issue Mar 11, 2020 · 6 comments · Fixed by #126

Comments

@urnathan
Copy link
Contributor

urnathan commented Mar 11, 2020

Given the resolution of DR2300 inline variables intialized to lambda need to uniquely identify the lambda across translation units.

The document does describe this (#closure-types), but I find the wording confusing as it focusses on static data members. I hit an issue with namespace-scope variables:
inline auto bob = []{};

The grammar has:
<data-member-prefix> ::= <member source-name> [<template-args>] M
'<member source-name>' appears nowhere else in the document. I think just '<source-name>' would suffice.

It would also be less confusing if '>data-member-prefix<' changed to '<data-prefix>', it's used in on place in the <prefix> reduction.

@rjmccall
Copy link
Collaborator

rjmccall commented Mar 17, 2020

The mangling grammar often uses <*thing* production> as a shorthand for specifying what the production is formed from. Sometimes it's unnecessary, but I don't think it does much harm.

The specification usually uses terms from the standard when it can. When these prefixes only arose from data members (or, well, only arose in an ABI-affecting way), data-member-prefix was a fine name; I agree it should be renamed now. "data" isn't used alone in this way in the standard; I think the most appropriate term might just be "variable" (thus variable-prefix).

@zygoloid
Copy link
Contributor

zygoloid commented May 5, 2021

The mangling rule here is also missing at least one case:

inline auto x = []{};

... has no mangling for the lambda. The reason is that the only place we accept a <data-member-prefix> is after another <prefix>. Clang and GCC disagree on how to mangle this; GCC uses N1xMUlvE_E and Clang uses simply 1xMUlvE_. Note that Clang's mangling collides with the mangling for x followed by a pointer-to-member mangling for a member of the lambda closure type, so we should use the GCC mangling.

I think we're missing a <prefix> production:

<prefix> ::= <data-member-prefix>  # global variable

@zygoloid
Copy link
Contributor

zygoloid commented May 5, 2021

We also have a problem with substitutions. Given:

template<typename ...T> int f(T...) {}
inline int x[2] = { f([]{}, []{}, (int*)0, (int*)0) };
auto f() { return x; }
  • GCC produces _Z1fIJN1xMUlvE_EN1xMUlvE0_EPiS2_EEiDpT_
    • Substitutions are: f, x::<first lambda>, x::<second lambda>, int*
  • Clang produces _Z1fIJ1xMUlvE_1xMUlvE0_PiS2_EEiDpT_ (matching GCC other than the N...E)
    • Substitutions are: f, x::<first lambda>, x::<second lambda>, int*
  • ICC produces _Z1fIJN17_INTERNAL65e770d81xMUlvE_ENS1_UlvE0_EPiS4_EEiDpT_
    • Substitutions are: f, <INTERNAL namespace, I assume>, x, x::<first lambda>, x::<second lambda>, int*
  • LLVM demangler expects _Z1fIJN1xMUlvE_EN1xMUlvE0_EPiS4_EEiDpT_
    • Substitutions are: f, x, x::'lambda0'(), x, x::'lambda1'(), int*
  • GNU demangler expects _Z1fIJN1xMUlvE_EN1xMUlvE0_EPiS6_EEiDpT_
    • Substitutions are: f, x, {lambda()#1}, x::{lambda()#1}, x, {lambda()#2}, x::{lambda()#2}, ...

I think the GNU demangler treating {lambda()#1} as substitutable is simply a bug, as is the INTERNAL namespace that is visible in ICC's mangling.

The LLVM demangler seems to most closely be following the ABI rule, as does ICC if you ignore the spurious namespace. Should we update Clang and GCC to match, or change the ABI rule to say that a <data-member-prefix> is not a substitutable entity? I expect the ABI risk of making a change here is pretty small.

@zygoloid
Copy link
Contributor

zygoloid commented May 5, 2021

While we're here: it would be convenient for the <data-member-prefix> mangling to also cover structured bindings (even though I don't think there's any way currently for a lambda in a structured binding's initializer to be ABI-relevant). I'd suggest:

-<data-member-prefix> ::= <source-name> [ <template-args> ] M
+<data-member-prefix> ::= <unqualified-name> [ <template-args> ] M

zygoloid added a commit to zygoloid/cxx-abi that referenced this issue May 5, 2021
Extend grammar to cover additional contexts where this can be used.

Fixes itanium-cxx-abi#94.
@zygoloid
Copy link
Contributor

zygoloid commented May 5, 2021

Also: should we treat the template-name in a data-member-prefix that names a variable template specialization as a substitutable entity? Currently we don't do so.

zygoloid added a commit to zygoloid/cxx-abi that referenced this issue May 6, 2021
Extend grammar to cover additional contexts where this can be used.

Fixes itanium-cxx-abi#94.
@rjmccall
Copy link
Collaborator

Hmm. Variable template names probably ought to be substitutable, yeah. We can probably still get away with that.

I agree with all of your proposed changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants