-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
RFC: Introduce Mut<T>, remove Cell<T> #9429
Conversation
Replace the common Cell idiom with Mut<T> before: let upvar_cell = Cell::new(x); do spawn { let xyz = upvar_cell.take().recv() } after: let upvar_cell = Mut::new_some(x) do spawn { let xyz = upvar_cell.take_unwrap().recv() }
I'm personally a fan of this, it looks pretty awesome. Here's some thoughts:
I don't entirely remember why Nice work, though! |
Thank you for the direct points!
I'll explain why I think this is impossible. The lifetime relations are crucial.
The
The derived borrowed pointer can maximally live as long as the
I'm not sure if this is possible! The
The Cell type is/was rarely named at all, anywhere; It's used in stack values, and so the choice of constructor function: |
If (But this simplification is harder to use/more restricted, especially when used together with Rc.) |
Isn't this just wrong? From the manual:
|
The lack of chaining is #3511 (which I am in the process of fixing...) In general, I like this PR, but this is definitely something that should be discussed in meeting before merging (as the RFC title suggests). |
Remove Mut from the prelude, it should not be there.
@glehel good question, I can't answer and haven't been able to find out. Rustc encodes logic for inheriting Freeze where even |
This needs a rebase. |
Today's meeting didn't seem to dicuss this, but the closure changes are of big importance: If |
I'm personally in favor of this, but like composition I feel like this warrants more discussion. I would love to remove as many users of Cell as possible, I need to get up to speed on the function changes... Regardless, I think that |
It's not actually possible to replace |
It looks like 71acc54 fixes the |
Closing due to lack of activity, but feel free to reopen if this gets rebased! |
Based off of blake2-ppc's work in rust-lang#9429.
This is based off of @blake2-ppc's work on #9429. That PR bitrotted and I haven't been able to contact the original author so I decided to take up the cause. Overview ====== `Mut` encapsulates a mutable, non-nullable slot. The `Cell` type is currently used to do this, but `Cell` is much more commonly used as a workaround for the inability to move values into non-once functions. `Mut` provides a more robust API. `Mut` duplicates the semantics of borrowed pointers with enforcement at runtime instead of compile time. ```rust let x = Mut::new(0); { // make some immutable borrows let p = x.borrow(); let y = *p.get() + 10; // multiple immutable borrows are allowed simultaneously let p2 = x.borrow(); // this would throw a runtime failure // let p_mut = x.borrow_mut(); } // now we can mutably borrow let p = x.borrow_mut(); *p.get() = 10; ``` `borrow` returns a `Ref` type and `borrow_mut` returns a `RefMut` type, both of which are simple smart pointer types with a single method, `get`, which returns a reference to the wrapped data. This also allows `RcMut<T>` to be deleted, as it can be replaced with `Rc<Mut<T>>`. Changes ====== I've done things a little bit differently than the original proposal. * I've added `try_borrow` and `try_borrow_mut` methods that return `Option<Ref<T>>` and `Option<RefMut<T>>` respectively instead of failing on a borrow check failure. I'm not totally sure when that'd be useful, but I don't see any reason to not put them in and @cmr requested them. * `ReadPtr` and `WritePtr` have been renamed to `Ref` and `RefMut` respectively, as `Ref` is to `ref foo` and `RefMut` is to `ref mut foo` as `Mut` is to `mut foo`. * `get` on `MutRef` now takes `&self` instead of `&mut self` for consistency with `&mut`. As @alexcrichton pointed, out this violates soundness by allowing aliasing `&mut` references. * `Cell` is being left as is. It solves a different problem than `Mut` is designed to solve. * There are no longer methods implemented for `Mut<Option<T>>`. Since `Cell` isn't going away, there's less of a need for these, and I didn't feel like they provided a huge benefit, especially as that kind of `impl` is very uncommon in the standard library. Open Questions ============ * `Cell` should now be used exclusively for movement into closures. Should this be enforced by reducing its API to `new` and `take`? It seems like this use case will be completely going away once the transition to `proc` and co. finishes. * Should there be `try_map` and `try_map_mut` methods along with `map` and `map_mut`?
Make `derivable_impls` machine applicable changelog: [`derivable_impls`]: Now machine applicable
Mut<T>
implements an internally mutable slot, using#[no_freeze]
andtransmute_mut
. It is much like Cell without the nullable part. It implements dynamic borrow checking, which is just an enum counting the current readers and writers.Mut<T>
provides methods.borrow() -> ReadPtr<'a, T>
and.borrow_mut() -> WritePtr<'a, T>
to obtain a wrapped borrowed pointer. These wrapped pointers have destructors that update the borrow flag in theMut<T>
when they go out of scope. SeveralReadPtr
can be alive at the same time.Example:
For convenience, and to remove Cell, extra methods are implemented on
Mut<Option<U>>
that make it easy to treat it like an optional value. When replacing Cell's.take()
, the only overhead ofMut<T>
at the moment is the destructor function call ofutil::NonCopyable
that's inserted.Mut<T>
is more generic thanCell<T>
, and so can replace it better when it is used as an actual mutable slot. This represents very few previous users of Cell in the tree (see commit https://github.com/blake2-ppc/rust/commit/4f8c5b1954aef4e01c40935708757373d9b04b6f )Mut<T>
is also used to replaceextra::rc::RcMut<T>
withRc<Mut<T>>
. This requires a further restriction for cycle prevention (unless I'm mistaken) so that onlyT: Send
values are allowed insideRc<Mut<T>>
. (This is becauseRc<Mut<T>>
isFreeze
, whileRcMut<T>
was not.)Clarifications and Known issues:
BorrowFlag
enum is 16 bytes and is too large. This can be solved inelegantly by simply using an integer.Mut<T>
should be noncopyable because it needs to clear the borrow status when cloned.Mutable<T>
was not possible since it conflicts withstd::container::Mutable
Requests for comment:
.take()
? This represents the use case where closures wish to modify their moved-in upvalues.impl<T> Mut<T>
andimpl<U> Mut<Option<U>>
?Mut<T>
allow multipleReadPtr
(and thus it has to count them)?