Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support tool lints with the #[expect] attribute (RFC 2383) #95542

Merged
merged 6 commits into from
May 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,13 @@ struct HandlerInner {

future_breakage_diagnostics: Vec<Diagnostic>,

/// The [`Self::unstable_expect_diagnostics`] should be empty when this struct is
/// dropped. However, it can have values if the compilation is stopped early
/// or is only partially executed. To avoid ICEs, like in rust#94953 we only
/// check if [`Self::unstable_expect_diagnostics`] is empty, if the expectation ids
/// have been converted.
check_unstable_expect_diagnostics: bool,

/// 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
Expand Down Expand Up @@ -497,10 +504,12 @@ impl Drop for HandlerInner {
);
}

assert!(
self.unstable_expect_diagnostics.is_empty(),
"all diagnostics with unstable expectations should have been converted",
);
if self.check_unstable_expect_diagnostics {
assert!(
self.unstable_expect_diagnostics.is_empty(),
"all diagnostics with unstable expectations should have been converted",
);
}
}
}

Expand Down Expand Up @@ -574,6 +583,7 @@ impl Handler {
emitted_diagnostics: Default::default(),
stashed_diagnostics: Default::default(),
future_breakage_diagnostics: Vec::new(),
check_unstable_expect_diagnostics: false,
unstable_expect_diagnostics: Vec::new(),
fulfilled_expectations: Default::default(),
}),
Expand Down Expand Up @@ -988,12 +998,13 @@ impl Handler {
&self,
unstable_to_stable: &FxHashMap<LintExpectationId, LintExpectationId>,
) {
let diags = std::mem::take(&mut self.inner.borrow_mut().unstable_expect_diagnostics);
let mut inner = self.inner.borrow_mut();
let diags = std::mem::take(&mut inner.unstable_expect_diagnostics);
inner.check_unstable_expect_diagnostics = true;
if diags.is_empty() {
return;
}

let mut inner = self.inner.borrow_mut();
for mut diag in diags.into_iter() {
diag.update_unstable_expectation_id(unstable_to_stable);

Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1009,6 +1009,10 @@ fn analysis(tcx: TyCtxt<'_>, (): ()) -> Result<()> {
});
}
);

// This check has to be run after all lints are done processing. We don't
// define a lint filter, as all lint checks should have finished at this point.
sess.time("check_lint_expectations", || tcx.check_expectations(None));
});

Ok(())
Expand Down
12 changes: 10 additions & 2 deletions compiler/rustc_lint/src/expect.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
use crate::builtin;
use rustc_hir::HirId;
use rustc_middle::ty::query::Providers;
use rustc_middle::{lint::LintExpectation, ty::TyCtxt};
use rustc_session::lint::LintExpectationId;
use rustc_span::symbol::sym;
use rustc_span::Symbol;

pub fn check_expectations(tcx: TyCtxt<'_>) {
pub(crate) fn provide(providers: &mut Providers) {
*providers = Providers { check_expectations, ..*providers };
}

fn check_expectations(tcx: TyCtxt<'_>, tool_filter: Option<Symbol>) {
if !tcx.sess.features_untracked().enabled(sym::lint_reasons) {
return;
}
Expand All @@ -13,7 +19,9 @@ pub fn check_expectations(tcx: TyCtxt<'_>) {
let lint_expectations = &tcx.lint_levels(()).lint_expectations;

for (id, expectation) in lint_expectations {
if !fulfilled_expectations.contains(id) {
if !fulfilled_expectations.contains(id)
&& tool_filter.map_or(true, |filter| expectation.lint_tool == Some(filter))
{
// This check will always be true, since `lint_expectations` only
// holds stable ids
if let LintExpectationId::Stable { hir_id, .. } = id {
Expand Down
3 changes: 0 additions & 3 deletions compiler/rustc_lint/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,4 @@ pub fn check_crate<'tcx, T: LateLintPass<'tcx>>(
});
},
);

// This check has to be run after all lints are done processing for this crate
tcx.sess.time("check_lint_expectations", || crate::expect::check_expectations(tcx));
}
25 changes: 18 additions & 7 deletions compiler/rustc_lint/src/levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,12 @@ impl<'s> LintLevelsBuilder<'s> {
};
self.lint_expectations.push((
expect_id,
LintExpectation::new(reason, sp, is_unfulfilled_lint_expectations),
LintExpectation::new(
reason,
sp,
is_unfulfilled_lint_expectations,
tool_name,
),
));
}
let src = LintLevelSource::Node(
Expand Down Expand Up @@ -400,8 +405,10 @@ impl<'s> LintLevelsBuilder<'s> {
self.insert_spec(*id, (level, src));
}
if let Level::Expect(expect_id) = level {
self.lint_expectations
.push((expect_id, LintExpectation::new(reason, sp, false)));
self.lint_expectations.push((
expect_id,
LintExpectation::new(reason, sp, false, tool_name),
));
}
}
Err((Some(ids), ref new_lint_name)) => {
Expand Down Expand Up @@ -444,8 +451,10 @@ impl<'s> LintLevelsBuilder<'s> {
self.insert_spec(*id, (level, src));
}
if let Level::Expect(expect_id) = level {
self.lint_expectations
.push((expect_id, LintExpectation::new(reason, sp, false)));
self.lint_expectations.push((
expect_id,
LintExpectation::new(reason, sp, false, tool_name),
));
}
}
Err((None, _)) => {
Expand Down Expand Up @@ -550,8 +559,10 @@ impl<'s> LintLevelsBuilder<'s> {
}
}
if let Level::Expect(expect_id) = level {
self.lint_expectations
.push((expect_id, LintExpectation::new(reason, sp, false)));
self.lint_expectations.push((
expect_id,
LintExpectation::new(reason, sp, false, tool_name),
));
}
} else {
panic!("renamed lint does not exist: {}", new_name);
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ pub use rustc_session::lint::{LintArray, LintPass};

pub fn provide(providers: &mut Providers) {
levels::provide(providers);
expect::provide(providers);
*providers = Providers { lint_mod, ..*providers };
}

Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_middle/src/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,15 +210,20 @@ pub struct LintExpectation {
/// adjusted to include an additional note. Therefore, we have to track if
/// the expectation is for the lint.
pub is_unfulfilled_lint_expectations: bool,
/// This will hold the name of the tool that this lint belongs to. For
/// the lint `clippy::some_lint` the tool would be `clippy`, the same
/// goes for `rustdoc`. This will be `None` for rustc lints
pub lint_tool: Option<Symbol>,
}

impl LintExpectation {
pub fn new(
reason: Option<Symbol>,
emission_span: Span,
is_unfulfilled_lint_expectations: bool,
lint_tool: Option<Symbol>,
) -> Self {
Self { reason, emission_span, is_unfulfilled_lint_expectations }
Self { reason, emission_span, is_unfulfilled_lint_expectations, lint_tool }
}
}

Expand Down
19 changes: 19 additions & 0 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,25 @@ rustc_queries! {
desc { "running analysis passes on this crate" }
}

/// This query checks the fulfillment of collected lint expectations.
/// All lint emitting queries have to be done before this is executed
/// to ensure that all expectations can be fulfilled.
///
/// This is an extra query to enable other drivers (like rustdoc) to
/// only execute a small subset of the `analysis` query, while allowing
/// lints to be expected. In rustc, this query will be executed as part of
/// the `analysis` query and doesn't have to be called a second time.
///
/// Tools can additionally pass in a tool filter. That will restrict the
/// expectations to only trigger for lints starting with the listed tool
/// name. This is useful for cases were not all linting code from rustc
/// was called. With the default `None` all registered lints will also
/// be checked for expectation fulfillment.
query check_expectations(key: Option<Symbol>) -> () {
eval_always
desc { "checking lint expectations (RFC 2383)" }
}

/// Maps from the `DefId` of an item (trait/struct/enum/fn) to its
/// associated generics.
query generics_of(key: DefId) -> ty::Generics {
Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_query_impl/src/keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,16 @@ impl Key for Symbol {
}
}

impl Key for Option<Symbol> {
#[inline(always)]
fn query_crate_is_local(&self) -> bool {
true
}
fn default_span(&self, _tcx: TyCtxt<'_>) -> Span {
DUMMY_SP
}
}

/// Canonical query goals correspond to abstract trait operations that
/// are not tied to any crate in particular.
impl<'tcx, T> Key for Canonical<'tcx, T> {
Expand Down
4 changes: 4 additions & 0 deletions src/librustdoc/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,8 @@ crate fn create_config(
rustc_lint::builtin::RENAMED_AND_REMOVED_LINTS.name.to_string(),
rustc_lint::builtin::UNKNOWN_LINTS.name.to_string(),
rustc_lint::builtin::UNEXPECTED_CFGS.name.to_string(),
// this lint is needed to support `#[expect]` attributes
rustc_lint::builtin::UNFULFILLED_LINT_EXPECTATIONS.name.to_string(),
];
lints_to_show.extend(crate::lint::RUSTDOC_LINTS.iter().map(|lint| lint.name.to_string()));

Expand Down Expand Up @@ -463,6 +465,8 @@ crate fn run_global_ctxt(
}
}

tcx.sess.time("check_lint_expectations", || tcx.check_expectations(Some(sym::rustdoc)));

if tcx.sess.diagnostic().has_errors_or_lint_errors().is_some() {
rustc_errors::FatalError.raise();
}
Expand Down
Loading