bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Auto merge of #87835 - xFrednet:rfc-2383-expect-attribute-with-ids, r=wesleywiser

Browse source 
Implementation of the `expect` attribute (RFC 2383)

This is an implementation of the `expect` attribute as described in [RFC-2383](https://rust-lang.github.io/rfcs/2383-lint-reasons.html). The attribute allows the suppression of lint message by expecting them. Unfulfilled lint expectations (meaning no expected lint was caught) will emit the `unfulfilled_lint_expectations` lint at the `expect` attribute.

### Example
#### input
```rs
// required feature flag
#![feature(lint_reasons)]

#[expect(unused_mut)] // Will warn about an unfulfilled expectation
#[expect(unused_variables)] // Will be fulfilled by x
fn main() {
    let x = 0;
}
```

#### output

```txt
warning: this lint expectation is unfulfilled
  --> $DIR/trigger_lint.rs:3:1
   |
LL | #[expect(unused_mut)] // Will warn about an unfulfilled expectation
   |          ^^^^^^^^^^
   |
   = note: `#[warn(unfulfilled_lint_expectations)]` on by default
```

### Implementation

This implementation introduces `Expect` as a new lint level for diagnostics, which have been expected. All lint expectations marked via the `expect` attribute are collected in the [`LintLevelsBuilder`] and assigned an ID that is stored in the new lint level. The `LintLevelsBuilder` stores all found expectations and the data needed to emit the `unfulfilled_lint_expectations` in the [`LintLevelsMap`] which is the result of the [`lint_levels()`] query.

The [`rustc_errors::HandlerInner`] is the central error handler in rustc and handles the emission of all diagnostics. Lint message with the level `Expect` are suppressed during this emission, while the expectation ID is stored in a set which marks them as fulfilled. The last step is then so simply check if all expectations collected by the [`LintLevelsBuilder`] in the [`LintLevelsMap`] have been marked as fulfilled in the [`rustc_errors::HandlerInner`]. Otherwise, a new lint message will be emitted.

The implementation of the `LintExpectationId` required some special handling to make it stable between sessions. Lints can be emitted during [`EarlyLintPass`]es. At this stage, it's not possible to create a stable identifier. The level instead stores an unstable identifier, which is later converted to a stable `LintExpectationId`.

### Followup TO-DOs
All open TO-DOs have been marked with `FIXME` comments in the code. This is the combined list of them:

* [ ] The current implementation doesn't cover cases where the `unfulfilled_lint_expectations` lint is actually expected by another `expect` attribute.
   * This should be easily possible, but I wanted to get some feedback before putting more work into this.
   * This could also be done in a new PR to not add to much more code to this one
* [ ] Update unstable documentation to reflect this change.
* [ ] Update unstable expectation ids in [`HandlerInner::stashed_diagnostics`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_errors/struct.HandlerInner.html#structfield.stashed_diagnostics)

### Open questions
I also have a few open questions where I would like to get feedback on:
1. The RFC discussion included a suggestion to change the `expect` attribute to something else. (Initiated by `@Ixrec` [here](https://github.com/rust-lang/rfcs/pull/2383#issuecomment-378424091), suggestion from `@scottmcm` to use `#[should_lint(...)]` [here](https://github.com/rust-lang/rfcs/pull/2383#issuecomment-378648877)). No real conclusion was drawn on that point from my understanding. Is this still open for discussion, or was this discarded with the merge of the RFC?
2. How should the expect attribute deal with the new `force-warn` lint level?

---

This approach was inspired by a discussion with `@LeSeulArtichaut.`

RFC tracking issue: #54503

Mentoring/Implementation issue: #85549

[`LintLevelsBuilder`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/levels/struct.LintLevelsBuilder.html
[`LintLevelsMap`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/lint/struct.LintLevelMap.html
[`lint_levels()`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/context/struct.TyCtxt.html#method.lint_levels
[`rustc_errors::HandlerInner`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_errors/struct.HandlerInner.html
[`EarlyLintPass`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.EarlyLintPass.html
This commit is contained in:
bors 2022-03-03 18:59:32 +00:00
commit 10913c0001
37 changed files with 1040 additions and 26 deletions
Download patch file Download diff file Expand all files Collapse all files

93
compiler/rustc_errors/src/lib.rs
View file

@ -20,11 +20,12 @@ extern crate tracing;
pub use emitter::ColorConfig;
use rustc_lint_defs::LintExpectationId;
use Level::*;
use emitter::{is_case_difference, Emitter, EmitterWriter};
use registry::Registry;
use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap};
use rustc_data_structures::stable_hasher::StableHasher;
use rustc_data_structures::sync::{self, Lock, Lrc};
use rustc_data_structures::AtomicRef;
@ -450,6 +451,22 @@ struct HandlerInner {
deduplicated_warn_count: usize,
future_breakage_diagnostics: Vec<Diagnostic>,
/// Expected [`Diagnostic`]s store a [`LintExpectationId`] as part of
/// the lint level. [`LintExpectationId`]s created early during the compilation
/// (before `HirId`s have been defined) are not stable and can therefore not be
/// stored on disk. This buffer stores these diagnostics until the ID has been
/// replaced by a stable [`LintExpectationId`]. The [`Diagnostic`]s are the
/// submitted for storage and added to the list of fulfilled expectations.
unstable_expect_diagnostics: Vec<Diagnostic>,
/// expected diagnostic will have the level `Expect` which additionally
/// carries the [`LintExpectationId`] of the expectation that can be
/// marked as fulfilled. This is a collection of all [`LintExpectationId`]s
/// that have been marked as fulfilled this way.
///
/// [RFC-2383]: https://rust-lang.github.io/rfcs/2383-lint-reasons.html
fulfilled_expectations: FxHashSet<LintExpectationId>,
}
/// A key denoting where from a diagnostic was stashed.
@ -570,6 +587,8 @@ impl Handler {
emitted_diagnostics: Default::default(),
stashed_diagnostics: Default::default(),
future_breakage_diagnostics: Vec::new(),
unstable_expect_diagnostics: Vec::new(),
fulfilled_expectations: Default::default(),
}),
}
}
@ -677,6 +696,11 @@ impl Handler {
DiagnosticBuilder::new(self, Level::Allow, msg)
}
/// Construct a builder at the `Expect` level with the `msg`.
pub fn struct_expect(&self, msg: &str, id: LintExpectationId) -> DiagnosticBuilder<'_, ()> {
DiagnosticBuilder::new(self, Level::Expect(id), msg)
}
/// Construct a builder at the `Error` level at the given `span` and with the `msg`.
pub fn struct_span_err(
&self,
@ -906,6 +930,48 @@ impl Handler {
pub fn emit_unused_externs(&self, lint_level: &str, unused_externs: &[&str]) {
self.inner.borrow_mut().emit_unused_externs(lint_level, unused_externs)
}
pub fn update_unstable_expectation_id(
&self,
unstable_to_stable: &FxHashMap<LintExpectationId, LintExpectationId>,
) {
let diags = std::mem::take(&mut self.inner.borrow_mut().unstable_expect_diagnostics);
if diags.is_empty() {
return;
}
let mut inner = self.inner.borrow_mut();
for mut diag in diags.into_iter() {
let mut unstable_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);
}
}
/// This methods steals all [`LintExpectationId`]s that are stored inside
/// [`HandlerInner`] and indicate that the linked expectation has been fulfilled.
pub fn steal_fulfilled_expectation_ids(&self) -> FxHashSet<LintExpectationId> {
assert!(
self.inner.borrow().unstable_expect_diagnostics.is_empty(),
"`HandlerInner::unstable_expect_diagnostics` should be empty at this point",
);
std::mem::take(&mut self.inner.borrow_mut().fulfilled_expectations)
}
}
impl HandlerInner {
@ -951,9 +1017,21 @@ impl HandlerInner {
return;
}
// The `LintExpectationId` can be stable or unstable depending on when it was created.
// Diagnostics created before the definition of `HirId`s are unstable and can not yet
// be stored. Instead, they are buffered until the `LintExpectationId` is replaced by
// a stable one by the `LintLevelsBuilder`.
if let Level::Expect(LintExpectationId::Unstable { .. }) = diagnostic.level {
self.unstable_expect_diagnostics.push(diagnostic.clone());
return;
}
(*TRACK_DIAGNOSTICS)(diagnostic);
if diagnostic.level == Allow {
if let Level::Expect(expectation_id) = diagnostic.level {
self.fulfilled_expectations.insert(expectation_id);
return;
} else if diagnostic.level == Allow {
return;
}
@ -1250,6 +1328,7 @@ pub enum Level {
Help,
FailureNote,
Allow,
Expect(LintExpectationId),
}
impl fmt::Display for Level {
@ -1275,7 +1354,7 @@ impl Level {
spec.set_fg(Some(Color::Cyan)).set_intense(true);
}
FailureNote => {}
Allow => unreachable!(),
Allow | Expect(_) => unreachable!(),
}
spec
}
@ -1289,12 +1368,20 @@ impl Level {
Help => "help",
FailureNote => "failure-note",
Allow => panic!("Shouldn't call on allowed error"),
Expect(_) => panic!("Shouldn't call on expected error"),
}
}
pub fn is_failure_note(&self) -> bool {
matches!(*self, FailureNote)
}
pub fn get_expectation_id(&self) -> Option<LintExpectationId> {
match self {
Level::Expect(id) => Some(*id),
_ => None,
}
}
}
// FIXME(eddyb) this doesn't belong here AFAICT, should be moved to callsite.