-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
split Index into Index, IndexMut and IndexAssign #6515
Comments
Relative of #5992 -- dupe? |
@graydon: not quite, that's more about adding in-place versions of the binary operators with a mention of index-assign |
How about |
@chris-morgan I imagine that would be handled by |
I'll mention the role of Edit: To give the traits purpose, for |
cc me, I am working on overloading deref and so on, and this is related. I'll put in the specifics of my proposal in a bit. |
To move vec builders into the library we need overloadable index to be working correctly. Nominating. |
Accepted for 1.0, P-high. |
To be concrete, I think it should work like this. // self[element] -- if used as rvalue, implicitly a deref of the result
trait Index<E,R> {
fn index<'a>(&'a self, element: &E) -> &'a R;
}
// &mut self[element] -- when used as a mutable lvalue
trait IndexMut<E,R> {
fn index_mut<'a>(&'a mut self, element: &E) -> &'a mut R;
}
// self[element] = value
trait IndexSet<E,V> {
fn index_set(&mut self, element: E, value: V);
} These specifications make one crucial assumption: that the implementer can return a reference to the result. This is suitable for many if not all uses cases, but it disallows e.g. an implementer that was going to synthesize the result. An alternative is to add a fourth trait, corresponding to // self[element] in an rvalue context
trait IndexGet<E,R> {
fn index_get(&self, element: &E) -> R;
} which would be intended for those cases where I am more-or-less indifferent as to which of these would be best. |
@nikomatsakis: I think it would be fine to reserve the syntax for in-memory data structures able to return a reference. As long as it's possible to implement what slices do today in a user-defined type, I'll be happy! |
@nikomatsakis this looks fine as a starting point. I think I want this as well: // self[element] = value
trait IndexSetRef<E,V> {
fn index_set(&mut self, element: &E, value: V);
} I'm happy to pick up the work for your three traits and see how far I get. |
I think it's plausible to have various set traits so long as a type implements at most one (or rather we try them in a specific order, so there's no point in implementing more than one) |
I am happy to mentor this bug fyi |
With respect to PR #11977, @wycats and I talked privately over IRC some time ago. This current patch doesn't implement the desired semantics. For example, if |
I've given some more thought to this and I'm not sure that the |
See comments here: Most of this applies equally to index. The only difference is the |
With the new |
I'd appreciate the ability to return things by value, personally. It allows for some nice sugar: |
Nominating for backcompat-lang since this will change the defn. of the Index trait. |
Does this need a real RFC? |
This is RFC #111. |
This will break code that used the old `Index` trait. Change this code to use the new `Index` traits. For reference, here are their signatures: pub trait Index<Index,Result> { fn index<'a>(&'a self, index: &Index) -> &'a Result; } pub trait IndexMut<Index,Result> { fn index_mut<'a>(&'a mut self, index: &Index) -> &'a mut Result; } Closes #6515. [breaking-change] r? @nick29581
[beta] Backport of rust-lang#6375 - field_reassign_with_default fix With the pinned nightly we can test backports to our beta branch now 🎉 cc rust-lang#6515 changelog: beta 1.50: Backport of private fields fix in [`field_reassign_with_default`] lints
The current
Index
isn't great because you have to use&self
and return&T
orT
(via clone) for a map/vector type.I think it would be best to split it into
Index
(with&self
),IndexMut
(with&mut self
) andIndexAssign
(&mut self
, inserting a value).I'm unsure about the syntax. It would be nice to make it uniform with vector syntax and have
&a["foo"]
callIndex
for&T
, etc.The text was updated successfully, but these errors were encountered: