From e620807aa7957febe3a523cfb93cfa7967e19129 Mon Sep 17 00:00:00 2001 From: Martin Nordholts Date: Mon, 1 Jan 2024 11:13:03 +0100 Subject: [PATCH] Revert "rustc_lint: Make `LintLevelsProvider::current_specs()` return `&FxIndexMap`" This reverts commit 231dbbcb6ac0bc32e77dd7f3bf0443f493161c65. It regressed performance. See # 119251. It is not a pure revert because we also need to allow `rustc::potential_query_instability` in one more place now because of the revert. --- compiler/rustc_errors/src/diagnostic.rs | 4 +-- compiler/rustc_errors/src/lib.rs | 4 +-- compiler/rustc_lint/src/levels.rs | 40 ++++++++++++++----------- compiler/rustc_middle/src/lint.rs | 4 +-- 4 files changed, 29 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 49431fb7b3f18..c226b2d41bdef 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -3,7 +3,7 @@ use crate::{ CodeSuggestion, DiagnosticBuilder, DiagnosticMessage, EmissionGuarantee, Level, MultiSpan, SubdiagnosticMessage, Substitution, SubstitutionPart, SuggestionStyle, }; -use rustc_data_structures::fx::{FxHashMap, FxIndexMap}; +use rustc_data_structures::fx::FxHashMap; use rustc_error_messages::fluent_value_from_str_list_sep_by_and; use rustc_error_messages::FluentValue; use rustc_lint_defs::{Applicability, LintExpectationId}; @@ -259,7 +259,7 @@ impl Diagnostic { pub(crate) fn update_unstable_expectation_id( &mut self, - unstable_to_stable: &FxIndexMap, + unstable_to_stable: &FxHashMap, ) { if let Level::Expect(expectation_id) | Level::Warning(Some(expectation_id)) = &mut self.level diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index e9507dcfed7e4..e436591fdd99b 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -55,7 +55,7 @@ pub use termcolor::{Color, ColorSpec, WriteColor}; use crate::diagnostic_impls::{DelayedAtWithNewline, DelayedAtWithoutNewline}; use emitter::{is_case_difference, DynEmitter, Emitter, EmitterWriter}; use registry::Registry; -use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet}; +use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap, FxIndexSet}; use rustc_data_structures::stable_hasher::{Hash128, StableHasher}; use rustc_data_structures::sync::{Lock, Lrc}; use rustc_data_structures::AtomicRef; @@ -1318,7 +1318,7 @@ impl DiagCtxt { pub fn update_unstable_expectation_id( &self, - unstable_to_stable: &FxIndexMap, + unstable_to_stable: &FxHashMap, ) { let mut inner = self.inner.borrow_mut(); let diags = std::mem::take(&mut inner.unstable_expect_diagnostics); diff --git a/compiler/rustc_lint/src/levels.rs b/compiler/rustc_lint/src/levels.rs index 5950bc76adeb7..081b923669d05 100644 --- a/compiler/rustc_lint/src/levels.rs +++ b/compiler/rustc_lint/src/levels.rs @@ -15,7 +15,7 @@ use crate::{ }; use rustc_ast as ast; use rustc_ast_pretty::pprust; -use rustc_data_structures::fx::FxIndexMap; +use rustc_data_structures::fx::FxHashMap; use rustc_errors::{DecorateLint, DiagnosticBuilder, DiagnosticMessage, MultiSpan}; use rustc_feature::{Features, GateIssue}; use rustc_hir as hir; @@ -73,7 +73,7 @@ rustc_index::newtype_index! { struct LintSet { // -A,-W,-D flags, a `Symbol` for the flag itself and `Level` for which // flag. - specs: FxIndexMap, + specs: FxHashMap, parent: LintStackIndex, } @@ -86,7 +86,7 @@ impl LintLevelSets { &self, lint: &'static Lint, idx: LintStackIndex, - aux: Option<&FxIndexMap>, + aux: Option<&FxHashMap>, sess: &Session, ) -> LevelAndSource { let lint = LintId::of(lint); @@ -101,7 +101,7 @@ impl LintLevelSets { &self, id: LintId, mut idx: LintStackIndex, - aux: Option<&FxIndexMap>, + aux: Option<&FxHashMap>, ) -> (Option, LintLevelSource) { if let Some(specs) = aux { if let Some(&(level, src)) = specs.get(&id) { @@ -132,8 +132,8 @@ fn lint_expectations(tcx: TyCtxt<'_>, (): ()) -> Vec<(LintExpectationId, LintExp cur: hir::CRATE_HIR_ID, specs: ShallowLintLevelMap::default(), expectations: Vec::new(), - unstable_to_stable_ids: FxIndexMap::default(), - empty: FxIndexMap::default(), + unstable_to_stable_ids: FxHashMap::default(), + empty: FxHashMap::default(), }, lint_added_lints: false, store, @@ -161,7 +161,7 @@ fn shallow_lint_levels_on(tcx: TyCtxt<'_>, owner: hir::OwnerId) -> ShallowLintLe tcx, cur: owner.into(), specs: ShallowLintLevelMap::default(), - empty: FxIndexMap::default(), + empty: FxHashMap::default(), attrs, }, lint_added_lints: false, @@ -209,14 +209,14 @@ pub struct TopDown { } pub trait LintLevelsProvider { - fn current_specs(&self) -> &FxIndexMap; + fn current_specs(&self) -> &FxHashMap; fn insert(&mut self, id: LintId, lvl: LevelAndSource); fn get_lint_level(&self, lint: &'static Lint, sess: &Session) -> LevelAndSource; fn push_expectation(&mut self, _id: LintExpectationId, _expectation: LintExpectation) {} } impl LintLevelsProvider for TopDown { - fn current_specs(&self) -> &FxIndexMap { + fn current_specs(&self) -> &FxHashMap { &self.sets.list[self.cur].specs } @@ -234,12 +234,12 @@ struct LintLevelQueryMap<'tcx> { cur: HirId, specs: ShallowLintLevelMap, /// Empty hash map to simplify code. - empty: FxIndexMap, + empty: FxHashMap, attrs: &'tcx hir::AttributeMap<'tcx>, } impl LintLevelsProvider for LintLevelQueryMap<'_> { - fn current_specs(&self) -> &FxIndexMap { + fn current_specs(&self) -> &FxHashMap { self.specs.specs.get(&self.cur.local_id).unwrap_or(&self.empty) } fn insert(&mut self, id: LintId, lvl: LevelAndSource) { @@ -257,13 +257,13 @@ struct QueryMapExpectationsWrapper<'tcx> { /// Level map for `cur`. specs: ShallowLintLevelMap, expectations: Vec<(LintExpectationId, LintExpectation)>, - unstable_to_stable_ids: FxIndexMap, + unstable_to_stable_ids: FxHashMap, /// Empty hash map to simplify code. - empty: FxIndexMap, + empty: FxHashMap, } impl LintLevelsProvider for QueryMapExpectationsWrapper<'_> { - fn current_specs(&self) -> &FxIndexMap { + fn current_specs(&self) -> &FxHashMap { self.specs.specs.get(&self.cur.local_id).unwrap_or(&self.empty) } fn insert(&mut self, id: LintId, lvl: LevelAndSource) { @@ -486,7 +486,7 @@ impl<'s> LintLevelsBuilder<'s, TopDown> { .provider .sets .list - .push(LintSet { specs: FxIndexMap::default(), parent: COMMAND_LINE }); + .push(LintSet { specs: FxHashMap::default(), parent: COMMAND_LINE }); self.add_command_line(); } @@ -512,7 +512,7 @@ impl<'s> LintLevelsBuilder<'s, TopDown> { ) -> BuilderPush { let prev = self.provider.cur; self.provider.cur = - self.provider.sets.list.push(LintSet { specs: FxIndexMap::default(), parent: prev }); + self.provider.sets.list.push(LintSet { specs: FxHashMap::default(), parent: prev }); self.add(attrs, is_crate_node, source_hir_id); @@ -547,7 +547,7 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> { self.features } - fn current_specs(&self) -> &FxIndexMap { + fn current_specs(&self) -> &FxHashMap { self.provider.current_specs() } @@ -1030,6 +1030,12 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> { } if self.lint_added_lints && !is_crate_node { + // We break on the first lint, and it does not matter which one. + // Strictly speaking the lint we use shall be deterministic, but + // this code path is very edge-casey, and introducing determinism + // regresses performance. See + // https://github.com/rust-lang/rust/pull/119251#issuecomment-1873035640. + #[allow(rustc::potential_query_instability)] for (id, &(level, ref src)) in self.current_specs().iter() { if !id.lint.crate_level_only { continue; diff --git a/compiler/rustc_middle/src/lint.rs b/compiler/rustc_middle/src/lint.rs index ae432a0406567..d34d9160d55bf 100644 --- a/compiler/rustc_middle/src/lint.rs +++ b/compiler/rustc_middle/src/lint.rs @@ -1,6 +1,6 @@ use std::cmp; -use rustc_data_structures::fx::FxIndexMap; +use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::sorted_map::SortedMap; use rustc_errors::{Diagnostic, DiagnosticBuilder, DiagnosticId, DiagnosticMessage, MultiSpan}; use rustc_hir::{HirId, ItemLocalId}; @@ -61,7 +61,7 @@ pub type LevelAndSource = (Level, LintLevelSource); /// by the attributes for *a single HirId*. #[derive(Default, Debug, HashStable)] pub struct ShallowLintLevelMap { - pub specs: SortedMap>, + pub specs: SortedMap>, } /// From an initial level and source, verify the effect of special annotations: