Skip to content
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

Implement Index for &[T] #16529

Closed
kennytm opened this issue Aug 16, 2014 · 5 comments · Fixed by #18698
Closed

Implement Index for &[T] #16529

kennytm opened this issue Aug 16, 2014 · 5 comments · Fixed by #18698

Comments

@kennytm
Copy link
Member

kennytm commented Aug 16, 2014

As titled. Although slices support the a[b] operator, it does not implement Index<uint, T>, so one cannot pass in a slice in the following example:

fn do_something<V: Clone, T: Collection + Index<uint, V>>(t: &T) -> Vec<V> {
    let mut v = Vec::with_capacity(t.len());
    for i in range(0, t.len()) {
        v.push((*t)[i].clone());
    }
    v
}

fn main() {
    println!("{}", do_something(&vec![1u,2,3]));  // works
    println!("{}", do_something(&&[1u,2,3])); // does not work
}
@huonw
Copy link
Member

huonw commented Aug 16, 2014

I'm somewhat inclined to think we could just remove the built-in indexing from &[T] entirely and thus remove/reduce the need for the special lang-items, although there's at least three considerations:

  • the failure message for out-of-bounds indexing would be less informative (no file/line information), but debugging any other fail! (e.g. Option.unwrap) already requires a debugger to get this info, so maybe it's not so bad.
  • [T, .. n] can't yet implement Index properly, so this would still need built-in support with [] (and thus the out-of-bounds failure lang items can't be entirely removed just yet), unless implicit coercions will work.
  • Improve method lookup auto-deref behavior (subissue of trait reform) #12825 needs to be fixed before IndexMut will work properly (and I guess DST would make it work more nicely).

Users would theoretically not see a change, since this would all happen inside std (and core).

@thestinger
Copy link
Contributor

@huonw: It's not possible due to lack of CTFE.

@aturon
Copy link
Member

aturon commented Oct 3, 2014

cc me

@japaric
Copy link
Member

japaric commented Nov 6, 2014

Should I send a PR implementing Index/IndexMut for [T] now, and we can evaluate later removing the built-in indexing?

I already tested that PR, and the built-in indexing is preferred in the operation slice[index], but one go through the Index trait using slice.index(&index).

@aturon
Copy link
Member

aturon commented Nov 6, 2014

@japaric Yes, please go ahead and do so. Thanks!

japaric pushed a commit to japaric/rust that referenced this issue Nov 7, 2014
bors added a commit that referenced this issue Nov 7, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants