Skip to content

Revert const hacks and use const closures in std#155957

Open
oli-obk wants to merge 2 commits intorust-lang:mainfrom
oli-obk:const-closure-std
Open

Revert const hacks and use const closures in std#155957
oli-obk wants to merge 2 commits intorust-lang:mainfrom
oli-obk:const-closure-std

Conversation

@oli-obk
Copy link
Copy Markdown
Contributor

@oli-obk oli-obk commented Apr 29, 2026

This revealed some smaller bugs in stability checking that I fixed where needed:

  • const closures use the const stability of their parent
  • trait method default bodies use the const stability of their trait

Otherwise trivial reverts of the const hacks that were added

fixes #155781

@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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 29, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 29, 2026

r? @jhpratt

rustbot has assigned @jhpratt.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: @scottmcm, libs
  • @scottmcm, libs expanded to 8 candidates
  • Random selection from Mark-Simulacrum, jhpratt, nia-e, scottmcm

Copy link
Copy Markdown
Member

@jhpratt jhpratt left a comment

Choose a reason for hiding this comment

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

r=me from a libs side with the one question resolved. I'm not a compiler reviewer, but the small change is obviously correct.

View changes since this review

/// Not currently planned to be exposed publicly, so just `pub(crate)`.
#[repr(transparent)]
pub(crate) struct NeverShortCircuit<T>(pub T);
// FIXME(const-hack): replace with `|a| NeverShortCircuit(f(a))` when const closures added.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are these simply not needed any more?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yeah, they were only for wrap_mut_1. but, i think the wrap_mut_1 function should still exist? it wasnt added as a const hack.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

hmm... I guess we could keep it and return a const closure from it, but is it really worth it?

Copy link
Copy Markdown
Contributor

@bend-n bend-n Apr 29, 2026

Choose a reason for hiding this comment

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

i mean i think so.. thats what was done pre const hack at least, and i dont think it was a bad design?
the fixme i wrote on the const-hack was intended to be put in wrap_mut_1

@rust-log-analyzer

This comment has been minimized.

@theemathas
Copy link
Copy Markdown
Contributor

Does this fix #155781?

@jhpratt
Copy link
Copy Markdown
Member

jhpratt commented Apr 29, 2026

I'm pretty sure it will

F: [const] FnMut(usize) -> T + [const] Destruct,
{
try_from_fn(NeverShortCircuit::wrap_mut_1(f)).0
try_from_fn(const move |a| NeverShortCircuit(f(a))).0
Copy link
Copy Markdown
Contributor Author

@oli-obk oli-obk Apr 29, 2026

Choose a reason for hiding this comment

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

Yea, the Wrapped type was only used in these two cases

View changes since the review

debug_assert!(elem_layout.size() == elem_layout.pad_to_align().size());

// FIXME(const-hack) return to using `map` and `map_err` once `const_closures` is implemented
match elem_layout.repeat_packed(cap) {
Copy link
Copy Markdown
Contributor

@bend-n bend-n Apr 29, 2026

Choose a reason for hiding this comment

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

why isn't it repeat_packed?

View changes since the review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think this was a very recent change and I messed up a rebase over it, fixed

@oli-obk oli-obk force-pushed the const-closure-std branch from 911b6a6 to 9b213cb Compare April 29, 2026 16:23
@oli-obk oli-obk force-pushed the const-closure-std branch from 9b213cb to 3b0794b Compare April 29, 2026 16:25
@oli-obk
Copy link
Copy Markdown
Contributor Author

oli-obk commented Apr 29, 2026

Does this fix #155781?

oh. yes! I ran into this issue but didn't realize there was an issue open for it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PG-const-traits Project group: Const traits 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. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Const closures inside const_unstable functions are claimed to be exposed to stable

7 participants