overflowing_literals error is confusing because of type inference

[shreepads]

While experimenting with the Rust By Example playground on the Inference section at https://doc.rust-lang.org/stable/rust-by-example/types/inference.html I came across what I believe is an error in the error produced by the compiler.

I do not expect this example to work but the error literal out of range for 'i8' is reported against line 4 let e2 = 230; which is a perfectly valid statement. The problem is lower down where two different types are pushed onto the same Vector but possibly due to the way the inference engine works this is being incorrectly reported against line 4. If I comment out the line vec.push(e2);, the example works fine, apart from a warning: unused variable: 'e2'.

Code

fn main() {
    // Because of the annotation, the compiler knows that `elem` has type i8.
    let elem = 6i8;
    let e2 = 230;

    // Create an empty vector (a growable array).
    let mut vec = Vec::new();
    // At this point the compiler doesn't know the exact type of `vec`, it
    // just knows that it's a vector of something (`Vec<_>`).

    // Insert `elem` in the vector.
    vec.push(e2);
    vec.push(elem);
    // Aha! Now the compiler knows that `vec` is a vector of `u8`s (`Vec<u8>`)
    // TODO ^ Try commenting out the `vec.push(elem)` line

    println!("{:?}", vec);
}

Meta

rustc --version --verbose: I am unable to determine the exact compiler version in the Rust By Example playground. I have run the same example on my Fedora 32 Workstation machine and with the same result. The rustc version details for this machine are as follows

rustc 1.48.0
binary: rustc
commit-hash: unknown
commit-date: unknown
host: x86_64-unknown-linux-gnu
release: 1.48.0
LLVM version: 10.0

Error output

   Compiling playground v0.0.1 (/playground)
error: literal out of range for `i8`
 --> src/main.rs:4:14
  |
4 |     let e2 = 230;
  |              ^^^
  |
  = note: `#[deny(overflowing_literals)]` on by default
  = note: the literal `230` does not fit into the type `i8` whose range is `-128..=127`

error: aborting due to previous error

error: could not compile `playground`

To learn more, run the command again with --verbose.

Backtrace

Sorry I don't know exactly how to provide this.

<backtrace>

[jonas-schievink]

e2 also has type i8, because you push it onto the same Vec as elem, so the error message is correct – the literal is out of range for the target type i8

[shreepads]

While that is true (and as I mentioned, I do not expect this to work) the problem is the error reported by the compiler doesn't help the user resolve the problem. In a small example like this it's easy to see where the problem lies but in a larger one the compiler error would send the user in the wrong direction.

[jonas-schievink]

okay, marking as a diagnostics enhancement

[camelid]

I'm guessing this would be hard to implement since we'd need to somehow track the spot(s) that caused us to infer a particular type.

[jonas-schievink]

Yes, seems very hard to implement. I think it would be reasonable to close this as wontfix, but I don't work on diagnostics, so I'll defer to other people.

[shreepads]

Sidenote: Coming from a C/ Java/ Python coding background I was pleasantly surprised with the quality of the diagnostics and suggestions generated by the compiler till I came across this one, which is what really prompted me to submit this issue (high expectations!).

I think the inference/ diagnostics logic in the compiler is already able to track the correct spot and what I've reported is probably an unhandled edge-case, because if I replace let e2 = 230; with let e2 = 230.0; the compiler produces a much better (expected) error diagnostic.

Code

fn main() {
    // Because of the annotation, the compiler knows that `elem` has type i8.
    let elem = 6i8;
    let e2 = 230.0;

    // Create an empty vector (a growable array).
    let mut vec = Vec::new();
    // At this point the compiler doesn't know the exact type of `vec`, it
    // just knows that it's a vector of something (`Vec<_>`).

    // Insert `elem` in the vector.
    vec.push(e2);
    vec.push(elem);
    // Aha! Now the compiler knows that `vec` is a vector of `u8`s (`Vec<u8>`)
    // TODO ^ Try commenting out the `vec.push(elem)` line

    println!("{:?}", vec);
}

Error Output

error[E0308]: mismatched types
  --> rustc-defect.rs:13:14
   |
13 |     vec.push(elem);
   |              ^^^^ expected floating-point number, found `i8`

error: aborting due to previous error

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

[camelid]

I think the inference/ diagnostics logic in the compiler is already able to track the correct spot and what I've reported is probably an unhandled edge-case,

Unfortunately, I don't think that's the case.

In the first example, all the compiler knows is that e2 is an integer, so when it sees you push it into what it's inferred to be a Vec<i8>, it says "OK! I guess e2 must be an i8". So this program type-checks. Then, you get an instance of the overflowing_literals lint, which is not a type error. It says that 230 is too big for i8. If you allowed this lint, your code would compile but would reinterpret 230's bytes as an i8; in this case, that produces -26 (not 230!).

However, in the second example, the compiler knows that e2 is a floating-point number and so it reports a type error when you then push an i8 into what it's inferred to be a Vec<{float}>. This is a type error, not the overflowing_literals lint. Note that if you switch the order of the pushes, you will get the reverse type error:

error[E0308]: mismatched types
  --> src/main.rs:13:14
   |
13 |     vec.push(e2);
   |              ^^ expected `i8`, found floating-point number

Here it's expecting an i8 and getting a float, where before it was the opposite.

[camelid]

Now that I've thought about this some more, I think this would be extremely hard to fix for not a ton of gain. The issue is that this error is not even coming out of the type-checker; it comes from rustc_mir_build, which is after inference has finished.

[camelid]

Reposting my idea from Zulip:

Perhaps we could store something in the TyCtxt or Sess that maps every AST node to its "inference source". There would be a struct InferenceSource that would hold the spans that caused this node to be inferred to be a particular type:

pub struct InferenceSource {
    pub source_spans: Vec<Span>,
}

Over time we could add more fields if we wanted more detailed diagnostics.

[camelid]

I'm not sure how feasible that is, but seems potentially possible.

[camelid]

One thing we'd need to figure out is when and how much to show of the inference-source stack. In some situations, it may not be helpful, and in others it may be too much information.

[shreepads]

In the first example, all the compiler knows is that e2 is an integer, so when it sees you push it into what it's inferred to be a Vec<i8>, it says "OK! I guess e2 must be an i8".

This had me confused for a bit as well, i.e. I'm first pushing e2 onto the Vector so I thought it would infer it to be Vec<i32> and then complain about adding the i8 but I guess the inference engine builds some kind of constraint model and uses the i8 even though it is pushed second because the e2 literal is not explicitly typed.

I experimented with let e2 = 2300; and let e2 = 2300i16; and in the first case it again reports literal out of range for 'i8' while in the second it reports mismatched types so I think I understand the inference model a little better!

One thing we'd need to figure out is when and how much to show of the inference-source stack. In some situations, it may not be helpful, and in others it may be too much information.

From an end-user perspective I couldn't follow much of what you said but if the diagnostic was to point to the line where the element was pushed (or otherwise used in a manner that causes a type mismatch) and not where it has its value set, that would be better.

[scottmcm]

I experimented with let e2 = 2300; and let e2 = 2300i16;

I think this suggests an inexpensive way forward for a diagnostic here: suggest that they add a suffix and recompile. For example, it could see the 230 and say something like "help: if you're unsure why it was inferred as i8, try changing it to 230_u8 and seeing where you get a type error". (Of course, if it's already suffixed, like 300_u8, then it shouldn't suggest that.)

[camelid]

That’s a good idea! :)