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.

Sign up for GitHub

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

Jump to bottom

Multiple immediate invocation expression-statements of lambda expressions should be constant expressions #50455

Closed
JohelEGP mannequin opened this issue Jul 15, 2021 · 8 comments · Fixed by llvm/llvm-project-release-prs#145
Closed

Multiple immediate invocation expression-statements of lambda expressions should be constant expressions #50455

JohelEGP mannequin opened this issue Jul 15, 2021 · 8 comments · Fixed by llvm/llvm-project-release-prs#145
Assignees
@usx95
Labels
bugzilla Issues migrated from bugzilla c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" consteval C++20 consteval rejects-valid release:backport release:merged
Milestone
LLVM 15.0.1 Release

Comments

@JohelEGP
Copy link
Mannequin

JohelEGP mannequin commented Jul 15, 2021

Bugzilla Link 51111
Version trunk
OS Linux
CC @JohelEGP,@zygoloid

Extended Description

See https://godbolt.org/z/YqG7P977v.

void f() {
  []() consteval { int i{}; }();
  []() consteval { int i{}; ++i; }();
}
void g() {
  (void)[](int i) consteval { return i; }(0);
  (void)[](int i) consteval { return i; }(0);
}
@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 11, 2021
@JohelEGP JohelEGP mentioned this issue Jun 6, 2022
Open
@usx95 usx95 added the consteval C++20 consteval label Aug 20, 2022
@usx95 usx95 self-assigned this Aug 30, 2022
@usx95 usx95 closed this as completed in e7eec38 Sep 2, 2022
@EugeneZelenko EugeneZelenko added the clang:frontend Language frontend issues, e.g. anything involving "Sema" label Sep 2, 2022
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 2, 2022

@llvm/issue-subscribers-clang-frontend

@royjacobson
Copy link
Contributor

@usx95 did you consider backporting to Clang 15.x? Seems like a pretty safe fix.

@usx95
Copy link
Contributor

usx95 commented Sep 6, 2022

/cherry-pick e7eec38

@usx95 usx95 reopened this Sep 6, 2022
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 6, 2022

/cherry-pick e7eec38

Error: Command failed due to missing milestone.

@EugeneZelenko EugeneZelenko added this to the LLVM 15.0.1 Release milestone Sep 6, 2022
@EugeneZelenko
Copy link
Contributor

/cherry-pick e7eec38

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 6, 2022

Failed to cherry-pick: e7eec38

https://github.com/llvm/llvm-project/actions/runs/3002632023

Please manually backport the fix and push it to your github fork. Once this is done, please add a comment like this:

/branch <user>/<repo>/<branch>

usx95 added a commit to usx95/llvm-project that referenced this issue Sep 6, 2022
@usx95
[clang] Skip re-building lambda expressions in parameters to consteva…
2201b17 
…l fns.

As discussed in this [comment](llvm#56183 (comment)),
we end up building the lambda twice: once while parsing the function calls and then again while handling the immediate invocation.

This happens specially during removing nested immediate invocation.
Eg: When we have another consteval function as the parameter along with this lambda expression. Eg: `foo(bar([]{}))`, `foo(bar(), []{})`

While removing such nested immediate invocations, we should not rebuild this lambda. (IIUC, rebuilding a lambda would always generate a new type which will never match the original type from parsing)

Fixes: llvm#56183
Fixes: llvm#51695
Fixes: llvm#50455
Fixes: llvm#54872
Fixes: llvm#54587

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

(cherry picked from commit e7eec38)
@usx95
Copy link
Contributor

usx95 commented Sep 6, 2022

/branch usx95/llvm-project/release/15.x

@llvmbot llvmbot mentioned this issue Sep 6, 2022
Merged
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 6, 2022

/pull-request llvm/llvm-project-release-prs#145

tru pushed a commit to llvm/llvm-project-release-prs that referenced this issue Sep 8, 2022
@usx95 @tru
[clang] Skip re-building lambda expressions in parameters to consteva…
1c73596 
…l fns.

As discussed in this [comment](llvm/llvm-project#56183 (comment)),
we end up building the lambda twice: once while parsing the function calls and then again while handling the immediate invocation.

This happens specially during removing nested immediate invocation.
Eg: When we have another consteval function as the parameter along with this lambda expression. Eg: `foo(bar([]{}))`, `foo(bar(), []{})`

While removing such nested immediate invocations, we should not rebuild this lambda. (IIUC, rebuilding a lambda would always generate a new type which will never match the original type from parsing)

Fixes: llvm/llvm-project#56183
Fixes: llvm/llvm-project#51695
Fixes: llvm/llvm-project#50455
Fixes: llvm/llvm-project#54872
Fixes: llvm/llvm-project#54587

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

(cherry picked from commit e7eec38)
mem-frob pushed a commit to draperlaboratory/hope-llvm-project that referenced this issue Oct 7, 2022
@usx95
[clang] Skip re-building lambda expressions in parameters to consteva…
ee5f087 
…l fns.

As discussed in this [comment](llvm/llvm-project#56183 (comment)),
we end up building the lambda twice: once while parsing the function calls and then again while handling the immediate invocation.

This happens specially during removing nested immediate invocation.
Eg: When we have another consteval function as the parameter along with this lambda expression. Eg: `foo(bar([]{}))`, `foo(bar(), []{})`

While removing such nested immediate invocations, we should not rebuild this lambda. (IIUC, rebuilding a lambda would always generate a new type which will never match the original type from parsing)

Fixes: llvm/llvm-project#56183
Fixes: llvm/llvm-project#51695
Fixes: llvm/llvm-project#50455
Fixes: llvm/llvm-project#54872
Fixes: llvm/llvm-project#54587

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

(cherry picked from commit e7eec38246560781e0a4020b19c7eb038a8c5655)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@usx95 usx95

Labels
bugzilla Issues migrated from bugzilla c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" consteval C++20 consteval rejects-valid release:backport release:merged
Projects
C++ 20 in Clang
Status: Done
LLVM Release Status
Archived in project
Milestone
LLVM 15.0.1 Release
Development

Successfully merging a pull request may close this issue.

PR for llvm/llvm-project#50455 llvm/llvm-project-release-prs
5 participants
@fhahn @royjacobson @usx95 @EugeneZelenko @llvmbot