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

[clang] name mangling for lambdas in local classes broken since clang-13 #88906

Closed
daantimmer opened this issue Apr 16, 2024 · 4 comments · Fixed by #89204
Closed

[clang] name mangling for lambdas in local classes broken since clang-13 #88906

daantimmer opened this issue Apr 16, 2024 · 4 comments · Fixed by #89204
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party lambda C++11 lambda expressions

Comments

@daantimmer
Copy link

Minimal Reproducible Example:

#include <iostream>
#include <functional>

int main(){
  class Test{
    public:
    std::function<void()> a{[]{ std::cout << "a\n"; }};
    std::function<void()> b{[]{ std::cout << "b\n"; }};
    std::function<void()> c{[]{ std::cout << "c\n"; }};
  };
  
  Test test{};
  test.a();
  test.b();
  test.c();
}

// output:
c
c
c

// expected:
a
b
c

godbolt setup with clang-trunk, clang-12 and gcc-trunk: https://godbolt.org/z/EM36r4Ybo

So, if you have a local class with lambda's the name mangling seems to be missing/going wrong. Resulting in only one actual lambda being declared.

@github-actions github-actions bot added the clang Clang issues not falling into any other category label Apr 16, 2024
@cor3ntin cor3ntin added clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party labels Apr 16, 2024
@llvmbot
Copy link
Member

llvmbot commented Apr 16, 2024

@llvm/issue-subscribers-clang-frontend

Author: Daan Timmer (daantimmer)

Minimal Reproducible Example:
#include &lt;iostream&gt;
#include &lt;functional&gt;

int main(){
  class Test{
    public:
    std::function&lt;void()&gt; a{[]{ std::cout &lt;&lt; "a\n"; }};
    std::function&lt;void()&gt; b{[]{ std::cout &lt;&lt; "b\n"; }};
    std::function&lt;void()&gt; c{[]{ std::cout &lt;&lt; "c\n"; }};
  };
  
  Test test{};
  test.a();
  test.b();
  test.c();
}

// output:
c
c
c

// expected:
a
b
c

godbolt setup with clang-trunk, clang-12 and gcc-trunk: https://godbolt.org/z/EM36r4Ybo

So, if you have a local class with lambda's the name mangling seems to be missing/going wrong. Resulting in only one actual lambda being declared.

@cor3ntin
Copy link
Contributor

Here

void CXXNameMangler::mangleLambda(const CXXRecordDecl *Lambda) {
// When trying to be ABI-compatibility with clang 12 and before, mangle a
// <data-member-prefix> now, with no substitutions.
if (Decl *Context = Lambda->getLambdaContextDecl()) {
if (isCompatibleWith(LangOptions::ClangABI::Ver12) &&
(isa<VarDecl>(Context) || isa<FieldDecl>(Context)) &&
!isa<ParmVarDecl>(Context)) {
if (const IdentifierInfo *Name
= cast<NamedDecl>(Context)->getIdentifier()) {
mangleSourceName(Name);
const TemplateArgumentList *TemplateArgs = nullptr;
if (GlobalDecl TD = isTemplate(cast<NamedDecl>(Context), TemplateArgs))
mangleTemplateArgs(asTemplateName(TD), *TemplateArgs);
Out << 'M';
}
}
}

We seem to do the right thing for the 12ABI, but not for newer ABIs, which is odd.

Itanium specification: https://itanium-cxx-abi.github.io/cxx-abi/abi.html#closure-types

If the context of a closure type is an initializer for a class member (static or nonstatic), inline variable, or variable template, it is encoded in a qualified name with a of the form:

    <closure-prefix> ::= [ <prefix> ] <variable or member unqualified-name> M
                     ::= <variable template template-prefix> <template-args> M

@cor3ntin
Copy link
Contributor

This might have been changed by 5bb7e81
Any idea @zygoloid ?

@EugeneZelenko EugeneZelenko added lambda C++11 lambda expressions and removed clang Clang issues not falling into any other category labels Apr 16, 2024
@zygoloid
Copy link
Collaborator

We're supposed to call mangleClosurePrefix at a higher level in mangling to mangle the prefix, I think. We should figure out why that's not happening in this case.

@tbaederr tbaederr changed the title [clang] name mangling for lambda's in local classes broken since clang-13 [clang] name mangling for lambdas in local classes broken since clang-13 Apr 16, 2024
cor3ntin added a commit to cor3ntin/llvm-project that referenced this issue Apr 18, 2024
Lambdas used in the initializer of a local class were not mangling
the name of the member.

Fixes llvm#88906
cor3ntin added a commit that referenced this issue Apr 19, 2024
Lambdas used in the initializer of a local class were not mangling the
name of the member.

Fixes #88906
aniplcc pushed a commit to aniplcc/llvm-project that referenced this issue Apr 21, 2024
Lambdas used in the initializer of a local class were not mangling the
name of the member.

Fixes llvm#88906
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party lambda C++11 lambda expressions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants