Skip to content

Commit a48861a

Browse files
committed
Auto merge of #127313 - cjgillot:single-expect, r=jieyouxu
Rewrite lint_expectations in a single pass. This PR aims at reducing the perf regression from #120924 (comment) with drive-by simplifications. Basically, instead of using the lint level builder, which is slow, this PR splits `lint_expectations` logic in 2: - listing the `LintExpectations` is done in `shallow_lint_levels_on`, on a per-owner basis; - building the unstable->stable expectation id map is done by iterating on attributes. r? ghost for perf
2 parents 1a1cc05 + ff1fc68 commit a48861a

15 files changed

+171
-382
lines changed

compiler/rustc_errors/src/diagnostic.rs

+7-12
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use rustc_lint_defs::{Applicability, LintExpectationId};
1212
use rustc_macros::{Decodable, Encodable};
1313
use rustc_span::source_map::Spanned;
1414
use rustc_span::symbol::Symbol;
15-
use rustc_span::{Span, DUMMY_SP};
15+
use rustc_span::{AttrId, Span, DUMMY_SP};
1616
use tracing::debug;
1717

1818
use crate::snippet::Style;
@@ -356,24 +356,19 @@ impl DiagInner {
356356

357357
pub(crate) fn update_unstable_expectation_id(
358358
&mut self,
359-
unstable_to_stable: &FxIndexMap<LintExpectationId, LintExpectationId>,
359+
unstable_to_stable: &FxIndexMap<AttrId, LintExpectationId>,
360360
) {
361361
if let Level::Expect(expectation_id) | Level::ForceWarning(Some(expectation_id)) =
362362
&mut self.level
363+
&& let LintExpectationId::Unstable { attr_id, lint_index } = *expectation_id
363364
{
364-
if expectation_id.is_stable() {
365-
return;
366-
}
367-
368365
// The unstable to stable map only maps the unstable `AttrId` to a stable `HirId` with an attribute index.
369366
// The lint index inside the attribute is manually transferred here.
370-
let lint_index = expectation_id.get_lint_index();
371-
expectation_id.set_lint_index(None);
372-
let mut stable_id = unstable_to_stable
373-
.get(expectation_id)
374-
.expect("each unstable `LintExpectationId` must have a matching stable id")
375-
.normalize();
367+
let Some(stable_id) = unstable_to_stable.get(&attr_id) else {
368+
panic!("{expectation_id:?} must have a matching stable id")
369+
};
376370

371+
let mut stable_id = *stable_id;
377372
stable_id.set_lint_index(lint_index);
378373
*expectation_id = stable_id;
379374
}

compiler/rustc_errors/src/lib.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ use rustc_macros::{Decodable, Encodable};
6969
pub use rustc_span::fatal_error::{FatalError, FatalErrorMarker};
7070
use rustc_span::source_map::SourceMap;
7171
pub use rustc_span::ErrorGuaranteed;
72-
use rustc_span::{Loc, Span, DUMMY_SP};
72+
use rustc_span::{AttrId, Loc, Span, DUMMY_SP};
7373
pub use snippet::Style;
7474
// Used by external projects such as `rust-gpu`.
7575
// See https://github.com/rust-lang/rust/pull/115393.
@@ -1096,7 +1096,7 @@ impl<'a> DiagCtxtHandle<'a> {
10961096

10971097
pub fn update_unstable_expectation_id(
10981098
&self,
1099-
unstable_to_stable: &FxIndexMap<LintExpectationId, LintExpectationId>,
1099+
unstable_to_stable: FxIndexMap<AttrId, LintExpectationId>,
11001100
) {
11011101
let mut inner = self.inner.borrow_mut();
11021102
let diags = std::mem::take(&mut inner.unstable_expect_diagnostics);
@@ -1105,7 +1105,7 @@ impl<'a> DiagCtxtHandle<'a> {
11051105
if !diags.is_empty() {
11061106
inner.suppressed_expected_diag = true;
11071107
for mut diag in diags.into_iter() {
1108-
diag.update_unstable_expectation_id(unstable_to_stable);
1108+
diag.update_unstable_expectation_id(&unstable_to_stable);
11091109

11101110
// Here the diagnostic is given back to `emit_diagnostic` where it was first
11111111
// intercepted. Now it should be processed as usual, since the unstable expectation
@@ -1117,11 +1117,11 @@ impl<'a> DiagCtxtHandle<'a> {
11171117
inner
11181118
.stashed_diagnostics
11191119
.values_mut()
1120-
.for_each(|(diag, _guar)| diag.update_unstable_expectation_id(unstable_to_stable));
1120+
.for_each(|(diag, _guar)| diag.update_unstable_expectation_id(&unstable_to_stable));
11211121
inner
11221122
.future_breakage_diagnostics
11231123
.iter_mut()
1124-
.for_each(|diag| diag.update_unstable_expectation_id(unstable_to_stable));
1124+
.for_each(|diag| diag.update_unstable_expectation_id(&unstable_to_stable));
11251125
}
11261126

11271127
/// This methods steals all [`LintExpectationId`]s that are stored inside
@@ -1567,7 +1567,7 @@ impl DiagCtxtInner {
15671567
if let LintExpectationId::Unstable { .. } = expect_id {
15681568
unreachable!(); // this case was handled at the top of this function
15691569
}
1570-
self.fulfilled_expectations.insert(expect_id.normalize());
1570+
self.fulfilled_expectations.insert(expect_id);
15711571
if let Expect(_) = diagnostic.level {
15721572
// Nothing emitted here for expected lints.
15731573
TRACK_DIAGNOSTIC(diagnostic, &mut |_| None);

compiler/rustc_lint/src/expect.rs

+49-4
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,66 @@
1+
use rustc_data_structures::fx::FxIndexMap;
2+
use rustc_hir::{HirId, CRATE_OWNER_ID};
3+
use rustc_middle::lint::LintExpectation;
14
use rustc_middle::query::Providers;
25
use rustc_middle::ty::TyCtxt;
36
use rustc_session::lint::builtin::UNFULFILLED_LINT_EXPECTATIONS;
4-
use rustc_session::lint::LintExpectationId;
7+
use rustc_session::lint::{Level, LintExpectationId};
58
use rustc_span::Symbol;
69

710
use crate::lints::{Expectation, ExpectationNote};
811

912
pub(crate) fn provide(providers: &mut Providers) {
10-
*providers = Providers { check_expectations, ..*providers };
13+
*providers = Providers { lint_expectations, check_expectations, ..*providers };
14+
}
15+
16+
fn lint_expectations(tcx: TyCtxt<'_>, (): ()) -> Vec<(LintExpectationId, LintExpectation)> {
17+
let krate = tcx.hir_crate_items(());
18+
19+
let mut expectations = Vec::new();
20+
let mut unstable_to_stable_ids = FxIndexMap::default();
21+
22+
let mut record_stable = |attr_id, hir_id, attr_index| {
23+
let expect_id = LintExpectationId::Stable { hir_id, attr_index, lint_index: None };
24+
unstable_to_stable_ids.entry(attr_id).or_insert(expect_id);
25+
};
26+
let mut push_expectations = |owner| {
27+
let lints = tcx.shallow_lint_levels_on(owner);
28+
if lints.expectations.is_empty() {
29+
return;
30+
}
31+
32+
expectations.extend_from_slice(&lints.expectations);
33+
34+
let attrs = tcx.hir_attrs(owner);
35+
for &(local_id, attrs) in attrs.map.iter() {
36+
// Some attributes appear multiple times in HIR, to ensure they are correctly taken
37+
// into account where they matter. This means we cannot just associate the AttrId to
38+
// the first HirId where we see it, but need to check it actually appears in a lint
39+
// level.
40+
// FIXME(cjgillot): Can this cause an attribute to appear in multiple expectation ids?
41+
if !lints.specs.contains_key(&local_id) {
42+
continue;
43+
}
44+
for (attr_index, attr) in attrs.iter().enumerate() {
45+
let Some(Level::Expect(_)) = Level::from_attr(attr) else { continue };
46+
record_stable(attr.id, HirId { owner, local_id }, attr_index.try_into().unwrap());
47+
}
48+
}
49+
};
50+
51+
push_expectations(CRATE_OWNER_ID);
52+
for owner in krate.owners() {
53+
push_expectations(owner);
54+
}
55+
56+
tcx.dcx().update_unstable_expectation_id(unstable_to_stable_ids);
57+
expectations
1158
}
1259

1360
fn check_expectations(tcx: TyCtxt<'_>, tool_filter: Option<Symbol>) {
1461
let lint_expectations = tcx.lint_expectations(());
1562
let fulfilled_expectations = tcx.dcx().steal_fulfilled_expectation_ids();
1663

17-
tracing::debug!(?lint_expectations, ?fulfilled_expectations);
18-
1964
for (id, expectation) in lint_expectations {
2065
// This check will always be true, since `lint_expectations` only
2166
// holds stable ids

0 commit comments

Comments
 (0)