From 231dbbcb6ac0bc32e77dd7f3bf0443f493161c65 Mon Sep 17 00:00:00 2001 From: Martin Nordholts Date: Sun, 31 Dec 2023 13:04:43 +0100 Subject: [PATCH 1/2] rustc_lint: Make `LintLevelsProvider::current_specs()` return `&FxIndexMap` So that lint iteration order becomes predicitable. Discovered with `rustc::potential_query_instability`. --- compiler/rustc_errors/src/diagnostic.rs | 4 +-- compiler/rustc_errors/src/lib.rs | 4 +-- compiler/rustc_lint/src/levels.rs | 34 ++++++++++++------------- compiler/rustc_middle/src/lint.rs | 4 +-- 4 files changed, 23 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index c226b2d41bdef..49431fb7b3f18 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; +use rustc_data_structures::fx::{FxHashMap, FxIndexMap}; 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: &FxHashMap, + unstable_to_stable: &FxIndexMap, ) { 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 e436591fdd99b..e9507dcfed7e4 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::{FxHashMap, FxHashSet, FxIndexMap, FxIndexSet}; +use rustc_data_structures::fx::{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: &FxHashMap, + unstable_to_stable: &FxIndexMap, ) { 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 c229728cb7971..5950bc76adeb7 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::FxHashMap; +use rustc_data_structures::fx::FxIndexMap; 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: FxHashMap, + specs: FxIndexMap, parent: LintStackIndex, } @@ -86,7 +86,7 @@ impl LintLevelSets { &self, lint: &'static Lint, idx: LintStackIndex, - aux: Option<&FxHashMap>, + aux: Option<&FxIndexMap>, sess: &Session, ) -> LevelAndSource { let lint = LintId::of(lint); @@ -101,7 +101,7 @@ impl LintLevelSets { &self, id: LintId, mut idx: LintStackIndex, - aux: Option<&FxHashMap>, + aux: Option<&FxIndexMap>, ) -> (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: FxHashMap::default(), - empty: FxHashMap::default(), + unstable_to_stable_ids: FxIndexMap::default(), + empty: FxIndexMap::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: FxHashMap::default(), + empty: FxIndexMap::default(), attrs, }, lint_added_lints: false, @@ -209,14 +209,14 @@ pub struct TopDown { } pub trait LintLevelsProvider { - fn current_specs(&self) -> &FxHashMap; + fn current_specs(&self) -> &FxIndexMap; 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) -> &FxHashMap { + fn current_specs(&self) -> &FxIndexMap { &self.sets.list[self.cur].specs } @@ -234,12 +234,12 @@ struct LintLevelQueryMap<'tcx> { cur: HirId, specs: ShallowLintLevelMap, /// Empty hash map to simplify code. - empty: FxHashMap, + empty: FxIndexMap, attrs: &'tcx hir::AttributeMap<'tcx>, } impl LintLevelsProvider for LintLevelQueryMap<'_> { - fn current_specs(&self) -> &FxHashMap { + fn current_specs(&self) -> &FxIndexMap { 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: FxHashMap, + unstable_to_stable_ids: FxIndexMap, /// Empty hash map to simplify code. - empty: FxHashMap, + empty: FxIndexMap, } impl LintLevelsProvider for QueryMapExpectationsWrapper<'_> { - fn current_specs(&self) -> &FxHashMap { + fn current_specs(&self) -> &FxIndexMap { 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: FxHashMap::default(), parent: COMMAND_LINE }); + .push(LintSet { specs: FxIndexMap::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: FxHashMap::default(), parent: prev }); + self.provider.sets.list.push(LintSet { specs: FxIndexMap::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) -> &FxHashMap { + fn current_specs(&self) -> &FxIndexMap { self.provider.current_specs() } diff --git a/compiler/rustc_middle/src/lint.rs b/compiler/rustc_middle/src/lint.rs index d34d9160d55bf..ae432a0406567 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::FxHashMap; +use rustc_data_structures::fx::FxIndexMap; 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: From 295d6003acaa8da531248422a9d8567e30aeb52a Mon Sep 17 00:00:00 2001 From: Martin Nordholts Date: Sat, 23 Dec 2023 14:30:54 +0100 Subject: [PATCH 2/2] rustc_lint: Enforce `rustc::potential_query_instability` lint Stop allowing `rustc::potential_query_instability` on all of `rustc_lint` and instead allow it on a case-by-case basis if it is safe to do so. In this particular crate, all lints were safe to allow. --- compiler/rustc_lint/src/context.rs | 2 ++ compiler/rustc_lint/src/context/diagnostics.rs | 5 +++++ compiler/rustc_lint/src/lib.rs | 1 - compiler/rustc_lint/src/non_ascii_idents.rs | 6 ++++++ 4 files changed, 13 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_lint/src/context.rs b/compiler/rustc_lint/src/context.rs index 88a02917f368a..d0fd019a8b122 100644 --- a/compiler/rustc_lint/src/context.rs +++ b/compiler/rustc_lint/src/context.rs @@ -430,6 +430,8 @@ impl LintStore { // Note: find_best_match_for_name depends on the sort order of its input vector. // To ensure deterministic output, sort elements of the lint_groups hash map. // Also, never suggest deprecated lint groups. + // We will soon sort, so the initial order does not matter. + #[allow(rustc::potential_query_instability)] let mut groups: Vec<_> = self .lint_groups .iter() diff --git a/compiler/rustc_lint/src/context/diagnostics.rs b/compiler/rustc_lint/src/context/diagnostics.rs index cfb0dff31f619..75756c6946a6e 100644 --- a/compiler/rustc_lint/src/context/diagnostics.rs +++ b/compiler/rustc_lint/src/context/diagnostics.rs @@ -197,6 +197,8 @@ pub(super) fn builtin( if let Some(ExpectedValues::Some(best_match_values)) = sess.parse_sess.check_config.expecteds.get(&best_match) { + // We will soon sort, so the initial order does not matter. + #[allow(rustc::potential_query_instability)] let mut possibilities = best_match_values.iter().flatten().map(Symbol::as_str).collect::>(); possibilities.sort(); @@ -298,6 +300,9 @@ pub(super) fn builtin( ); }; let mut have_none_possibility = false; + // We later sort possibilities if it is not empty, so the + // order here does not matter. + #[allow(rustc::potential_query_instability)] let possibilities: Vec = values .iter() .inspect(|a| have_none_possibility |= a.is_none()) diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 0fc24e88b3b21..93904fb5c5658 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -25,7 +25,6 @@ //! //! This API is completely unstable and subject to change. -#![allow(rustc::potential_query_instability)] #![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")] #![doc(rust_logo)] #![feature(rustdoc_internals)] diff --git a/compiler/rustc_lint/src/non_ascii_idents.rs b/compiler/rustc_lint/src/non_ascii_idents.rs index 4f92fcd71c63d..08b2bf6af3731 100644 --- a/compiler/rustc_lint/src/non_ascii_idents.rs +++ b/compiler/rustc_lint/src/non_ascii_idents.rs @@ -174,6 +174,8 @@ impl EarlyLintPass for NonAsciiIdents { // Sort by `Span` so that error messages make sense with respect to the // order of identifier locations in the code. + // We will soon sort, so the initial order does not matter. + #[allow(rustc::potential_query_instability)] let mut symbols: Vec<_> = symbols.iter().collect(); symbols.sort_by_key(|k| k.1); @@ -287,6 +289,8 @@ impl EarlyLintPass for NonAsciiIdents { } if has_suspicious { + // The end result is put in `lint_reports` which is sorted. + #[allow(rustc::potential_query_instability)] let verified_augmented_script_sets = script_states .iter() .flat_map(|(k, v)| match v { @@ -299,6 +303,8 @@ impl EarlyLintPass for NonAsciiIdents { let mut lint_reports: BTreeMap<(Span, Vec), AugmentedScriptSet> = BTreeMap::new(); + // The end result is put in `lint_reports` which is sorted. + #[allow(rustc::potential_query_instability)] 'outerloop: for (augment_script_set, usage) in script_states { let ScriptSetUsage::Suspicious(mut ch_list, sp) = usage else { continue };