Skip to content

make RefCell unstably const #137843

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
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

make RefCell unstably const #137843

wants to merge 1 commit into from
+154 −42

Conversation

Daniel-Aaron-Bloom
Copy link

@Daniel-Aaron-Bloom Daniel-Aaron-Bloom commented Mar 1, 2025 •
edited
Loading

Now that we can do interior mutability in const, most of the RefCell API can be const fn. The main exceptions are APIs which use FnOnce (RefCell::replace_with and Ref[Mut]::[filter_]map[_split]) and RefCell::take which calls Default::default.

Tracking issue: #137844

@rustbot
Copy link
Collaborator

rustbot commented Mar 1, 2025

r? @Amanieu

rustbot has assigned @Amanieu.
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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 1, 2025
@Daniel-Aaron-Bloom Daniel-Aaron-Bloom mentioned this pull request Mar 1, 2025
Open
3 tasks
@Daniel-Aaron-Bloom Daniel-Aaron-Bloom changed the title Draft: make Cell more unstably const make RefCell more unstably const Mar 1, 2025
@Daniel-Aaron-Bloom
Copy link
Author

@rustbot label -T-libs +T-libs-api
r? libs-api

@Daniel-Aaron-Bloom Daniel-Aaron-Bloom marked this pull request as ready for review March 1, 2025 07:29
@rustbot rustbot added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 1, 2025
@rustbot rustbot assigned joshtriplett and unassigned Amanieu Mar 1, 2025
@Daniel-Aaron-Bloom Daniel-Aaron-Bloom changed the title make RefCell more unstably const make RefCell unstably const Mar 1, 2025
@Daniel-Aaron-Bloom Daniel-Aaron-Bloom changed the title make RefCell unstably const make RefCell unstably const Mar 1, 2025
@Daniel-Aaron-Bloom Daniel-Aaron-Bloom changed the title make RefCell unstably const make RefCell unstably const Mar 1, 2025
@tgross35
Copy link
Contributor

tgross35 commented Mar 2, 2025

Cc @rust-lang/wg-const-eval

@RalfJung
Copy link
Member

RalfJung commented Mar 3, 2025

The main problem is dropping. Please make sure to also add a test actually using a RefCell; that will probably not work since the Ref/RefMut Drop impls are not const.

RalfJung
RalfJung reviewed Mar 3, 2025
View reviewed changes
@RalfJung
Copy link
Member

RalfJung commented Mar 3, 2025

@compiler-errors what is the state of Drop in const? Can one impl const Drop and it will "just work"? Are const Destruct bounds ready for use in std (for APIs that are stable but const-unstable)?

@traviscross
Copy link
Contributor

Since we probably all want to hear the answer to that question...

cc @rust-lang/lang

@oli-obk
Copy link
Contributor

oli-obk commented Mar 3, 2025 •
edited
Loading

Are const Destruct bounds ready for use in std (for APIs that are stable but const-unstable)?

No. Until we know what const traits syntax to implement, adding more const trait impls is not something we want. The libstd churn is not worth it

@oli-obk
Copy link
Contributor

oli-obk commented Mar 3, 2025

Can one impl const Drop and it will "just work"?

Yes

@RalfJung
Copy link
Member

RalfJung commented Mar 3, 2025 •
edited
Loading

@Daniel-Aaron-Bloom okay so please remove the fn const_drop hack and instead try impl const Drop for BorrowRef and BorrowRefMut. If that does not work, then I think it is better to wait until that works; nobody will be able to use RefCell in const otherwise anyway (not even on nightly).

@Daniel-Aaron-Bloom
Copy link
Author

Should I also remove try to remove my added private function RefMut::as_mut and use const_deref instead?

@RalfJung
Copy link
Member

RalfJung commented Mar 3, 2025

Oh that's what those functions are for?

Sure, seems fine to me as long no incomplete nightly features have to be enabled for this.

RalfJung
RalfJung reviewed Mar 3, 2025
View reviewed changes
RalfJung
RalfJung reviewed Mar 3, 2025
View reviewed changes
RalfJung
RalfJung reviewed Mar 3, 2025
View reviewed changes
RalfJung
RalfJung reviewed Mar 3, 2025
View reviewed changes
RalfJung
RalfJung reviewed Mar 3, 2025
View reviewed changes
@oli-obk oli-obk added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Mar 4, 2025
RalfJung
RalfJung reviewed Mar 4, 2025
View reviewed changes
@rustbot rustbot added has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 27, 2025
@rustbot

This comment has been minimized.

Sign in to view
@rustbot rustbot removed has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 27, 2025
@bors
Copy link
Collaborator

bors commented May 5, 2025

☔ The latest upstream changes (presumably #140646) made this pull request unmergeable. Please resolve the merge conflicts.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors
Copy link
Collaborator

bors commented Jun 1, 2025

☔ The latest upstream changes (presumably #141842) made this pull request unmergeable. Please resolve the merge conflicts.

@oli-obk oli-obk removed the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Jun 2, 2025
@oli-obk
Copy link
Contributor

oli-obk commented Jun 2, 2025

With #119899 this is unblocked

tgross35
tgross35 reviewed Jun 2, 2025
View reviewed changes
oli-obk
oli-obk reviewed Jun 3, 2025
View reviewed changes
library/core/src/cell.rs
Some(BorrowRef { borrow })
}
}

/// FIXME(const-hack): `Clone` is not a `const_trait`, so work around that by making our own method
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's do that in a separate PR first.

Copy link
Author

Choose a reason for hiding this comment

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

Apologies, I don't quite understand. Are you saying to move this const version of clone into its own PR? It depends on a good number of other methods being const, so I can do that but I'm not sure it's the cleanest approach.

Copy link
Author

Choose a reason for hiding this comment

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

Or are you saying to add const_trait to Clone in a separate PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

yea, Let's make Clone const before landing this PR, then this PR doesn't need a const hack

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

@RalfJung RalfJung RalfJung left review comments

@oli-obk oli-obk oli-obk left review comments

@tgross35 tgross35 tgross35 left review comments

Assignees

@joshtriplett joshtriplett

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs-api Relevant to the library API 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.
10 participants
@Daniel-Aaron-Bloom @rustbot @tgross35 @RalfJung @traviscross @oli-obk @bors @rust-log-analyzer @joshtriplett @Amanieu