-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Use primitive indexing in slice's Index/IndexMut #36454
Conversation
r? @aturon (rust_highfive has picked a reviewer for you, use r? to override) |
@bors: r+ Thanks! |
📌 Commit 20b9f1c has been approved by |
⌛ Testing commit 20b9f1c with merge 5675d4f... |
💔 Test failed - auto-win-msvc-64-opt-rustbuild |
@bors: retry On Tue, Sep 13, 2016 at 7:48 PM, bors [email protected] wrote:
|
Thanks @sfackler |
20b9f1c
to
7e425d9
Compare
So huh, it has using the Index trait for arrays, that's not how I understood it. |
Fixed the test by amending the commit. |
To explain the mystery, it looks like array indexing uses the Index trait for slice in constexprs, and compiler-supplied indexing otherwise (do you know about this @oli-obk ?) |
@bors: r+ |
📌 Commit 7e425d9 has been approved by |
The error pattern is still wrong. It should not include "assertion failed". The constexpr behaviour sounds very odd. Might be related to @eddyb's new const trans? |
@bors: r- Indeed looks like travis is still failing |
Not sure if relevant, but if integer type inference is being relied on, #33903 could be involved. |
[T]'s Index implementation is normally not used for indexing, instead the compiler supplied indexing is used. Use the compiler supplied version in Index/IndexMut. This removes an inconsistency: Compiler supplied bound check failures look like this: thread 'main' panicked at 'index out of bounds: the len is 3 but the index is 4' If you convince Rust to use the Index impl for slices, bounds check failure looks like this instead: thread 'main' panicked at 'assertion failed: index < self.len()' The latter is used if you for example use Index generically:: use std::ops::Index; fn foo<T: ?Sized>(x: &T) where T: Index<usize> { &x[4]; } foo(&[1, 2, 3][..])
Oh! Sorry, that was dumb. I'm fixing this up. |
7e425d9
to
a4ee9c6
Compare
@bors r=alexcrichton |
📌 Commit a4ee9c6 has been approved by |
…crichton Use primitive indexing in slice's Index/IndexMut [T]'s Index implementation is normally not used for indexing, instead the compiler supplied indexing is used. Use the compiler supplied version in Index/IndexMut. This removes an inconsistency: Compiler supplied bound check failures look like this: thread 'main' panicked at 'index out of bounds: the len is 3 but the index is 4' If you convince Rust to use the Index impl for slices, bounds check failure looks like this instead: thread 'main' panicked at 'assertion failed: index < self.len()' The latter is used if you for example use Index generically: ```rust use std::ops::Index; fn foo<T: ?Sized>(x: &T) where T: Index<usize> { &x[4]; } foo(&[1, 2, 3][..]) ```
[T]'s Index implementation is normally not used for indexing, instead
the compiler supplied indexing is used.
Use the compiler supplied version in Index/IndexMut.
This removes an inconsistency:
Compiler supplied bound check failures look like this:
thread 'main' panicked at 'index out of bounds: the len is 3 but the index is 4'
If you convince Rust to use the Index impl for slices, bounds check
failure looks like this instead:
thread 'main' panicked at 'assertion failed: index < self.len()'
The latter is used if you for example use Index generically: