rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.3k stars 12.72k forks source link

Bad advice given for arr[0i32.into()] #107292

Open d0sboots opened 1 year ago

d0sboots commented 1 year ago

Code

fn main() {
    let arr: [u32; 3] = [0, 1, 2];
    let idx = 0i32;
    println!("{}", arr[idx.into()]);
}

Current output

error[E0282]: type annotations needed
 --> src/main.rs:4:20
  |
4 |     println!("{}", arr[idx.into()]);
  |                    ^^^^^^^^^^^^^^^ cannot infer type of the type parameter `T` declared on the associated function `new_display`
  |
  = note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider specifying the generic argument
  |
4 |     println!("{}", arr[idx.into()]::<T>);
  |                                   +++++

For more information about this error, try `rustc --explain E0282`.

Desired output

I'm too new to Rust to say exactly, but:

Rationale and extra context

Naively, one one expect that the requirement of array access being usize means an into() would work here. Clearly, that's not the case but I'm still not sure why.

Other cases

fn main() {
    let arr: [u32; 3] = [0, 1, 2];
    let idx = 0i32;
    let val = arr[idx.into()];
    println!("{}", val);
}

still gives bad advice, although less catastrophically:

error[E0282]: type annotations needed
 --> src/main.rs:4:9
  |
4 |     let val = arr[idx.into()];
  |         ^^^
  |
help: consider giving `val` an explicit type
  |
4 |     let val: _ = arr[idx.into()];
  |            +++

For more information about this error, try `rustc --explain E0282`.

Only when you take that bad advice, but with an explicit type instead of _, do you get a useful message:

fn main() {
    let arr: [u32; 3] = [0, 1, 2];
    let idx = 0i32;
    let val: u32 = arr[idx.into()];
    println!("{}", val);
}
error[E0282]: type annotations needed
 --> src/main.rs:4:28
  |
4 |     let val: u32 = arr[idx.into()];
  |                            ^^^^
  |
help: try using a fully qualified path to specify the expected types
  |
4 |     let val: u32 = arr[<i32 as Into<T>>::into(idx)];
  |                        +++++++++++++++++++++++   ~

For more information about this error, try `rustc --explain E0282`.

Anything else?

No response

fmease commented 1 year ago

The suggestion is syntactically incorrect. @rustbot label A-suggestion-diagnostics D-incorrect

PatchMixolydic commented 1 year ago

Naively, one one expect that the requirement of array access being usize means an into() would work here. Clearly, that's not the case but I'm still not sure why.

usize is allowed to be 16 bits wide, so it can't implement From<i32>. It would be nice if the compiler could suggest using try_into instead, but I don't know how feasible that is.

d0sboots commented 1 year ago

usize is allowed to be 16 bits wide, so it can't implement From<i32>.

Oof. I actually already knew that, since I have a bunch of as coercions elsewhere in my code. (It's not just because of 16-bit machines - i32 can't be converted to u32 transparently.)

However, that's not the whole explanation. u8, which does convert to usize, doesn't work either (but it has much better diagnostics):

fn main() {
    let arr: [u32; 3] = [0, 1, 2];
    let idx = 0u8;
    let val: u32 = arr[idx.into()];
    println!("{}", val);
}
error[[E0282]](https://doc.rust-lang.org/stable/error-index.html#E0282): type annotations needed
 --> src/main.rs:4:28
  |
4 |     let val: u32 = arr[idx.into()];
  |                            ^^^^
  |
help: try using a fully qualified path to specify the expected types
  |
4 |     let val: u32 = arr[<u8 as Into<T>>::into(idx)];
  |                        ++++++++++++++++++++++   ~

For more information about this error, try `rustc --explain E0282`.

(Also, for some reason usize doesn't implement From<i8>, but that's a different issue.)

fmease commented 1 year ago

It seems like this issue is already fixed on nightly:

error[E0282]: type annotations needed
 --> src/main.rs:4:28
  |
4 |     println!("{}", arr[idx.into()]);
  |                            ^^^^
  |
help: try using a fully qualified path to specify the expected types
  |
4 |     println!("{}", arr[<i32 as Into<T>>::into(idx)]);
  |                        +++++++++++++++++++++++   ~
d0sboots commented 1 year ago

It seems like this issue is already fixed on nightly:

error[E0282]: type annotations needed
 --> src/main.rs:4:28
  |
4 |     println!("{}", arr[idx.into()]);
  |                            ^^^^
  |
help: try using a fully qualified path to specify the expected types
  |
4 |     println!("{}", arr[<i32 as Into<T>>::into(idx)]);
  |                        +++++++++++++++++++++++   ~

Not completely, that's just a different rearrangement of syntax that doesn't compile:

Compiling playground v0.0.1 (/playground)
error[[E0412]](https://doc.rust-lang.org/nightly/error-index.html#E0412): cannot find type `T` in this scope
 --> src/main.rs:4:37
  |
4 |     println!("{}", arr[<i32 as Into<T>>::into(idx)]);
  |                                     ^ not found in this scope

For more information about this error, try `rustc --explain E0412`.
error: could not compile `playground` due to previous error
estebank commented 1 year ago

@d0sboots the compiler is giving you the syntax that you can use to specify what you're turning into, but it can't guess which type you want to convert to. Until #114811 we weren't showing you the full list because it was too long, but after it the output is:

error[E0283]: type annotations needed
 --> f71.rs:4:28
  |
4 |     let val: u32 = arr[idx.into()];
  |                            ^^^^
  |
  = note: multiple `impl`s satisfying `_: From<u8>` found in the following crates: `core`, `std`:
          - impl From<u8> for AtomicU8;
          - impl From<u8> for ExitCode;
          - impl From<u8> for char;
          - impl From<u8> for f32;
          - impl From<u8> for f64;
          - impl From<u8> for i128;
          - impl From<u8> for i16;
          - impl From<u8> for i32;
          - impl From<u8> for i64;
          - impl From<u8> for isize;
          - impl From<u8> for std::sys::unix::process::process_common::ExitCode;
          - impl From<u8> for u128;
          - impl From<u8> for u16;
          - impl From<u8> for u32;
          - impl From<u8> for u64;
          - impl From<u8> for usize;
  = note: required for `u8` to implement `Into<_>`
help: try using a fully qualified path to specify the expected types
  |
4 |     let val: u32 = arr[<u8 as Into<T>>::into(idx)];
  |                        ++++++++++++++++++++++   ~

You need to substitute T with usize to make your code work.


The original report with the mentioned PR adds the list of impls to the output:

error[E0283]: type annotations needed
 --> f71.rs:4:28
  |
4 |     println!("{}", arr[idx.into()]);
  |                            ^^^^
  |
  = note: multiple `impl`s satisfying `_: From<i32>` found in the following crates: `core`, `std`:
          - impl From<i32> for AtomicI32;
          - impl From<i32> for f64;
          - impl From<i32> for i128;
          - impl From<i32> for i64;
          - impl From<i32> for std::sys::unix::process::process_inner::ExitStatus;
  = note: required for `i32` to implement `Into<_>`
help: try using a fully qualified path to specify the expected types
  |
4 |     println!("{}", arr[<i32 as Into<T>>::into(idx)]);
  |                        +++++++++++++++++++++++   ~

Only outstanding work is to have rustc understand From/Into impls specifically and use the appropriate types in the suggestion.

d0sboots commented 1 year ago

So, there's one thing I still don't understand with this. Array indexing is always a usize context, right? So why is it showing the full list of all From impls for u8, and unable to infer the correct destination type?

Because of this, the real code change here (for the non-working code sample) IMO is to assign to a temporary variable, so that the type inference works better.

estebank commented 1 year ago

Correct, the indexing operation doesn't (today) influence inference in the way that we would need for this to work.