rust-lang / rust

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

Misleading diagnostics in -(2147483649-1) #115619

Open Alcaro opened 1 year ago

Alcaro commented 1 year ago

Code

fn main() {
    let _ = -(2147483649-1);
}

Current output

Compiling playground v0.0.1 (/playground)
error: this arithmetic operation will overflow
 --> src/main.rs:2:13
  |
2 |     let _ = -(2147483649-1);
  |             ^^^^^^^^^^^^^^^ attempt to negate `i32::MIN`, which would overflow
  |
  = note: `#[deny(arithmetic_overflow)]` on by default

error: literal out of range for `i32`
 --> src/main.rs:2:15
  |
2 |     let _ = -(2147483649-1);
  |               ^^^^^^^^^^
  |
  = note: the literal `2147483649` does not fit into the type `i32` whose range is `-2147483648..=2147483647`
  = help: consider using the type `u32` instead
  = note: `#[deny(overflowing_literals)]` on by default

error: could not compile `playground` (bin "playground") due to 2 previous errors

Desired output

error: literal out of range for `i32`
 --> src/main.rs:2:15
  |
2 |     let _ = -(2147483649-1);
  |               ^^^^^^^^^^
  |
  = note: the literal `2147483649` does not fit into the type `i32` whose range is `-2147483648..=2147483647`
  = help: consider using the type `u32` instead
  = note: `#[deny(overflowing_literals)]` on by default

Rationale and extra context

The paren's contents are not i32::MIN; it's denied by overflowing_literals, which means the expression doesn't have a value, and issuing diagnostics about a non-value is just misleading.

Especially considering it's above the real error; the top error is usually the most important, but not here.

Other cases

No response

Anything else?

No response

mj10021 commented 1 year ago

@rustbot claim

mj10021 commented 1 year ago

@rustbot release-assignment

mj10021 commented 1 year ago

2147483649-1 is equal to i32::MIN, which is then negated causing overflow, no?

asquared31415 commented 1 year ago

So the problem here is that this arithmetic operation will overflow is useless if and only if the literal out of range shows up. Remember that these lints can be toggled independently.

This code:

#[allow(overflowing_literals)]
let _ = -(2147483649-1); // note: 2147483649 == -2147483647_i32

errors with the expected and helpful lint

error: this arithmetic operation will overflow
 --> src/main.rs:3:13
  |
3 |     let _ = -(2147483649-1); // note: 2147483649 == -2147483647_i32
  |             ^^^^^^^^^^^^^^^ attempt to negate `i32::MIN`, which would overflow
  |
  = note: `#[deny(arithmetic_overflow)]` on by default

This might be helped by emitting "literal out of range" before other integer related lints, but I am not sure if that's possible or useful in the general case.

kiscad commented 1 year ago

@rustbot claim

kiscad commented 1 year ago

It looks the literal out of range doesn't always trigger arithmetic overflow. For example, 128_i8 + 1 only emits literal out of range.

When a literal is out of range, the wrapped value of literal will be used in the check of arithmetic overflow.

So I think the diagnostic message in this issue is not a problem.

Noratrieb commented 11 months ago

which means the expression doesn't have a value, and issuing diagnostics about a non-value is just misleading.

This is not correct, out of range literals just wrap around, in this case to -1. The diagnostic is correct, it's just a bit confusing. The overflowing literals lint should probably mention what actual value will be used.