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

out of line definition should reject template parameter with lambda of unevaluated context #51416

Closed
nickhuang99 opened this issue Oct 5, 2021 · 11 comments
Assignees
Labels
bugzilla Issues migrated from bugzilla c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" release:backport release:reviewed

Comments

@nickhuang99
Copy link

Bugzilla Link 52074
Version trunk
OS Linux
CC @dwblaikie,@zygoloid

Extended Description

The following code should be rejected as no matching declaration found as GCC/MSVC++ does. clang-13 incorrectly passes without considering that two lambda expressions are never considered as equivalent. [temp.over.link#5.sentence-4]

template
struct A{
void spam(decltype([]{}) );
};

template
void A::spam(decltype([]{}))
{}

struct A{
template
void spam(decltype([]{}) );
};

template
void A::spam(decltype([]{}))
{}

Both of above should be rejected as no declaration found.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 11, 2021
@EugeneZelenko EugeneZelenko added the clang:frontend Language frontend issues, e.g. anything involving "Sema" label Mar 25, 2022
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 25, 2022

@llvm/issue-subscribers-clang-frontend

@cor3ntin cor3ntin added this to the LLVM 14.0.1 Release milestone Mar 25, 2022
@tstellar
Copy link
Collaborator

tstellar commented Apr 2, 2022

/cherry-pick 3784e8c

@llvmbot
Copy link
Collaborator

llvmbot commented Apr 2, 2022

/branch llvmbot/llvm-project/issue51416

llvmbot pushed a commit to llvmbot/llvm-project that referenced this issue Apr 2, 2022
Unlike other types, when lambdas are instanciated,
they are recreated from scratch.
When an unevaluated lambdas appear in the type of a function,
parameter it is instanciated in the wrong declaration context,
as parameters are transformed before the function.

To support lambda in function parameters, we try to
compute whether they are dependant without looking at the
declaration context.

This is a short term stopgap solution to avoid clang
iceing. A better fix might be to inject some kind of
transparent declaration with correctly computed dependency
for function parameters, variable templates, etc.

Fixes llvm#50376
Fixes llvm#51414
Fixes llvm#51416
Fixes llvm#51641
Fixes llvm#54296

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D121532

(cherry picked from commit 3784e8c)
llvmbot pushed a commit to llvmbot/llvm-project that referenced this issue Apr 2, 2022
Unlike other types, when lambdas are instanciated,
they are recreated from scratch.
When an unevaluated lambdas appear in the type of a function,
parameter it is instanciated in the wrong declaration context,
as parameters are transformed before the function.

To support lambda in function parameters, we try to
compute whether they are dependant without looking at the
declaration context.

This is a short term stopgap solution to avoid clang
iceing. A better fix might be to inject some kind of
transparent declaration with correctly computed dependency
for function parameters, variable templates, etc.

Fixes llvm#50376
Fixes llvm#51414
Fixes llvm#51416
Fixes llvm#51641
Fixes llvm#54296

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D121532

(cherry picked from commit 3784e8c)
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 2, 2022

/pull-request llvmbot#148

llvmbot pushed a commit to llvmbot/llvm-project that referenced this issue Apr 2, 2022
Unlike other types, when lambdas are instanciated,
they are recreated from scratch.
When an unevaluated lambdas appear in the type of a function,
parameter it is instanciated in the wrong declaration context,
as parameters are transformed before the function.

To support lambda in function parameters, we try to
compute whether they are dependant without looking at the
declaration context.

This is a short term stopgap solution to avoid clang
iceing. A better fix might be to inject some kind of
transparent declaration with correctly computed dependency
for function parameters, variable templates, etc.

Fixes llvm#50376
Fixes llvm#51414
Fixes llvm#51416
Fixes llvm#51641
Fixes llvm#54296

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D121532

(cherry picked from commit 3784e8c)
llvmbot pushed a commit to llvmbot/llvm-project that referenced this issue Apr 2, 2022
Unlike other types, when lambdas are instanciated,
they are recreated from scratch.
When an unevaluated lambdas appear in the type of a function,
parameter it is instanciated in the wrong declaration context,
as parameters are transformed before the function.

To support lambda in function parameters, we try to
compute whether they are dependant without looking at the
declaration context.

This is a short term stopgap solution to avoid clang
iceing. A better fix might be to inject some kind of
transparent declaration with correctly computed dependency
for function parameters, variable templates, etc.

Fixes llvm#50376
Fixes llvm#51414
Fixes llvm#51416
Fixes llvm#51641
Fixes llvm#54296

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D121532

(cherry picked from commit 3784e8c)
@tstellar
Copy link
Collaborator

tstellar commented Apr 5, 2022

@AaronBallman Is this safe to backport? 3784e8c

@AaronBallman
Copy link
Collaborator

@AaronBallman Is this safe to backport? 3784e8c

Yes, I think it is (I also double-checked with @erichkeane).

@tstellar
Copy link
Collaborator

tstellar commented Apr 7, 2022

@cor3ntin We can't merge the patch as is, because there are ABI breaking changes to CreateLambda() and createLambdaClosureType(). Is there someway to rework the patch without changing the ABI?

@cor3ntin
Copy link
Contributor

cor3ntin commented Apr 7, 2022

@tstellar That seems pretty difficult if not impossible. I guess if we can't backport this, we might need to consider reverting https://reviews.llvm.org/rG22aa3680eaccb9b77ca224711c4da3a354aa2d45 in the 14x branche instead, would that be possible?

The state of affair in the 14 branch is pretty sad, as it crashes on very simple and non unusual code, like
https://godbolt.org/z/P3qPorsq1, such that having that commit in is likely to do more harm that good (to the extent that users use C++20 features).

What would be the process for that?

@AaronBallman
Copy link
Collaborator

Oof, I didn't spot the ABI break, that is unfortunate. (Good catch @tstellar!). We could do something like this: https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaDeclCXX.cpp#L7013 to differ the ABI based on what the user wants to be compatible with, but that may be a risky approach this late (or maybe it actually turns out to be straightforward, I don't know).

@tstellar
Copy link
Collaborator

tstellar commented Apr 14, 2022

@AaronBallman I'm just talking about the ABI of libclang-cpp.so. Is there also an ABI issue with the code that the complier generates with this patch?

@tstellar
Copy link
Collaborator

@cor3ntin I think you can fix it by adding an overload of createLambdaClosureType() with the old parameter types and have that wrap the new implementation.

mem-frob pushed a commit to draperlaboratory/hope-llvm-project that referenced this issue Oct 7, 2022
Unlike other types, when lambdas are instanciated,
they are recreated from scratch.
When an unevaluated lambdas appear in the type of a function,
parameter it is instanciated in the wrong declaration context,
as parameters are transformed before the function.

To support lambda in function parameters, we try to
compute whether they are dependant without looking at the
declaration context.

This is a short term stopgap solution to avoid clang
iceing. A better fix might be to inject some kind of
transparent declaration with correctly computed dependency
for function parameters, variable templates, etc.

Fixes llvm/llvm-project#50376
Fixes llvm/llvm-project#51414
Fixes llvm/llvm-project#51416
Fixes llvm/llvm-project#51641
Fixes llvm/llvm-project#54296

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D121532
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:frontend Language frontend issues, e.g. anything involving "Sema" release:backport release:reviewed
Projects
None yet
Development

No branches or pull requests

6 participants