Conversions from `&mut T` to `&Cell<T>`

[Kimundi]

A proposal about legitimizing the cast of a &mut T to a &Cell<T>.

Rendered

[edef1c]

Having this trait makes no real sense — I'd rather see Cell::from_mut and Cell::from_mut_slice than a trait that covers two semantically incompatible conversions. Having a trait only makes sense when one can meaningfully abstract over its implementors.

[Kimundi]

True, the trait only exists to enable method call syntax - maybe thats too extravagant...

[edef1c]

Making this usable with method call syntax only serves to further obscure what's going on, and this whole proposal is already designed to do non-obvious (but very useful) things by its very nature.

[camlorn]

How does this interact with the borrow checker? It seems to me that the point here is to get rid of the &mut so that you can do things with the Vec again, but if you're already iterating over the Vec via iter_mut, our current borrow checker isn't smart enough to let you. At least, in so far as I know.

Can you add an example to the RFC that shows usage of the trait? You've only got one that shows how it's currently a problem and one that shows how I can make it work in current Rust. I would like to see this so I can conceptualize it, because I really feel like the current borrow checker brings this crashing down.

[edef1c]

@camlorn Casting &mut T to &Cell<T> works today — this RFC merely proposes adding it to the stdlib. Concurrently obtaining another &mut T would be undefined behaviour, however.
Iterating and mutating concurrently can be done by converting an &mut [T] into &[Cell<T>] and then iterating over the cells (with iter, not iter_mut — that would create aliased mutable references)

[glaebhoerl]

Just for reference, there is a crate (by @huonw) providing this on crates.io. (See also #1106)

For this kind of thing, where the interesting part is not the writing down of the transmutes, but the clauses implicit in the language definition which make them OK, I do think it makes more sense to provide these directly in std than in an external crate. In other words my feeling is that, like Cell itself and Rc and others, these are essentially language primitives (part of the "trusted code base") that we're choosing to make available as std types and functions instead of as built-in syntax, for aesthetic reasons or whatever.

[Kimundi]

Alright, I added an example of how it would be used, and switched over to using Cell constructors instead.

Also adding the lang team, since the main point here is whether the language can guarantee the cast to be correct.

[camlorn]

I see it now. I focused in on the first conversion and missed the second.

What is the use case of the first conversion? This RFC is all about iteration, but I'm not seeing how &mut T->Cell<T> helps. I think it will in the future, but not until the borrow checker is no longer based on blocks. Is the future the point?

[eddyb]

@camlorn Can you give an example of code you think won't work? If I transmute &mut T to &Cell<T> I can make as many copies as I want, and the compiler will treat the mutable borrow as expiring when the last &Cell<T> goes away (each lexically scoped, of course).

[oli-obk]

nit: could just be for i in v_slice {

[glaebhoerl]

I think the &mut [T] -> &[Cell<T>] conversion (from_mut_slice) could be further decomposed into:

1. &mut [T] -> &Cell<[T]> (Cell<[T]>::from_mut)
2. &Cell<[T]> -> &[Cell<T>] -- perhaps using an Index impl?

So the API could look like this:

impl<T: ?Sized> Cell<T> {
    fn from_mut(val: &mut T) -> &Cell<T>;
}

impl<T> Index<usize> for Cell<[T]> {
    type Output = Cell<T>;
    fn index(&self, idx: usize) -> &Cell<T>;
}

impl<T> Index<Range<usize>> for Cell<[T]> {
    type Output = [Cell<T>];
    fn index(&self, idx: Range<usize>) -> &[Cell<T>];
}

impl<T> Index<RangeFrom<usize>> for Cell<[T]> {
    type Output = [Cell<T>];
    fn index(&self, idx: RangeFrom<usize>) -> &[Cell<T>];
}

impl<T> Index<RangeTo<usize>> for Cell<[T]> {
    type Output = [Cell<T>];
    fn index(&self, idx: RangeTo<usize>) -> &[Cell<T>];
}

impl<T> Index<RangeFull> for Cell<[T]> {
    type Output = [Cell<T>];
    fn index(&self, idx: RangeFull) -> &[Cell<T>];
}

So given a cell of a slice (&Cell<[T]>), you could index it however you like and get cells or slices of cells (&[Cell<T>]).

As an aside, it'd also analogously make sense to support &Cell<(A, B)> -> &(Cell<A>, Cell<B>), and likewise for tuples of other sizes and structs. It might be possible to express the former in the general case with #1582, the latter would almost certainly need compiler support. (No need to have these as part of this RFC, just mentioning it for the record.)

[eddyb]

I don't think &Cell<[T]> -> &[Cell<T>] is sound, it violates the no interior references principle, although I'm not sure I can prove any problematic interactions. You'd need code that works on Cell<[T]> and doesn't take the existence of the transformation into account.

That said, I do quite like xs[i].set(x), there's a nice aesthetic to it.

[glaebhoerl]

The idea is that internal references (&) together with shared mutability (Cell) is safe and sound as long as the structure is "flat": the references don't go under indirections (e.g. Box) or branches (enums). That means there is no way for the outer references to invalidate the memory observed by the inner references, because mutation cannot change the structure (because there is only one possible structure), only the data.

Furthermore, given two references to the same memory, one typed as &Cell<(X, Y, Z)>, and the other as &(Cell<X>, Cell<Y>, Cell<Z>), doing cell.set((x, y, z)) with the first is completely equivalent to doing cell.0.set(x); cell.1.set(y); cell.2.set(z) with the second, which is some kind of algebraic property I'm sure. (The same principle applies to arrays (slices), it's just nicer to illustrate with tuples.)

[camlorn]

@eddyb
it's hard for me to come up with code here because I don't understand the proposed use case of the first conversion. The problem as I see it is that the lifetime of the &Cell<T> cannot be larger than the lifetime of the &mut T, consequently you still have the &mut T in the way of all the interesting manipulations you might want to do.

Let me put it this way instead: can someone show me why it's useful, and then I'll come back with an objection if I still have one?

[eddyb]

@camlorn I've used it in a constraint solver. A highly heterogenous UI tree contained f32s indicating coordinates on screen, scattered inside it. They were mutably traversed and &mut f32 was turned into &Cell<f32>. Now once I had the &Cell<f32> I could copy it many types and use it as a "variable reference" in constraints, and every single one of them could update the original if they wanted to.

I could've done all of that through a Vec<&mut f32> and integer indices though, so maybe not the best example? It just felt cleaner, at least that part (you really don't want to read the actual "solver").

[camlorn]

@eddyb
Ah, I kind of see. It sounds like the kind of thing where it's not useful save in larger programs. That seems workable, though, yes.

[Kimundi]

@camlorn: &mut T -> &Cell<T> is also the general approach you can use if you just have a iterator that yields you &mut T references, as oppose of a slice directly.

let mut set: HashSet<i32> = ...;

let tmp: Vec<&Cell<i32>> = set.iter_mut().map(Cell::from_mut).collect();
for i in &tmp {
    for j in &tmp {
        i.set(j.get() + i.get());
    }
}

[camlorn]

@Kimundi
Oh. That's clever. Though I will say that I don't like the temporary Vec.

[Kimundi]

Sure, but the need for the vector is kind of independent to this feature, and only caused by the need to have random access to all elements.

Eg, the &Cell<i32>'s might just as well have been picked up by a consumer of the iterator directly and stashed away in whatever data structure works with them.

[camlorn]

@Kimundi
I know. I still don't like the need for temporary intermediate data structures.

One of the problems of Rust (orthogonal to this RFC) is that some things like this suddenly require you to collect into a vec, which is fine until the container is large. Other such scenarios include deleting multiple keys from a map based on a predicate. The solution to this problem is for me to write up the extensions I think we should have and put them in an RFC, as I think most of them would be accepted without much argument.

Nevertheless, every time I see something that ends in .collect::<Vec<_>>() just because it's Rust and not C++, I get a little sad inside.

[bluss]

Collecting into a vec is a workaround for when a more specific solution do not exist yet, for example an (random access?) iterator where each iterator element is a &Cell<T>. The canonicalization of this conversion does pave the way for such iterators to be written.

[Havvy]

No discussion has been going on in this RFC for almost a month. I'd nominate for FCP, but I don't know think I can.

[Kimundi]

Agreed - I updated the RFC text a bit for a post-T: Copy world, but so far I haven't seen any actual objection, so I'm putting this into fcp. :)

@rfcbot fcp merge

[Kimundi]

Glad to see this RFC received so much formal scrutiny. :)

@nrc / @aturon: Sorry, I totally forgot to address your comments. I've pushed updated wordings.

@pnkfelix: Yeah, the slice conversion is independent to the core proposal, and weird in its own way. If there where major objections I'd be happy to remove it from this RFC, but apparently no one sees an actual issue with it. (notwithstanding @RalfJung's confusion 😄)

[rfcbot]

The final comment period is now complete.

[aturon]

Huzzah! This RFC has been merged. Tracking issue.

[whitequark]

Question, why is this merged but slice::ref_slice is deprecated? Aren't these fundamentally fairly similar?

[eddyb]

@whitequark There is no slice <-> non-slice conversion here. I don't know why ref_slice is deprecated, it's really useful in quite a few cases.

[whitequark]

@eddyb I mean this comment by @alexcrichton:

The libs team discussed this during triage today and the decision was to deprecate these functions. The standard library currently doesn't house many nuggets of functionality like this (e.g. see the recent discussions around Box::leak), and we've also deprecated similar functionality (Option::as_slice).

While certainly useful from time to time, it's not clear that these functions belong in the standard library right now. There can always be a helper crate implementing all sorts of conversions like this, however!

This seems to me to fall right into the "little nugget of functionality" area. I would rather we get ref_slice back, it's pretty useful in embedded contexts, if you have no Box!

[mzji]

@whitequark Then should it go to libcore? These useful functions could be a part of core, just won't be exported by std, thus, they will available in #[no_std] but not in the "normal" crates.

[whitequark]

I don't think we have precedent for that and it seems like a really odd decision to me.

[edef1c]

Note that ref_slice is available as a Cargo crate, should you need it.
It doesn't require any additional unsafe guarantees from the compiler, so it is fine to implement outside the stdlib.

[whitequark]

It doesn't require any additional unsafe guarantees from the compiler, so it is fine to implement outside the stdlib.

This proposal could have been done the same way and without even any new RFCs, just mark Cell as #[repr(transparent)] I think.

[aturon]

I think the libs team would be open to reconsidering ref_slice (which was deprecated quite a long time ago), if there are more documented use-cases.

[nikomatsakis]

For what it's worth, I still find uses for ref_slice relatively frequently. Luckily, in the compiler, we have a copy available. Here is a quick analysis of the various uses in the compiler. As one would expect, there are two main patterns.

Most commonly, you have a function that takes an &[T], and you have a &T. Here are the cases that seem to be like that:

src/librustc/middle/resolve_lifetime.rs:431:            self.resolve_elided_lifetimes(slice::ref_slice(lifetime_ref));
src/librustc_const_eval/check_match.rs:100:        self.check_patterns(false, slice::ref_slice(&loc.pat));
src/librustc_const_eval/check_match.rs:108:            self.check_patterns(false, slice::ref_slice(&arg.pat));
src/librustc_lint/unused.rs:93:                self.check_unused_mut_pat(cx, slice::ref_slice(&l.pat));
src/librustc_lint/unused.rs:106:            self.check_unused_mut_pat(cx, slice::ref_slice(&a.pat));
src/librustc_typeck/astconv.rs:850:        self.prohibit_type_params(slice::ref_slice(item_segment));
src/librustc_typeck/astconv.rs:925:        self.prohibit_type_params(slice::ref_slice(item_segment));
src/librustc_typeck/check/mod.rs:3987:        (def, Some(ty), slice::ref_slice(&**item_segment))
src/librustc_typeck/check/mod.rs:4119:                Some(e) => ref_slice(e),

Less commonly, you have an enum that sometimes has T and sometimes Vec<T>, and you want to return a &[T]. This occurs primarily in the MIR, except that we are returning Cow<[T]>:

src/librustc/mir/mod.rs:579:            Goto { target: ref b } => slice::ref_slice(b).into_cow(),
src/librustc/mir/mod.rs:586:                slice::ref_slice(t).into_cow(),
src/librustc/mir/mod.rs:587:            Call { destination: None, cleanup: Some(ref c), .. } => slice::ref_slice(c).into_cow(),
src/librustc/mir/mod.rs:595:                slice::ref_slice(target).into_cow()
src/librustc/mir/mod.rs:598:            Assert { ref target, .. } => slice::ref_slice(target).into_cow(),

I continue to think this function is a natural candidate for the standard library. It's a bit of unsafe code that is useful as a bit of glue in various scenarios and absolutely no dependencies or runtime implications. It is not entirely obvious that it is safe -- in particular, it requires understanding Rust's memory layout rules (which of course are not entirely defined). So putting it in the standard library is a way for us to advertise that indeed it is safe. =) It's also something that people might not realize is possible, and hence they wouldn't know to search for the crate.

It's also ok for it to exist in a crate, of course, but I don't think there's any good reason to put it there.

[whitequark]

I'd like to get it back in std. What's the process here? Is it an RFC or just a PR?

[aturon]

@whitequark I think it's a small enough addition that a PR suffices. Feel free to CC me.

[comex]

This is late, but I have a concern about this RFC's design. To go from &mut [T] to &[Cell<T>] it requires going through &Cell<[T]>. That's a cute decomposition and works fine - for Cell.

However, a natural extension to the proposed use case would be a thread safe variant: in particular, a way to go from &mut u8 to &AtomicU8 and from &mut [u8] to &[AtomicU8], and similarly for other atomic types. That would be its own RFC of course, but notably, the API design couldn't directly mirror this one; there would have to be a direct conversion function/method from &mut [u8] to &[AtomicU8], since there's no type like &Atomic<[u8]> to serve as an intermediary (after all, you can't atomically mutate arbitrarily large slices). For the sake of consistency, therefore, it might make sense to revise this RFC to add a direct conversion from &mut [T] to &[Cell<T>].

[edef1c]

@comex You also can't swap out the contents of a Cell<[T]> with another [T] freely, so that seems analogous?

[glaebhoerl]

I was going to reply with something similar but I think their point was that there is no Atomic<T> type in the first place that you could instantiate to Atomic<[T]>.

[comex]

@edef1c Ah... I kind of forgot about that. It might be possible to support the full Cell API on Cell<[T]> in the future with stack-based DSTs. But if that doesn't happen / isn't desired, it only strengthens my belief that the design should be changed. If Cell<[T]> does not provide any of the normal Cell functionality, what's the point of having it? It's not 'modular' to go through an intermediary type if the intermediary type is completely useless - other than for the purpose of being an intermediary for that exact conversion.

But in any case, as @glaebhoerl said, there is no Atomic<T>. Perhaps there should be one in the future, but it would be quite strange to have an Atomic<T> that can't actually be read/written atomically...