-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Conversions from &mut T
to &Cell<T>
#1789
Conversation
Having this trait makes no real sense — I'd rather see |
True, the trait only exists to enable method call syntax - maybe thats too extravagant... |
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. |
How does this interact with the borrow checker? It seems to me that the point here is to get rid of the 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. |
@camlorn Casting |
Just for reference, there is a crate (by @huonw) providing this on For this kind of thing, where the interesting part is not the writing down of the |
Alright, I added an example of how it would be used, and switched over to using Also adding the lang team, since the main point here is whether the language can guarantee the cast to be correct. |
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 |
@camlorn Can you give an example of code you think won't work? If I transmute |
text/0000-as-cell.md
Outdated
let v_slice: &[Cell<T>] = Cell::from_mut_slice(&mut v); | ||
|
||
// example 1 | ||
for i in v_slice.iter() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: could just be for i in v_slice {
I think the
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 ( As an aside, it'd also analogously make sense to support |
I don't think That said, I do quite like |
The idea is that internal references ( Furthermore, given two references to the same memory, one typed as |
@eddyb 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? |
@camlorn I've used it in a constraint solver. A highly heterogenous UI tree contained I could've done all of that through a |
@eddyb |
@camlorn: 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());
}
}
|
@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 |
@Kimundi 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 |
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 |
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. |
AsCell
conversion from &mut T
to &Cell<T>
&mut T
to &Cell<T>
&mut T
to &Cell<T>
&mut T
to &Cell<T>
Agreed - I updated the RFC text a bit for a post- @rfcbot fcp merge |
Ah, I see... even with a fn truth_about_slices<T, F: FnOnce(&mut [T])>(x: &mut [T], f: F) {
let len = x.len();
f(x);
assert_eq!(x.len(), len);
} I was not aware of this, sorry for the noise. Well, actually the reason is even deeper than not changing the size... if I think about trait objects, it's not legal even to change the type to sth. else of the same size, right? The existential quantifier is, so to speak, outside of the reference, because it is literally stored on the "outside". Hm. |
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 😄) |
The final comment period is now complete. |
Huzzah! This RFC has been merged. Tracking issue. |
Question, why is this merged but slice::ref_slice is deprecated? Aren't these fundamentally fairly similar? |
@whitequark There is no slice <-> non-slice conversion here. I don't know why |
@eddyb I mean this comment by @alexcrichton:
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! |
@whitequark Then should it go to |
I don't think we have precedent for that and it seems like a really odd decision to me. |
Note that |
This proposal could have been done the same way and without even any new RFCs, just mark |
I think the libs team would be open to reconsidering |
For what it's worth, I still find uses for Most commonly, you have a function that takes an
Less commonly, you have an enum that sometimes has
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. |
I'd like to get it back in std. What's the process here? Is it an RFC or just a PR? |
@whitequark I think it's a small enough addition that a PR suffices. Feel free to CC me. |
This is late, but I have a concern about this RFC's design. To go from However, a natural extension to the proposed use case would be a thread safe variant: in particular, a way to go from |
@comex You also can't swap out the contents of a |
I was going to reply with something similar but I think their point was that there is no |
@edef1c Ah... I kind of forgot about that. It might be possible to support the full But in any case, as @glaebhoerl said, there is no |
Bring back slice::ref_slice as slice::from_ref. These functions were deprecated and removed in 1.5, but such simple functionality shouldn't require using unsafe code, and it isn't cluttering libstd too much. The original removal was quite contentious (see #27774), since then we've had precedent for including such nuggets of functionality (see rust-lang/rfcs#1789), and @nikomatsakis has provided a lot of use cases in rust-lang/rfcs#1789 (comment). Hence this PR. I'm not too sure what to do with stability, feel free to correct me. It seems pointless to go through stabilization for these functions though. cc @aturon
A proposal about legitimizing the cast of a
&mut T
to a&Cell<T>
.Rendered