Implement Index and IndexMut for Vec

[nrc]

No description provided.

[huonw]

This should probably be blocked on #15525, or else we'll risk some surprising segfaults and invalid code.

[erickt]

It'd be nice to talk about the plan for what happens with Vec::get and Vec::get_mut. Originally I thought we should deprecate these methods, but WalrusPony on #rust-internals suggested instead that we could change the old methods return Option<&T>. That would let us right fail-free Vec code.

[emberian]

Yes, get and get_mut should return to their proper Option<&T> status.

[alexcrichton]

There are many methods on slice that aren't currently available on Vec, awaiting further judgement for ergonomics. I would expect this to deprecate get and get_mut on Vec to assist in transitioning to foo[T] syntax.

The verdict about the ergonomics of slice/Vec types would decide how the get and get_mut methods on slices would be accessed. It would be nice to reduce duplication for now.

[retep998]

So perhaps deprecate get and get_mut for now, then later remove them, and then even later re-add them with Option<&T> if we decide to duplicate that sort of functionality between Vec and &[T].

[lilyball]

Assuming Vec ends up implementing Deref<[T]> and DerefMut<[T]> post-DST, then I think it's reasonable to remove all Vec methods that it would inherit from [T]. get/get_mut are part of that.

But in the meantime I'm in favor of deprecating get/get_mut along side the implementation of the indexing traits.

[nrc]

So I tried deprecating get and get_mut and there are a lot of warnings when building rustc and errors in librustrt (and presumably other crates which deny deprecated). So is it worth deprecating now or waiting and doing and a changeover from get to indexing in rustc?

[huonw]

Can they be changed in this PR? (Doing it in a separate commit would be nice, if it does work.)

[nrc]

Not very easily - it would mean using cfgs for 10s or 100s of methods, even then I'm not sure if you can gate #deprecated on a cfg, so you would still get warnings. Much easier to wait for a snapshot.

[nrc]

#15525 has a fix and doing anything for get/get_mut now seems impractical. So, r?

[nrc]

Filed #15672 as a follow up to deprecate get and get_mut

[alexcrichton]

This shouldn't need to wait for a snapshot, everything should be able to move immediately to the new syntax, what were the roadblocks you encountered?

[nrc]

I changed the first error I saw in librustrt and that gave me a compile error when doing the stage0 build because indexing for Vecs is not supported. Perhaps the new indexing work in the compiler hasn't made it to the snapshot and that is what we need?

error: cannot index a value of type &mut collections::vec::Vec<core::option::Option<(*const u8,Box<local_data::LocalData:Send>,uint)>>

[huonw]

Maybe that isn't derefing, and so needs (*v)[i]? (eww)

[nrc]

Yeah, on second glance it looks like it might be a problem like that - I tried changing another use of get and it seems to be OK

[nrc]

And it turns out sometimes you want a &mut ref to pass somewhere rather than do an immediate assignment, so maybe we can't deprecate get_mut in any case.

[huonw]

I think that's handled by &mut v[i]?

[nrc]

That won't deref v[i] to a value then re-ref it?

[huonw]

No, it shouldn't. (Otherwise vector indexing would be useless for non-Copy types.)

[lilyball]

&mut v[i] is the expected way to get a &mut reference to an indexed element. You can consider it roughly equivalent to &mut *v.index_mut(&i) if you want, which is perfectly valid (and doesn't cause any issue with non-Copy types).

[nrc]

So, that might all be moot - #12825 is a problem here too - the compiler prefers IndexMut to Index which causes problems if the vec is not mutable. So, I think I will just implement Index and leave get_mut for now.

[alexcrichton]

I believe that no matter what we will want these implementations in any future world basically exactly as-is, so I'm going to r+ this. We can work on the deprecation of get and get_mut in a later PR as it sounds like it's nontrivial.

[nrc]

@alexcrichton I believe it might be better for now, not to implement IndexMut - it prevents Index working. In that case deprecating get should not be too hard, having a go atm.

[nrc]

@alexcrichton r? on just Vec::get ?

[sfackler]

There are some doc test failures: https://travis-ci.org/rust-lang/rust/jobs/29948445

[richo]

<3 this makes writing code involving Vec a jillion times less frustrating