Repository navigation
"arithmetic_overflow" and "unconditional_panic" lints do not fire on const fn that is only const-used in bin crate #81265
Description
I tried the following code:
const fn overflow() -> u32 {
0 - 1
}
pub const X: u32 = overflow();
fn main() {}On stable, in release mode, the emits a lint (err-by-default) about the arithmetic overflow. On nightly, it does not.
This probably is related to #78407. However, the part I do not understand is that I can only reproduce the problem with a binary crate, not with a library crate. Cc @oli-obk
Activity
[-]"arithmetic_overflow" and "unconditional_panic" lints do not fire on `const fn` in bin crate that is only const-used[/-][+]"arithmetic_overflow" and "unconditional_panic" lints do not fire on `const fn` that is only const-used in bin crate[/+]oli-obk commentedon Jan 22, 2021
That is odd indeed. I think it is a bug. Apparently the library crate will build the
optimized_mirforoverfloweven thoughoverflowis not public.Is it a problem that we do not emit these lints on
const fnthat are only used in constants?RalfJung commentedon Jan 22, 2021
Yeah I think so. We should properly emit lints for all
fn, no matter if they areconstor not and no matter if they are used or not and no matter if this is a bin crate or a lib crate.Emiting lints as part of an optimizations makes this hard. To me that just shows it's a mistake to emit lints as part of an optimization.^^
oli-obk commentedon Jan 22, 2021
Note that there are a bunch of lints that we do not emit in dead code. I feel like this is a similar situation. Either the lint is triggered on dead code in the const fn, or the interpretation of the const fn as part of the interpretation of a const item will report the same error anyway.
RalfJung commentedon Jan 22, 2021
If this is a dead code argument, then I assume we certainly should still emit the lint in a lib crate if the fn is
pub?No error will be reported in release mode, since overflow checks are disabled. So the fact that there is an overflow is entirely lost when no lint is emitted.
oli-obk commentedon Jan 22, 2021
Yes, and we would, because libraries generate
optimized_mirfor all their pub functionsugh, right.
So... if we want these lints unconditionally, we can run const prop just in analysis mode in the place where we also run borrowck and similar guaranteed MIR passes. in the optimization pipeline we'd then run const prop in mutating mode and possibly even not report any diagnostics from there.
apiraino commentedon Jan 27, 2021
Assigning
P-mediumas discussed as part of the Prioritization Working Group procedure and removingI-prioritize.emilyalbini commentedon May 4, 2021
I'm a bit confused about this being labeled a regression, it seems to correctly emit an error on stable, beta and nightly.
RalfJung commentedon May 4, 2021
@pietroalbini try doing a release build. Then it does not error on nightly, but AFAIK it used to error in the past (stable at the time the report was made). I will edit the OP to mention the "release" bit.
emilyalbini commentedon May 4, 2021
Ugh. Then this does not only affect nightly, this affects at least stable 1.51.0.
emilyalbini commentedon May 4, 2021
Yeah the PR you mention could've caused it landed in 1.51.0. Updated the labels accordingly.