Compiler suggests unnecessary try into

[lopopolo]

I updated a dependency and the type of a struct field changed from an i32 to a u16.

I am assigning a u16 value to this struct field. Before, I was calling i32::from to convert the u16 to an i32. With the upgrade this is unnecessary since the field and the value are both u16.

The rust compiler suggests this fix:

error[E0308]: mismatched types
  --> artichoke-frontend/src/parser.rs:90:37
   |
90 |             (*self.parser).lineno = i32::from((*self.context).lineno);
   |                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected u16, found i32
help: you can convert an `i32` to `u16` and panic if the converted value wouldn't fit
   |
90 |             (*self.parser).lineno = i32::from((*self.context).lineno).try_into().unwrap();
   |                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.
error: Could not compile `artichoke-frontend`.
warning: build failed, waiting for other jobs to finish...
error: build failed

But it should really suggest removing the i32::from and using the u16 directly.

[lopopolo]

I am using Rust 1.37.0.

[Centril]

cc @estebank

[lopopolo]

Similarly, this code should suggest replacing i32::from with u16::from.

#[derive(Debug)]
struct Member(u16);

pub fn main() {
    let m = Member(i32::from(0_u8));
    println!("{:?}", m);
}

   Compiling playground v0.0.1 (/playground)
error[E0308]: mismatched types
 --> src/main.rs:5:20
  |
5 |     let m = Member(i32::from(0_u8));
  |                    ^^^^^^^^^^^^^^^ expected u16, found i32
help: you can convert an `i32` to `u16` and panic if the converted value wouldn't fit
  |
5 |     let m = Member(i32::from(0_u8).try_into().unwrap());
  |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

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

playground

[estebank]

For the case where we have i32::from(0_i16) and an i16 was expected, what should be the correct behavior? I believe simply suggesting i16::from(0_i16) is appropriate, if funny looking, because the 0_i16 could be coming from outside a macro, and the error be inside of one. Thoughts?

[lopopolo]

@estebank that sounds good to me 👍

[awaitlink]

@rustbot claim

[awaitlink]

@estebank Which of these options makes more sense?

---

Check if the new (suggested) conversion is infallible

• always (doesn't make sense in the 1st case?)
• only in the 2nd case (if the new conversion is fallible, I presume we fall back to other suggestions and .try_from().unwrap() should be suggested, which makes a bit of sense, since it highlights the possible panic?)
• never (if SomeNumberTy::from(...) is detected, always suggest to replace with ExpectedTy::from(...))

---

1. Seems like the user already opted in for panicking if the conversion fails, so conversion was fallible and will stay so:

struct A(i16);

fn main() {
    let _ = A(i32::from(0_i64));
}

2. The conversion is infallible but will become fallible if the suggestion is applied:

struct A(i16);

fn main() {
    let _ = A(i128::from(0_i64));
}