Skip to content

Always use ConstFn context for const closures#155808

Open
lapla-cogito wants to merge 1 commit intorust-lang:mainfrom
lapla-cogito:issue_155803
Open

Always use ConstFn context for const closures#155808
lapla-cogito wants to merge 1 commit intorust-lang:mainfrom
lapla-cogito:issue_155803

Conversation

@lapla-cogito
Copy link
Copy Markdown
Contributor

@lapla-cogito lapla-cogito commented Apr 26, 2026 •
edited
Loading

fixes #155803

Since e8a4611, hir_body_const_context() returned the parent's const context for a const closure. But a const closure can escape its enclosing body via a fn-pointer-typed const item or an opaque return type and be invoked at runtime, so it must be const-checked under the most restrictive ConstFn context like a regular const fn.

r? oli-obk (since you authored the commit mentioned above, feel free to reroll)

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 26, 2026

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 26, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 26, 2026

oli-obk is not on the review rotation at the moment.
They may take a while to respond.

@oli-obk
Copy link
Copy Markdown
Contributor

oli-obk commented Apr 26, 2026

Hmmm. We shouldn't ever invoke optimized_mir on const closures defined in const items I think?

Tho I guess with an opaque type as the const's type we could indeed return a const closure. Or just via a const item of fn ptr type?

I'll need to think about this some more, but if you have any thoughts about it lmk. Please add tests for both of these situations, too.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@lapla-cogito
Copy link
Copy Markdown
Contributor Author

lapla-cogito commented Apr 26, 2026 •
edited
Loading

We shouldn't ever invoke optimized_mir on const closures defined in const items I think?

Is this because const items are only evaluated during CTFE (IIUC)? If so, the examples you mention are counterexamples.

Tho I guess with an opaque type as the const's type we could indeed return a const closure. Or just via a const item of fn ptr type?

I've added a test for the fn ptr type variant, which causes an ICE on current nightly. I also tried opaque-type variants, but they were rejected by unrelated checks before reaching the ICE path 🤔
Therefore, I think there's now a stronger case for invoking optimized_mir on const closures defined inside const items. Thanks.

@oli-obk
Copy link
Copy Markdown
Contributor

oli-obk commented Apr 28, 2026

I did some digging in the compiler, and I think the right solution is to treat all const closures as ConstContext::ConstFn regardless of their body. My rationale is as follows:

  1. you can return a const closure from a const item
  2. then you can invoke it at runtime
  3. if the const closure calls const-only code (not merged yet Add very basic "comptime" fn implementation #148820) then that would "work" when the const closure is defined in a const item
  4. but when called at runtime, there is no code to call, likely leading to an ICE in the future when fetching optimized_mir for a function that doesn't have runtime logic and thus no optimized_mir

@oli-obk oli-obk added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 28, 2026
@rust-bors

This comment has been minimized.

Sign in to view
@lapla-cogito lapla-cogito changed the title Dispatch MIR pipeline by BodyOwnerKind instead of const context Always use ConstFn context for const closures May 5, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 5, 2026

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

oli-obk
oli-obk approved these changes May 5, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@oli-obk oli-obk left a comment •
edited by rustbot
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors r+ rollup

View changes since this review

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented May 5, 2026

📌 Commit 0e72a29 has been approved by oli-obk

It is now in the queue for this repository.

@rust-bors rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 5, 2026
jhpratt added a commit to jhpratt/rust that referenced this pull request May 5, 2026
@jhpratt
Rollup merge of rust-lang#155808 - lapla-cogito:issue_155803, r=oli-obk
c58d866 
Always use `ConstFn` context for `const` closures

fixes rust-lang#155803

Since rust-lang@e8a4611, `hir_body_const_context()` returned the parent's const context for a `const` closure. But a `const` closure can escape its enclosing body via a `fn`-pointer-typed const item or an opaque return type and be invoked at runtime, so it must be const-checked under the most restrictive `ConstFn` context like a regular `const fn`.

r? oli-obk (since you authored the commit mentioned above, feel free to reroll)
@jhpratt jhpratt mentioned this pull request May 5, 2026
Closed
jhpratt added a commit to jhpratt/rust that referenced this pull request May 5, 2026
@jhpratt
Rollup merge of rust-lang#155808 - lapla-cogito:issue_155803, r=oli-obk
78363cf 
Always use `ConstFn` context for `const` closures

fixes rust-lang#155803

Since rust-lang@e8a4611, `hir_body_const_context()` returned the parent's const context for a `const` closure. But a `const` closure can escape its enclosing body via a `fn`-pointer-typed const item or an opaque return type and be invoked at runtime, so it must be const-checked under the most restrictive `ConstFn` context like a regular `const fn`.

r? oli-obk (since you authored the commit mentioned above, feel free to reroll)
@jhpratt jhpratt mentioned this pull request May 5, 2026
Closed
rust-bors Bot pushed a commit that referenced this pull request May 5, 2026
@bors
Auto merge of #156213 - jhpratt:rollup-CkSSHhL, r=jhpratt
4be9dc1 
Rollup of 12 pull requests

Successful merges:

 - #155341 (generic_const_args: allow paths to non type consts)
 - #156062 (Added command-line argument support for `wasm32-wali-linux-musl`)
 - #156159 ([AIX] add -bdbg:namedsects:ss link arg)
 - #156174 (Wasm: remove implicit `__heap_base`/`__data_end` exports)
 - #156186 (fix: remap ci-llvm debug paths via `-ffile-prefix-map`)
 - #156193 (port `rustc_ast*` crates from `box_` to `deref_patterns`)
 - #156201 (Don't run ui-fulldeps tests twice in stage 1)
 - #155808 (Always use `ConstFn` context for `const` closures)
 - #156105 (interpret: correctly deal with repr(transparent) enums)
 - #156148 (Use `all_impls` instead of handrolling it)
 - #156156 (Adjust getMCSubtargetInfo signature for LLVM 23+)
 - #156205 (move generalization test)
jhpratt added a commit to jhpratt/rust that referenced this pull request May 6, 2026
@jhpratt
Rollup merge of rust-lang#155808 - lapla-cogito:issue_155803, r=oli-obk
7eb0a28 
Always use `ConstFn` context for `const` closures

fixes rust-lang#155803

Since rust-lang@e8a4611, `hir_body_const_context()` returned the parent's const context for a `const` closure. But a `const` closure can escape its enclosing body via a `fn`-pointer-typed const item or an opaque return type and be invoked at runtime, so it must be const-checked under the most restrictive `ConstFn` context like a regular `const fn`.

r? oli-obk (since you authored the commit mentioned above, feel free to reroll)
@jhpratt jhpratt mentioned this pull request May 6, 2026
Open
rust-bors Bot pushed a commit that referenced this pull request May 6, 2026
@bors
Auto merge of #156217 - jhpratt:rollup-XVslat0, r=jhpratt
3f0e976 
Rollup of 15 pull requests

Successful merges:

 - #151122 (fix: more descriptive error message for enum to integer)
 - #155341 (generic_const_args: allow paths to non type consts)
 - #156062 (Added command-line argument support for `wasm32-wali-linux-musl`)
 - #156159 ([AIX] add -bdbg:namedsects:ss link arg)
 - #156174 (Wasm: remove implicit `__heap_base`/`__data_end` exports)
 - #156186 (fix: remap ci-llvm debug paths via `-ffile-prefix-map`)
 - #156193 (port `rustc_ast*` crates from `box_` to `deref_patterns`)
 - #156201 (Don't run ui-fulldeps tests twice in stage 1)
 - #155808 (Always use `ConstFn` context for `const` closures)
 - #156105 (interpret: correctly deal with repr(transparent) enums)
 - #156148 (Use `all_impls` instead of handrolling it)
 - #156156 (Adjust getMCSubtargetInfo signature for LLVM 23+)
 - #156170 (add known-bug test for coroutine 'static-yields-non-'static unsoundness (#144442))
 - #156195 (Move tests codegen)
 - #156205 (move generalization test)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oli-obk oli-obk oli-obk approved these changes

Assignees

@oli-obk oli-obk

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[ICE]: do not use optimized_mir for constants: Const { inline: false }

4 participants

@lapla-cogito @rustbot @oli-obk @rust-log-analyzer