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

lambda in unevaluated-context is parsed incorrectedly #51641

Closed
nickhuang99 opened this issue Oct 25, 2021 · 9 comments
Closed

lambda in unevaluated-context is parsed incorrectedly #51641

nickhuang99 opened this issue Oct 25, 2021 · 9 comments
Assignees
Labels
bugzilla Issues migrated from bugzilla c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party release:backport release:reviewed

Comments

@nickhuang99
Copy link

Bugzilla Link 52299
Version trunk
OS Linux
CC @AaronBallman,@zygoloid

Extended Description

The following code is accepted incorrectly by clang while GCC and MSVC++ all spot the failure of assertion. The type of "foo" is obviously NOT "void" at all.

template
void foo(decltype(+{}) lambda, T param);

static_assert(__is_same(decltype(foo), void));

@nickhuang99
Copy link
Author

This is the link of testing:
https://www.godbolt.org/z/zjxv4KsvP

@AaronBallman
Copy link
Collaborator

We form a TypeTraitExpr object because we think the trait is dependent, but then we never go and instantiate the static_assert declaration, so we never re-test the type trait expression on instantiation. We think the trait is dependent because we think the expression argument in decltype(foo<int>) is dependent. The expression argument is actually an unresolved lookup expression by the time we get into Sema::ActOnDecltypeExpression(), which calls Sema::CheckPlaceholderExpr().

I think the issue is in Sema::BuildTemplateIdExpr(), which is where the UnresolvedLookupExpr is created. There's a FIXME comment specifically about how "foo could identify a single function unambiguously", but this does not work currently. However, I stopped feeling confident I knew what the correct fix was and so I've not looked farther.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 11, 2021
@llvmbot llvmbot added the confirmed Verified by a second party label Jan 26, 2022
@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 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

/branch llvmbot/llvm-project/issue51641

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#149

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

I believe it is safe to backport (I also double-checked with @erichkeane).

@AaronBallman AaronBallman assigned tstellar and unassigned AaronBallman Apr 6, 2022
@tstellar tstellar changed the title lambda in unevaluated-context is parsed incorrectedly 3 May 18, 2022
@tstellar tstellar changed the title 3 lambda in unevaluated-context is parsed incorrectedly May 18, 2022
@tstellar
Copy link
Collaborator

The backport of 3784e8c was also requested in #51416 , so I'm dropping the release milestone from this issue. We'll track the progress in #51416.

@tstellar tstellar removed this from the LLVM 14.0.4 Release milestone May 19, 2022
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" confirmed Verified by a second party release:backport release:reviewed
Projects
None yet
Development

No branches or pull requests

6 participants