rust-lang / rust

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

[Nightly Regression] False positive warning with constants in conditional #47354

Open leonardo-m opened 6 years ago

leonardo-m commented 6 years ago

This is modified code from a real regression in my code:

#![allow(unused_variables)]
fn main() {
    const N: usize = 1_000;
    const M: usize = 4;
    const V: [u32; M] = [1, 2, 3, 4];

    let x = if N <= M {
        V[N - 1]
    } else {
        0
    };
}
warning: this expression will panic at run-time
 --> C:\lavoro\bugs\test.rs:9:9
  |
9 |         INIT[N - 1]
  |         ^^^^^^^^^^^ index out of bounds: the len is 4 but the index is 999

rustc 1.25.0-nightly (f62f77403 2018-01-10)
binary: rustc
commit-hash: f62f774035735a06c880c48c0b9017fcc0577e33
commit-date: 2018-01-10
host: x86_64-pc-windows-gnu
release: 1.25.0-nightly
LLVM version: 4.0

D language and C++17 avoid such problems using "static if" and "if constexpr".

oli-obk commented 6 years ago

That's very likely from my miri changes. I touched the very code emitting this message and improved the code that can be evaluated. This needs an RFC (which I'm currently writing).

leonardo-m commented 6 years ago

Fixed since few Nightlies.

leonardo-m commented 6 years ago

The problem is back (rustc 1.29.0-nightly 874dec25e 2018-07-21), but it's not visible compiling with just --emit=metadata (as in Issue #51491 ):

error: index out of bounds: the len is 4 but the index is 999
 --> test.rs:8:9
  |
8 |         V[N - 1]
  |         ^^^^^^^^
  |
  = note: #[deny(const_err)] on by default

error: aborting due to previous error
oli-obk commented 6 years ago

I'm surprised it was ever fixed. We have a test reproducing this and it hasn't been touched

kovaxis commented 5 years ago

I've just hit this issue in my own code, and the problem is reproducible just by copy-pasting the above code into the Rust playground in 1.33.0-stable.

I've looked around and found #52966, which was about a similar regression and was closed. If this behavior is intended, then this issue can be closed.

But just to clarify, are lints causing compile-time errors a thing now? It feels like they should not, given that it makes ie. changing a let binding to a const binding a compile-time error that is not a run-time error. In my mind (ideally) lints would err on the "false positive" side and assume things about the code (since they can be easily supressed). On the other hand errors would err on the "safe" side and only block compilation when it is certain that something is wrong.

steveklabnik commented 4 years ago

Triage: no change