From 47f3f66240eb20da1cd7583115a6ea1a2d25e388 Mon Sep 17 00:00:00 2001 From: xFrednet Date: Sat, 5 Mar 2022 21:54:49 +0100 Subject: [PATCH 1/4] Update unstable `ExpectationId`s in stored diagnostics --- compiler/rustc_errors/src/diagnostic.rs | 25 ++++++++++++++++++++- compiler/rustc_errors/src/lib.rs | 29 +++++++++++++++---------- 2 files changed, 41 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index a59d91ea78900..aa9c7829d8e72 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -5,7 +5,8 @@ use crate::Substitution; use crate::SubstitutionPart; use crate::SuggestionStyle; use crate::ToolMetadata; -use rustc_lint_defs::Applicability; +use rustc_data_structures::stable_map::FxHashMap; +use rustc_lint_defs::{Applicability, LintExpectationId}; use rustc_serialize::json::Json; use rustc_span::{MultiSpan, Span, DUMMY_SP}; use std::fmt; @@ -137,6 +138,28 @@ impl Diagnostic { } } + pub fn update_unstable_expectation_id( + &mut self, + unstable_to_stable: &FxHashMap, + ) { + if let Level::Expect(expectation_id) = &mut self.level { + if expectation_id.is_stable() { + return; + } + + // The unstable to stable map only maps the unstable `AttrId` to a stable `HirId` with an attribute index. + // The lint index inside the attribute is manually transferred here. + let lint_index = expectation_id.get_lint_index(); + expectation_id.set_lint_index(None); + let mut stable_id = *unstable_to_stable + .get(&expectation_id) + .expect("each unstable `LintExpectationId` must have a matching stable id"); + + stable_id.set_lint_index(lint_index); + *expectation_id = stable_id; + } + } + pub fn has_future_breakage(&self) -> bool { match self.code { Some(DiagnosticId::Lint { has_future_breakage, .. }) => has_future_breakage, diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 217d3ec2c247a..cba06448c4a6e 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -522,6 +522,11 @@ impl Drop for HandlerInner { "no warnings or errors encountered even though `delayed_good_path_bugs` issued", ); } + + assert!( + self.unstable_expect_diagnostics.is_empty(), + "all diagnostics with unstable expectations should have been converted", + ); } } @@ -942,25 +947,25 @@ impl Handler { let mut inner = self.inner.borrow_mut(); for mut diag in diags.into_iter() { - let mut unstable_id = diag + diag.update_unstable_expectation_id(unstable_to_stable); + + let stable_id = diag .level .get_expectation_id() .expect("all diagnostics inside `unstable_expect_diagnostics` must have a `LintExpectationId`"); - - // The unstable to stable map only maps the unstable `AttrId` to a stable `HirId` with an attribute index. - // The lint index inside the attribute is manually transferred here. - let lint_index = unstable_id.get_lint_index(); - unstable_id.set_lint_index(None); - let mut stable_id = *unstable_to_stable - .get(&unstable_id) - .expect("each unstable `LintExpectationId` must have a matching stable id"); - - stable_id.set_lint_index(lint_index); - diag.level = Level::Expect(stable_id); inner.fulfilled_expectations.insert(stable_id); (*TRACK_DIAGNOSTICS)(&diag); } + + inner + .stashed_diagnostics + .values_mut() + .for_each(|diag| diag.update_unstable_expectation_id(unstable_to_stable)); + inner + .future_breakage_diagnostics + .iter_mut() + .for_each(|diag| diag.update_unstable_expectation_id(unstable_to_stable)); } /// This methods steals all [`LintExpectationId`]s that are stored inside From 165b5583e56ad6d54922a7b5117a37c2ca2c89a1 Mon Sep 17 00:00:00 2001 From: xFrednet Date: Sun, 6 Mar 2022 11:47:08 +0100 Subject: [PATCH 2/4] Fix typos in `LintExpectationId` docs --- compiler/rustc_lint_defs/src/lib.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs index c78428765bb89..af41e9c06d933 100644 --- a/compiler/rustc_lint_defs/src/lib.rs +++ b/compiler/rustc_lint_defs/src/lib.rs @@ -54,7 +54,7 @@ pub enum Applicability { /// Expected `Diagnostic`s get the lint level `Expect` which stores the `LintExpectationId` /// to match it with the actual expectation later on. /// -/// The `LintExpectationId` has to be has stable between compilations, as diagnostic +/// The `LintExpectationId` has to be stable between compilations, as diagnostic /// instances might be loaded from cache. Lint messages can be emitted during an /// `EarlyLintPass` operating on the AST and during a `LateLintPass` traversing the /// HIR tree. The AST doesn't have enough information to create a stable id. The @@ -71,7 +71,7 @@ pub enum Applicability { #[derive(Clone, Copy, PartialEq, PartialOrd, Eq, Ord, Debug, Hash, Encodable, Decodable)] pub enum LintExpectationId { /// Used for lints emitted during the `EarlyLintPass`. This id is not - /// has stable and should not be cached. + /// hash stable and should not be cached. Unstable { attr_id: AttrId, lint_index: Option }, /// The [`HirId`] that the lint expectation is attached to. This id is /// stable and can be cached. The additional index ensures that nodes with @@ -113,7 +113,9 @@ impl HashStable for LintExpectationId { lint_index.hash_stable(hcx, hasher); } _ => { - unreachable!("HashStable should only be called for a filled `LintExpectationId`") + unreachable!( + "HashStable should only be called for filled and stable `LintExpectationId`" + ) } } } From d39d60971bafe34f626585e4887e7eec726fe972 Mon Sep 17 00:00:00 2001 From: xFrednet Date: Sun, 6 Mar 2022 14:18:28 +0100 Subject: [PATCH 3/4] Handle `#[expect(unfulfilled_lint_expectations)]` with a lint message --- compiler/rustc_errors/src/lib.rs | 1 + compiler/rustc_lint/src/expect.rs | 9 +++-- compiler/rustc_lint/src/levels.rs | 34 ++++++++++++---- compiler/rustc_middle/src/lint.rs | 12 +++++- .../expect_unfulfilled_expectation.rs | 39 +++++++++++++++++++ .../expect_unfulfilled_expectation.stderr | 38 ++++++++++++++++++ 6 files changed, 119 insertions(+), 14 deletions(-) create mode 100644 src/test/ui/lint/rfc-2383-lint-reason/expect_unfulfilled_expectation.rs create mode 100644 src/test/ui/lint/rfc-2383-lint-reason/expect_unfulfilled_expectation.stderr diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index cba06448c4a6e..5ba1fe9b4786f 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -970,6 +970,7 @@ impl Handler { /// This methods steals all [`LintExpectationId`]s that are stored inside /// [`HandlerInner`] and indicate that the linked expectation has been fulfilled. + #[must_use] pub fn steal_fulfilled_expectation_ids(&self) -> FxHashSet { assert!( self.inner.borrow().unstable_expect_diagnostics.is_empty(), diff --git a/compiler/rustc_lint/src/expect.rs b/compiler/rustc_lint/src/expect.rs index e6c9d0b0ab000..74fef0be9e98c 100644 --- a/compiler/rustc_lint/src/expect.rs +++ b/compiler/rustc_lint/src/expect.rs @@ -30,10 +30,6 @@ fn emit_unfulfilled_expectation_lint( hir_id: HirId, expectation: &LintExpectation, ) { - // FIXME: The current implementation doesn't cover cases where the - // `unfulfilled_lint_expectations` is actually expected by another lint - // expectation. This can be added here by checking the lint level and - // retrieving the `LintExpectationId` if it was expected. tcx.struct_span_lint_hir( builtin::UNFULFILLED_LINT_EXPECTATIONS, hir_id, @@ -43,6 +39,11 @@ fn emit_unfulfilled_expectation_lint( if let Some(rationale) = expectation.reason { diag.note(&rationale.as_str()); } + + if expectation.is_unfulfilled_lint_expectations { + diag.note("the `unfulfilled_lint_expectations` lint can't be expected and will always produce this message"); + } + diag.emit(); }, ); diff --git a/compiler/rustc_lint/src/levels.rs b/compiler/rustc_lint/src/levels.rs index f46f74fa45fb0..468aacc9bcaaa 100644 --- a/compiler/rustc_lint/src/levels.rs +++ b/compiler/rustc_lint/src/levels.rs @@ -14,7 +14,7 @@ use rustc_middle::lint::{ use rustc_middle::ty::query::Providers; use rustc_middle::ty::{RegisteredTools, TyCtxt}; use rustc_session::lint::{ - builtin::{self, FORBIDDEN_LINT_GROUPS}, + builtin::{self, FORBIDDEN_LINT_GROUPS, UNFULFILLED_LINT_EXPECTATIONS}, Level, Lint, LintExpectationId, LintId, }; use rustc_session::parse::feature_err; @@ -212,6 +212,14 @@ impl<'s> LintLevelsBuilder<'s> { } } } + + // The lint `unfulfilled_lint_expectations` can't be expected, as it would suppress itself. + // Handling expectations of this lint would add additional complexity with little to no + // benefit. The expect level for this lint will therefore be ignored. + if let Level::Expect(_) = level && id == LintId::of(UNFULFILLED_LINT_EXPECTATIONS) { + return; + } + if let Level::ForceWarn = old_level { specs.insert(id, (old_level, old_src)); } else { @@ -344,6 +352,20 @@ impl<'s> LintLevelsBuilder<'s> { self.store.check_lint_name(&name, tool_name, self.registered_tools); match &lint_result { CheckLintNameResult::Ok(ids) => { + // This checks for instances where the user writes `#[expect(unfulfilled_lint_expectations)]` + // in that case we want to avoid overriding the lint level but instead add an expectation that + // can't be fulfilled. The lint message will include an explanation, that the + // `unfulfilled_lint_expectations` lint can't be expected. + if let Level::Expect(expect_id) = level { + let is_unfulfilled_lint_expectations = match ids { + [lint] => *lint == LintId::of(UNFULFILLED_LINT_EXPECTATIONS), + _ => false, + }; + self.lint_expectations.push(( + expect_id, + LintExpectation::new(reason, sp, is_unfulfilled_lint_expectations), + )); + } let src = LintLevelSource::Node( meta_item.path.segments.last().expect("empty lint name").ident.name, sp, @@ -353,10 +375,6 @@ impl<'s> LintLevelsBuilder<'s> { self.check_gated_lint(id, attr.span); self.insert_spec(&mut specs, id, (level, src)); } - if let Level::Expect(expect_id) = level { - self.lint_expectations - .push((expect_id, LintExpectation::new(reason, sp))); - } } CheckLintNameResult::Tool(result) => { @@ -374,7 +392,7 @@ impl<'s> LintLevelsBuilder<'s> { } if let Level::Expect(expect_id) = level { self.lint_expectations - .push((expect_id, LintExpectation::new(reason, sp))); + .push((expect_id, LintExpectation::new(reason, sp, false))); } } Err((Some(ids), ref new_lint_name)) => { @@ -414,7 +432,7 @@ impl<'s> LintLevelsBuilder<'s> { } if let Level::Expect(expect_id) = level { self.lint_expectations - .push((expect_id, LintExpectation::new(reason, sp))); + .push((expect_id, LintExpectation::new(reason, sp, false))); } } Err((None, _)) => { @@ -511,7 +529,7 @@ impl<'s> LintLevelsBuilder<'s> { } if let Level::Expect(expect_id) = level { self.lint_expectations - .push((expect_id, LintExpectation::new(reason, sp))); + .push((expect_id, LintExpectation::new(reason, sp, false))); } } else { panic!("renamed lint does not exist: {}", new_name); diff --git a/compiler/rustc_middle/src/lint.rs b/compiler/rustc_middle/src/lint.rs index 894947fa70d96..1b301629b9c73 100644 --- a/compiler/rustc_middle/src/lint.rs +++ b/compiler/rustc_middle/src/lint.rs @@ -204,11 +204,19 @@ pub struct LintExpectation { pub reason: Option, /// The [`Span`] of the attribute that this expectation originated from. pub emission_span: Span, + /// Lint messages for the `unfulfilled_lint_expectations` lint will be + /// adjusted to include an additional note. Therefore, we have to track if + /// the expectation is for the lint. + pub is_unfulfilled_lint_expectations: bool, } impl LintExpectation { - pub fn new(reason: Option, attr_span: Span) -> Self { - Self { reason, emission_span: attr_span } + pub fn new( + reason: Option, + emission_span: Span, + is_unfulfilled_lint_expectations: bool, + ) -> Self { + Self { reason, emission_span, is_unfulfilled_lint_expectations } } } diff --git a/src/test/ui/lint/rfc-2383-lint-reason/expect_unfulfilled_expectation.rs b/src/test/ui/lint/rfc-2383-lint-reason/expect_unfulfilled_expectation.rs new file mode 100644 index 0000000000000..d38e65533869a --- /dev/null +++ b/src/test/ui/lint/rfc-2383-lint-reason/expect_unfulfilled_expectation.rs @@ -0,0 +1,39 @@ +// check-pass +// ignore-tidy-linelength + +#![feature(lint_reasons)] +#![warn(unused_mut)] + +#![expect(unfulfilled_lint_expectations, reason = "idk why you would expect this")] +//~^ WARNING this lint expectation is unfulfilled +//~| NOTE `#[warn(unfulfilled_lint_expectations)]` on by default +//~| NOTE idk why you would expect this +//~| NOTE the `unfulfilled_lint_expectations` lint can't be expected and will always produce this message + +#[expect(unfulfilled_lint_expectations, reason = "a local: idk why you would expect this")] +//~^ WARNING this lint expectation is unfulfilled +//~| NOTE a local: idk why you would expect this +//~| NOTE the `unfulfilled_lint_expectations` lint can't be expected and will always produce this message +pub fn normal_test_fn() { + #[expect(unused_mut, reason = "this expectation will create a diagnostic with the default lint level")] + //~^ WARNING this lint expectation is unfulfilled + //~| NOTE this expectation will create a diagnostic with the default lint level + let mut v = vec![1, 1, 2, 3, 5]; + v.sort(); + + // Check that lint lists including `unfulfilled_lint_expectations` are also handled correctly + #[expect(unused, unfulfilled_lint_expectations, reason = "the expectation for `unused` should be fulfilled")] + //~^ WARNING this lint expectation is unfulfilled + //~| NOTE the expectation for `unused` should be fulfilled + //~| NOTE the `unfulfilled_lint_expectations` lint can't be expected and will always produce this message + let value = "I'm unused"; +} + +#[expect(warnings, reason = "this suppresses all warnings and also suppresses itself. No warning will be issued")] +pub fn expect_warnings() { + // This lint trigger will be suppressed + #[warn(unused_mut)] + let mut v = vec![1, 1, 2, 3, 5]; +} + +fn main() {} diff --git a/src/test/ui/lint/rfc-2383-lint-reason/expect_unfulfilled_expectation.stderr b/src/test/ui/lint/rfc-2383-lint-reason/expect_unfulfilled_expectation.stderr new file mode 100644 index 0000000000000..9bfee79b03d70 --- /dev/null +++ b/src/test/ui/lint/rfc-2383-lint-reason/expect_unfulfilled_expectation.stderr @@ -0,0 +1,38 @@ +warning: this lint expectation is unfulfilled + --> $DIR/expect_unfulfilled_expectation.rs:7:11 + | +LL | #![expect(unfulfilled_lint_expectations, reason = "idk why you would expect this")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(unfulfilled_lint_expectations)]` on by default + = note: idk why you would expect this + = note: the `unfulfilled_lint_expectations` lint can't be expected and will always produce this message + +warning: this lint expectation is unfulfilled + --> $DIR/expect_unfulfilled_expectation.rs:13:10 + | +LL | #[expect(unfulfilled_lint_expectations, reason = "a local: idk why you would expect this")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: a local: idk why you would expect this + = note: the `unfulfilled_lint_expectations` lint can't be expected and will always produce this message + +warning: this lint expectation is unfulfilled + --> $DIR/expect_unfulfilled_expectation.rs:18:14 + | +LL | #[expect(unused_mut, reason = "this expectation will create a diagnostic with the default lint level")] + | ^^^^^^^^^^ + | + = note: this expectation will create a diagnostic with the default lint level + +warning: this lint expectation is unfulfilled + --> $DIR/expect_unfulfilled_expectation.rs:25:22 + | +LL | #[expect(unused, unfulfilled_lint_expectations, reason = "the expectation for `unused` should be fulfilled")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: the expectation for `unused` should be fulfilled + = note: the `unfulfilled_lint_expectations` lint can't be expected and will always produce this message + +warning: 4 warnings emitted + From be84049570d5be6d6e76e471ef88a11ae46292ad Mon Sep 17 00:00:00 2001 From: xFrednet Date: Wed, 9 Mar 2022 21:58:13 +0100 Subject: [PATCH 4/4] Add comment about `unfulfilled_lint_expectation` not being in a group (RFC 2383) --- compiler/rustc_lint/src/levels.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/compiler/rustc_lint/src/levels.rs b/compiler/rustc_lint/src/levels.rs index 468aacc9bcaaa..99bbe01566a94 100644 --- a/compiler/rustc_lint/src/levels.rs +++ b/compiler/rustc_lint/src/levels.rs @@ -357,6 +357,8 @@ impl<'s> LintLevelsBuilder<'s> { // can't be fulfilled. The lint message will include an explanation, that the // `unfulfilled_lint_expectations` lint can't be expected. if let Level::Expect(expect_id) = level { + // The `unfulfilled_lint_expectations` lint is not part of any lint groups. Therefore. we + // only need to check the slice if it contains a single lint. let is_unfulfilled_lint_expectations = match ids { [lint] => *lint == LintId::of(UNFULFILLED_LINT_EXPECTATIONS), _ => false,