Repository navigation
Future compat reports show confusing "lint level defined here" when there is an allow #121009
Description
To reproduce, build any kind of code that triggers a future breakage report in a crate that allows that warning. For instance:
#![allow(const_patterns_without_partial_eq)]
trait EnumSetType {
type Repr;
}
enum Enum8 { }
impl EnumSetType for Enum8 {
type Repr = u8;
}
#[derive(PartialEq, Eq)]
struct EnumSet<T: EnumSetType> {
__enumset_underlying: T::Repr,
}
const CONST_SET: EnumSet<Enum8> = EnumSet { __enumset_underlying: 3 };
fn main() {
match CONST_SET {
CONST_SET => { /* ok */ } //~ERROR: must implement `PartialEq`
_ => panic!("match fell through?"),
}
}Build this with cargo, then run cargo report future-incompatibilities --id 1:
The package `fut v0.1.0 (/home/r/src/rust/tmp/fut)` currently triggers the following future incompatibility lints:
> warning: to use a constant of type `EnumSet<Enum8>` in a pattern, the type must implement `PartialEq`
> --> src/main.rs:21:9
> |
> 21 | CONST_SET => { /* ok */ } //~ERROR: must implement `PartialEq`
> | ^^^^^^^^^
> |
> = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
> = note: for more information, see issue #116122 <https://github.com/rust-lang/rust/issues/116122>
> note: the lint level is defined here
> --> src/main.rs:1:10
> |
> 1 | #![allow(const_patterns_without_partial_eq)]
> | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
That is quite confusing obviously, since it points at an allow lint level to justify a warning!
It seems to me like these future compat reports shouldn't display that "lint level is defined here" at all, after all, their entire point is that they are not affected by the lint levels in the crate.
Cc @rust-lang/wg-diagnostics
Activity
Urgau commentedon Feb 13, 2024
Should probably be conditioned here on
future_incompatible.is_none():rust/compiler/rustc_middle/src/lint.rs
Line 399 in fd9bb7f
@rustbot label -needs-triage +E-easy +A-diagnostics +D-confusing +T-compiler
RalfJung commentedon Feb 13, 2024
No, that doesn't sound right. This needs to be conditioned on whether the lint is being rendered for regular rustc output or for the future-breakage report.
Urgau commentedon Feb 13, 2024
Indeed, it should instead be conditional on
has_future_breakage, which is used to set thehas_future_breakageofIsLint, which is then used to add to diagnostic to thefuture_breakage_diagnosticslist.(Or at least that's my understanding after looking at the Cargo code as well)
RalfJung commentedon Feb 13, 2024
has_future_breakageis also set when the same lint is printed as a regular lint, isn't it?I was assuming the lint is rendered twice, once as a regular lint and once for the report. Maybe that's not true. So maybe the actual change that should happen is to skip
explain_lint_level_sourcewhen the lint level isallow? In that case usually the lint would be skipped entirely, but ofc for future breakage it is still put into the JSON -- not printed to stderr, but stored in cargo's future breakage report.Urgau commentedon Feb 13, 2024
Yes. And I don't the lint is "render" twice, when it is being emitted the emiter will add the lint to it's list and transmit it to Cargo.
This is also a possibility. Either would work.
Do not point at `#[allow(_)]` as the reason for compat lint triggering
Do not point at `#[allow(_)]` as the reason for compat lint triggering
Rollup merge of rust-lang#121049 - estebank:issue-121009, r=fmease
Unrolled build for rust-lang#121049