From fb9cc984d0118c4d8316bca0535be43e45292c78 Mon Sep 17 00:00:00 2001 From: ismailarilik Date: Thu, 3 Oct 2024 21:59:52 +0300 Subject: [PATCH 01/15] Handle `rustc_resolve` cases of `rustc::potential_query_instability` lint --- compiler/rustc_middle/src/ty/mod.rs | 2 +- compiler/rustc_resolve/src/diagnostics.rs | 4 ++-- compiler/rustc_resolve/src/imports.rs | 8 +++---- compiler/rustc_resolve/src/late.rs | 14 +++++------ .../rustc_resolve/src/late/diagnostics.rs | 2 +- compiler/rustc_resolve/src/lib.rs | 23 +++++++++---------- compiler/rustc_resolve/src/macros.rs | 4 ++-- 7 files changed, 28 insertions(+), 29 deletions(-) diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index ed24fcc7eb88a..0c317741a9fd6 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -175,7 +175,7 @@ pub struct ResolverGlobalCtxt { /// Item with a given `LocalDefId` was defined during macro expansion with ID `ExpnId`. pub expn_that_defined: FxHashMap, pub effective_visibilities: EffectiveVisibilities, - pub extern_crate_map: FxHashMap, + pub extern_crate_map: FxIndexMap, pub maybe_unused_trait_imports: FxIndexSet, pub module_children: LocalDefIdMap>, pub glob_map: FxHashMap>, diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 5437ca65935f8..75bd4d36d3362 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -5,7 +5,7 @@ use rustc_ast::{ self as ast, CRATE_NODE_ID, Crate, ItemKind, MetaItemInner, MetaItemKind, ModKind, NodeId, Path, }; use rustc_ast_pretty::pprust; -use rustc_data_structures::fx::FxHashSet; +use rustc_data_structures::fx::{FxHashSet, FxIndexSet}; use rustc_errors::codes::*; use rustc_errors::{ Applicability, Diag, DiagCtxtHandle, ErrorGuaranteed, MultiSpan, SuggestionStyle, @@ -2842,7 +2842,7 @@ fn show_candidates( let mut kinds = accessible_path_strings .iter() .map(|(_, descr, _, _, _)| *descr) - .collect::>() + .collect::>() .into_iter(); let kind = if let Some(kind) = kinds.next() && let None = kinds.next() diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index 51fbcb8ebb805..f767934e10d4d 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -4,7 +4,7 @@ use std::cell::Cell; use std::mem; use rustc_ast::NodeId; -use rustc_data_structures::fx::FxHashSet; +use rustc_data_structures::fx::{FxHashSet, FxIndexSet}; use rustc_data_structures::intern::Interned; use rustc_errors::codes::*; use rustc_errors::{Applicability, MultiSpan, pluralize, struct_span_code_err}; @@ -220,7 +220,7 @@ impl<'ra> ImportData<'ra> { pub(crate) struct NameResolution<'ra> { /// Single imports that may define the name in the namespace. /// Imports are arena-allocated, so it's ok to use pointers as keys. - pub single_imports: FxHashSet>, + pub single_imports: FxIndexSet>, /// The least shadowable known binding for this name, or None if there are no known bindings. pub binding: Option>, pub shadowed_glob: Option>, @@ -482,7 +482,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let key = BindingKey::new(target, ns); let _ = this.try_define(import.parent_scope.module, key, dummy_binding, false); this.update_resolution(import.parent_scope.module, key, false, |_, resolution| { - resolution.single_imports.remove(&import); + resolution.single_imports.shift_remove(&import); }) }); self.record_use(target, dummy_binding, Used::Other); @@ -837,7 +837,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } let key = BindingKey::new(target, ns); this.update_resolution(parent, key, false, |_, resolution| { - resolution.single_imports.remove(&import); + resolution.single_imports.shift_remove(&import); }); } } diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 033cd7d58705d..ec3289017e758 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -15,7 +15,7 @@ use std::mem::{replace, swap, take}; use rustc_ast::ptr::P; use rustc_ast::visit::{AssocCtxt, BoundKind, FnCtxt, FnKind, Visitor, visit_opt, walk_list}; use rustc_ast::*; -use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap}; +use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap, FxIndexSet}; use rustc_errors::codes::*; use rustc_errors::{Applicability, DiagArgValue, IntoDiagArg, StashKey, Suggestions}; use rustc_hir::def::Namespace::{self, *}; @@ -44,7 +44,7 @@ mod diagnostics; type Res = def::Res; -type IdentMap = FxHashMap; +type IdentMap = FxIndexMap; use diagnostics::{ElisionFnParameter, LifetimeElisionCandidate, MissingLifetime}; @@ -640,7 +640,7 @@ struct DiagMetadata<'ast> { /// A list of labels as of yet unused. Labels will be removed from this map when /// they are used (in a `break` or `continue` statement) - unused_labels: FxHashMap, + unused_labels: FxIndexMap, /// Only used for better errors on `let x = { foo: bar };`. /// In the case of a parse error with `let x = { foo: bar, };`, this isn't needed, it's only @@ -1550,7 +1550,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> { // Allow all following defaults to refer to this type parameter. forward_ty_ban_rib .bindings - .remove(&Ident::with_dummy_span(param.ident.name)); + .shift_remove(&Ident::with_dummy_span(param.ident.name)); } GenericParamKind::Const { ref ty, kw_span: _, ref default } => { // Const parameters can't have param bounds. @@ -1578,7 +1578,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> { // Allow all following defaults to refer to this const parameter. forward_const_ban_rib .bindings - .remove(&Ident::with_dummy_span(param.ident.name)); + .shift_remove(&Ident::with_dummy_span(param.ident.name)); } } } @@ -2236,7 +2236,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> { let local_candidates = self.lifetime_elision_candidates.take(); if let Some(candidates) = local_candidates { - let distinct: FxHashSet<_> = candidates.iter().map(|(res, _)| *res).collect(); + let distinct: FxIndexSet<_> = candidates.iter().map(|(res, _)| *res).collect(); let lifetime_count = distinct.len(); if lifetime_count != 0 { parameter_info.push(ElisionFnParameter { @@ -4621,7 +4621,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> { Ok((node_id, _)) => { // Since this res is a label, it is never read. self.r.label_res_map.insert(expr.id, node_id); - self.diag_metadata.unused_labels.remove(&node_id); + self.diag_metadata.unused_labels.shift_remove(&node_id); } Err(error) => { self.report_error(label.ident.span, error); diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index f0632a21091bb..af0fecb59329b 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -960,7 +960,7 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> { Applicability::MaybeIncorrect, ); // Do not lint against unused label when we suggest them. - self.diag_metadata.unused_labels.remove(node_id); + self.diag_metadata.unused_labels.shift_remove(node_id); } } } diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 35d491cfc18e7..e913b196ca0f9 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -9,7 +9,6 @@ // tidy-alphabetical-start #![allow(internal_features)] #![allow(rustc::diagnostic_outside_of_impl)] -#![allow(rustc::potential_query_instability)] #![allow(rustc::untranslatable_diagnostic)] #![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")] #![doc(rust_logo)] @@ -1022,7 +1021,7 @@ pub struct Resolver<'ra, 'tcx> { graph_root: Module<'ra>, prelude: Option>, - extern_prelude: FxHashMap>, + extern_prelude: FxIndexMap>, /// N.B., this is used only for better diagnostics, not name resolution itself. field_names: LocalDefIdMap>, @@ -1055,7 +1054,7 @@ pub struct Resolver<'ra, 'tcx> { extra_lifetime_params_map: NodeMap>, /// `CrateNum` resolutions of `extern crate` items. - extern_crate_map: FxHashMap, + extern_crate_map: FxIndexMap, module_children: LocalDefIdMap>, trait_map: NodeMap>, @@ -1078,7 +1077,7 @@ pub struct Resolver<'ra, 'tcx> { /// some AST passes can generate identifiers that only resolve to local or /// lang items. empty_module: Module<'ra>, - module_map: FxHashMap>, + module_map: FxIndexMap>, binding_parent_modules: FxHashMap, Module<'ra>>, underscore_disambiguator: u32, @@ -1114,16 +1113,16 @@ pub struct Resolver<'ra, 'tcx> { macro_names: FxHashSet, builtin_macros: FxHashMap, registered_tools: &'tcx RegisteredTools, - macro_use_prelude: FxHashMap>, + macro_use_prelude: FxIndexMap>, macro_map: FxHashMap, dummy_ext_bang: Lrc, dummy_ext_derive: Lrc, non_macro_attr: MacroData, local_macro_def_scopes: FxHashMap>, ast_transform_scopes: FxHashMap>, - unused_macros: FxHashMap, + unused_macros: FxIndexMap, /// A map from the macro to all its potentially unused arms. - unused_macro_rules: FxIndexMap>, + unused_macro_rules: FxIndexMap>, proc_macro_stubs: FxHashSet, /// Traces collected during macro resolution and validated when it's complete. single_segment_macro_resolutions: @@ -1235,7 +1234,7 @@ impl<'ra> ResolverArenas<'ra> { expn_id: ExpnId, span: Span, no_implicit_prelude: bool, - module_map: &mut FxHashMap>, + module_map: &mut FxIndexMap>, module_self_bindings: &mut FxHashMap, NameBinding<'ra>>, ) -> Module<'ra> { let module = Module(Interned::new_unchecked(self.modules.alloc(ModuleData::new( @@ -1380,7 +1379,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { arenas: &'ra ResolverArenas<'ra>, ) -> Resolver<'ra, 'tcx> { let root_def_id = CRATE_DEF_ID.to_def_id(); - let mut module_map = FxHashMap::default(); + let mut module_map = FxIndexMap::default(); let mut module_self_bindings = FxHashMap::default(); let graph_root = arenas.new_module( None, @@ -1397,7 +1396,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ExpnId::root(), DUMMY_SP, true, - &mut FxHashMap::default(), + &mut FxIndexMap::default(), &mut FxHashMap::default(), ); @@ -1413,7 +1412,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let mut invocation_parents = FxHashMap::default(); invocation_parents.insert(LocalExpnId::ROOT, InvocationParent::ROOT); - let mut extern_prelude: FxHashMap> = tcx + let mut extern_prelude: FxIndexMap> = tcx .sess .opts .externs @@ -1513,7 +1512,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { macro_names: FxHashSet::default(), builtin_macros: Default::default(), registered_tools, - macro_use_prelude: FxHashMap::default(), + macro_use_prelude: FxIndexMap::default(), macro_map: FxHashMap::default(), dummy_ext_bang: Lrc::new(SyntaxExtension::dummy_bang(edition)), dummy_ext_derive: Lrc::new(SyntaxExtension::dummy_derive(edition)), diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index a9ebffea8a10b..bb99565f0274e 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -341,7 +341,7 @@ impl<'ra, 'tcx> ResolverExpand for Resolver<'ra, 'tcx> { fn record_macro_rule_usage(&mut self, id: NodeId, rule_i: usize) { let did = self.local_def_id(id); if let Some(rules) = self.unused_macro_rules.get_mut(&did) { - rules.remove(&rule_i); + rules.shift_remove(&rule_i); } } @@ -602,7 +602,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { match res { Res::Def(DefKind::Macro(_), def_id) => { if let Some(def_id) = def_id.as_local() { - self.unused_macros.remove(&def_id); + self.unused_macros.shift_remove(&def_id); if self.proc_macro_stubs.contains(&def_id) { self.dcx().emit_err(errors::ProcMacroSameCrate { span: path.span, From da177090151461fa8151eff966c5817697ab0027 Mon Sep 17 00:00:00 2001 From: ismailarilik Date: Fri, 25 Oct 2024 09:41:30 +0300 Subject: [PATCH 02/15] Add requested changes by reviewer --- compiler/rustc_middle/src/ty/mod.rs | 2 +- compiler/rustc_resolve/src/diagnostics.rs | 21 ++++++++++++++--- compiler/rustc_resolve/src/late.rs | 23 +++++++++++-------- .../rustc_resolve/src/late/diagnostics.rs | 6 ++++- compiler/rustc_resolve/src/lib.rs | 20 ++++++++-------- compiler/rustc_resolve/src/macros.rs | 8 +++++-- 6 files changed, 54 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index 0c317741a9fd6..ed24fcc7eb88a 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -175,7 +175,7 @@ pub struct ResolverGlobalCtxt { /// Item with a given `LocalDefId` was defined during macro expansion with ID `ExpnId`. pub expn_that_defined: FxHashMap, pub effective_visibilities: EffectiveVisibilities, - pub extern_crate_map: FxIndexMap, + pub extern_crate_map: FxHashMap, pub maybe_unused_trait_imports: FxIndexSet, pub module_children: LocalDefIdMap>, pub glob_map: FxHashMap>, diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 75bd4d36d3362..dafb944a6db85 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -5,7 +5,7 @@ use rustc_ast::{ self as ast, CRATE_NODE_ID, Crate, ItemKind, MetaItemInner, MetaItemKind, ModKind, NodeId, Path, }; use rustc_ast_pretty::pprust; -use rustc_data_structures::fx::{FxHashSet, FxIndexSet}; +use rustc_data_structures::fx::FxHashSet; use rustc_errors::codes::*; use rustc_errors::{ Applicability, Diag, DiagCtxtHandle, ErrorGuaranteed, MultiSpan, SuggestionStyle, @@ -1086,6 +1086,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { this.add_module_candidates(module, &mut suggestions, filter_fn, None); } Scope::MacroUsePrelude => { + // The suggestions are deterministically sorted later + #[allow(rustc::potential_query_instability)] suggestions.extend(this.macro_use_prelude.iter().filter_map( |(name, binding)| { let res = binding.res(); @@ -1104,6 +1106,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } } Scope::ExternPrelude => { + // The suggestions are deterministically sorted later + #[allow(rustc::potential_query_instability)] suggestions.extend(this.extern_prelude.iter().filter_map(|(ident, _)| { let res = Res::Def(DefKind::Mod, CRATE_DEF_ID.to_def_id()); filter_fn(res).then_some(TypoSuggestion::typo_from_ident(*ident, res)) @@ -1362,6 +1366,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ); if lookup_ident.span.at_least_rust_2018() { + // It will be sorted later. + #[allow(rustc::potential_query_instability)] for ident in self.extern_prelude.clone().into_keys() { if ident.span.from_expansion() { // Idents are adjusted to the root context before being @@ -1467,7 +1473,11 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { return; } - let unused_macro = self.unused_macros.iter().find_map(|(def_id, (_, unused_ident))| { + // Make ordering consistent before iteration + #[allow(rustc::potential_query_instability)] + let mut unused_macros: Vec<_> = self.unused_macros.iter().collect(); + unused_macros.sort_by_key(|&(_, (key, _))| key); + let unused_macro = unused_macros.iter().find_map(|(def_id, (_, unused_ident))| { if unused_ident.name == ident.name { Some((def_id, unused_ident)) } else { None } }); @@ -1954,6 +1964,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ident: Symbol, current_module: Module<'ra>, ) -> Option { + // It will be sorted later. + #[allow(rustc::potential_query_instability)] let mut candidates = self .extern_prelude .keys() @@ -2342,6 +2354,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { // Sort extern crate names in *reverse* order to get // 1) some consistent ordering for emitted diagnostics, and // 2) `std` suggestions before `core` suggestions. + #[allow(rustc::potential_query_instability)] let mut extern_crate_names = self.extern_prelude.keys().map(|ident| ident.name).collect::>(); extern_crate_names.sort_by(|a, b| b.as_str().partial_cmp(a.as_str()).unwrap()); @@ -2839,10 +2852,12 @@ fn show_candidates( } else { // Get the unique item kinds and if there's only one, we use the right kind name // instead of the more generic "items". + // Ordering is not important here. + #[allow(rustc::potential_query_instability)] let mut kinds = accessible_path_strings .iter() .map(|(_, descr, _, _, _)| *descr) - .collect::>() + .collect::>() .into_iter(); let kind = if let Some(kind) = kinds.next() && let None = kinds.next() diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index ec3289017e758..606b3695903a4 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -15,7 +15,7 @@ use std::mem::{replace, swap, take}; use rustc_ast::ptr::P; use rustc_ast::visit::{AssocCtxt, BoundKind, FnCtxt, FnKind, Visitor, visit_opt, walk_list}; use rustc_ast::*; -use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap, FxIndexSet}; +use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap}; use rustc_errors::codes::*; use rustc_errors::{Applicability, DiagArgValue, IntoDiagArg, StashKey, Suggestions}; use rustc_hir::def::Namespace::{self, *}; @@ -44,8 +44,6 @@ mod diagnostics; type Res = def::Res; -type IdentMap = FxIndexMap; - use diagnostics::{ElisionFnParameter, LifetimeElisionCandidate, MissingLifetime}; #[derive(Copy, Clone, Debug)] @@ -261,7 +259,7 @@ impl RibKind<'_> { /// resolving, the name is looked up from inside out. #[derive(Debug)] pub(crate) struct Rib<'ra, R = Res> { - pub bindings: IdentMap, + pub bindings: FxIndexMap, pub kind: RibKind<'ra>, } @@ -640,7 +638,7 @@ struct DiagMetadata<'ast> { /// A list of labels as of yet unused. Labels will be removed from this map when /// they are used (in a `break` or `continue` statement) - unused_labels: FxIndexMap, + unused_labels: FxHashMap, /// Only used for better errors on `let x = { foo: bar };`. /// In the case of a parse error with `let x = { foo: bar, };`, this isn't needed, it's only @@ -2236,7 +2234,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> { let local_candidates = self.lifetime_elision_candidates.take(); if let Some(candidates) = local_candidates { - let distinct: FxIndexSet<_> = candidates.iter().map(|(res, _)| *res).collect(); + let distinct: FxHashSet<_> = candidates.iter().map(|(res, _)| *res).collect(); let lifetime_count = distinct.len(); if lifetime_count != 0 { parameter_info.push(ElisionFnParameter { @@ -2260,6 +2258,8 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> { } })); } + // Ordering is not important here. + #[allow(rustc::potential_query_instability)] let mut distinct_iter = distinct.into_iter(); if let Some(res) = distinct_iter.next() { match elision_lifetime { @@ -3868,7 +3868,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> { } } - fn innermost_rib_bindings(&mut self, ns: Namespace) -> &mut IdentMap { + fn innermost_rib_bindings(&mut self, ns: Namespace) -> &mut FxIndexMap { &mut self.ribs[ns].last_mut().unwrap().bindings } @@ -4621,7 +4621,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> { Ok((node_id, _)) => { // Since this res is a label, it is never read. self.r.label_res_map.insert(expr.id, node_id); - self.diag_metadata.unused_labels.shift_remove(&node_id); + self.diag_metadata.unused_labels.remove(&node_id); } Err(error) => { self.report_error(label.ident.span, error); @@ -5046,7 +5046,12 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let mut late_resolution_visitor = LateResolutionVisitor::new(self); late_resolution_visitor.resolve_doc_links(&krate.attrs, MaybeExported::Ok(CRATE_NODE_ID)); visit::walk_crate(&mut late_resolution_visitor, krate); - for (id, span) in late_resolution_visitor.diag_metadata.unused_labels.iter() { + // Make ordering consistent before iteration + #[allow(rustc::potential_query_instability)] + let mut unused_labels: Vec<_> = + late_resolution_visitor.diag_metadata.unused_labels.iter().collect(); + unused_labels.sort_by_key(|&(key, _)| key); + for (id, span) in unused_labels { self.lint_buffer.buffer_lint( lint::builtin::UNUSED_LABELS, *id, diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index af0fecb59329b..4ebc2afd68d23 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -960,7 +960,7 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> { Applicability::MaybeIncorrect, ); // Do not lint against unused label when we suggest them. - self.diag_metadata.unused_labels.shift_remove(node_id); + self.diag_metadata.unused_labels.remove(node_id); } } } @@ -1928,6 +1928,8 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> { let Some(default_trait) = default_trait else { return; }; + // The ordering is not important here. + #[allow(rustc::potential_query_instability)] if self .r .extern_crate_map @@ -2166,6 +2168,8 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> { // Items from the prelude if !module.no_implicit_prelude { let extern_prelude = self.r.extern_prelude.clone(); + // It is sorted later so ordering is not important. + #[allow(rustc::potential_query_instability)] names.extend(extern_prelude.iter().flat_map(|(ident, _)| { self.r .crate_loader(|c| c.maybe_process_path_extern(ident.name)) diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index e913b196ca0f9..7d06c0b692047 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -1021,7 +1021,7 @@ pub struct Resolver<'ra, 'tcx> { graph_root: Module<'ra>, prelude: Option>, - extern_prelude: FxIndexMap>, + extern_prelude: FxHashMap>, /// N.B., this is used only for better diagnostics, not name resolution itself. field_names: LocalDefIdMap>, @@ -1054,7 +1054,7 @@ pub struct Resolver<'ra, 'tcx> { extra_lifetime_params_map: NodeMap>, /// `CrateNum` resolutions of `extern crate` items. - extern_crate_map: FxIndexMap, + extern_crate_map: FxHashMap, module_children: LocalDefIdMap>, trait_map: NodeMap>, @@ -1077,7 +1077,7 @@ pub struct Resolver<'ra, 'tcx> { /// some AST passes can generate identifiers that only resolve to local or /// lang items. empty_module: Module<'ra>, - module_map: FxIndexMap>, + module_map: FxHashMap>, binding_parent_modules: FxHashMap, Module<'ra>>, underscore_disambiguator: u32, @@ -1113,14 +1113,14 @@ pub struct Resolver<'ra, 'tcx> { macro_names: FxHashSet, builtin_macros: FxHashMap, registered_tools: &'tcx RegisteredTools, - macro_use_prelude: FxIndexMap>, + macro_use_prelude: FxHashMap>, macro_map: FxHashMap, dummy_ext_bang: Lrc, dummy_ext_derive: Lrc, non_macro_attr: MacroData, local_macro_def_scopes: FxHashMap>, ast_transform_scopes: FxHashMap>, - unused_macros: FxIndexMap, + unused_macros: FxHashMap, /// A map from the macro to all its potentially unused arms. unused_macro_rules: FxIndexMap>, proc_macro_stubs: FxHashSet, @@ -1234,7 +1234,7 @@ impl<'ra> ResolverArenas<'ra> { expn_id: ExpnId, span: Span, no_implicit_prelude: bool, - module_map: &mut FxIndexMap>, + module_map: &mut FxHashMap>, module_self_bindings: &mut FxHashMap, NameBinding<'ra>>, ) -> Module<'ra> { let module = Module(Interned::new_unchecked(self.modules.alloc(ModuleData::new( @@ -1379,7 +1379,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { arenas: &'ra ResolverArenas<'ra>, ) -> Resolver<'ra, 'tcx> { let root_def_id = CRATE_DEF_ID.to_def_id(); - let mut module_map = FxIndexMap::default(); + let mut module_map = FxHashMap::default(); let mut module_self_bindings = FxHashMap::default(); let graph_root = arenas.new_module( None, @@ -1396,7 +1396,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ExpnId::root(), DUMMY_SP, true, - &mut FxIndexMap::default(), + &mut FxHashMap::default(), &mut FxHashMap::default(), ); @@ -1412,7 +1412,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let mut invocation_parents = FxHashMap::default(); invocation_parents.insert(LocalExpnId::ROOT, InvocationParent::ROOT); - let mut extern_prelude: FxIndexMap> = tcx + let mut extern_prelude: FxHashMap> = tcx .sess .opts .externs @@ -1512,7 +1512,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { macro_names: FxHashSet::default(), builtin_macros: Default::default(), registered_tools, - macro_use_prelude: FxIndexMap::default(), + macro_use_prelude: FxHashMap::default(), macro_map: FxHashMap::default(), dummy_ext_bang: Lrc::new(SyntaxExtension::dummy_bang(edition)), dummy_ext_derive: Lrc::new(SyntaxExtension::dummy_derive(edition)), diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index bb99565f0274e..4ff70ce39a999 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -346,7 +346,11 @@ impl<'ra, 'tcx> ResolverExpand for Resolver<'ra, 'tcx> { } fn check_unused_macros(&mut self) { - for (_, &(node_id, ident)) in self.unused_macros.iter() { + // Make ordering consistent before iteration + #[allow(rustc::potential_query_instability)] + let mut unused_macros: Vec<_> = self.unused_macros.iter().collect(); + unused_macros.sort_by_key(|&(_, (key, _))| key); + for (_, &(node_id, ident)) in unused_macros { self.lint_buffer.buffer_lint( UNUSED_MACROS, node_id, @@ -602,7 +606,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { match res { Res::Def(DefKind::Macro(_), def_id) => { if let Some(def_id) = def_id.as_local() { - self.unused_macros.shift_remove(&def_id); + self.unused_macros.remove(&def_id); if self.proc_macro_stubs.contains(&def_id) { self.dcx().emit_err(errors::ProcMacroSameCrate { span: path.span, From 34c78ce9e2919a96323d383ef6326b7b3db4a8c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0smail=20Ar=C4=B1l=C4=B1k?= Date: Sun, 27 Oct 2024 09:56:25 +0300 Subject: [PATCH 03/15] Specify sort location in a comment Co-authored-by: Vadim Petrochenkov --- compiler/rustc_resolve/src/diagnostics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index dafb944a6db85..0846d04479ed4 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -1964,7 +1964,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ident: Symbol, current_module: Module<'ra>, ) -> Option { - // It will be sorted later. + // The candidates are sorted just below. #[allow(rustc::potential_query_instability)] let mut candidates = self .extern_prelude From aeac8a89a655b6e5f459f087e5b51b8dd2a156e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0smail=20Ar=C4=B1l=C4=B1k?= Date: Sun, 27 Oct 2024 09:56:51 +0300 Subject: [PATCH 04/15] Specify sort location in a comment Co-authored-by: Vadim Petrochenkov --- compiler/rustc_resolve/src/diagnostics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 0846d04479ed4..328013bf93db7 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -1086,7 +1086,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { this.add_module_candidates(module, &mut suggestions, filter_fn, None); } Scope::MacroUsePrelude => { - // The suggestions are deterministically sorted later + // The suggestions are deterministically sorted at the bottom of this function. #[allow(rustc::potential_query_instability)] suggestions.extend(this.macro_use_prelude.iter().filter_map( |(name, binding)| { From 64d3961adf1bdf35701c31f673ea933b66203a56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0smail=20Ar=C4=B1l=C4=B1k?= Date: Sun, 27 Oct 2024 09:57:06 +0300 Subject: [PATCH 05/15] Specify sort location in a comment Co-authored-by: Vadim Petrochenkov --- compiler/rustc_resolve/src/diagnostics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 328013bf93db7..37095a543060b 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -1106,7 +1106,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } } Scope::ExternPrelude => { - // The suggestions are deterministically sorted later + // The suggestions are deterministically sorted at the bottom of this function. #[allow(rustc::potential_query_instability)] suggestions.extend(this.extern_prelude.iter().filter_map(|(ident, _)| { let res = Res::Def(DefKind::Mod, CRATE_DEF_ID.to_def_id()); From 1121d656335648753071b89377b1c2fb4472ec4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0smail=20Ar=C4=B1l=C4=B1k?= Date: Sun, 27 Oct 2024 09:58:02 +0300 Subject: [PATCH 06/15] Clarify reasoning behind allowing this lint in a comment Co-authored-by: Vadim Petrochenkov --- compiler/rustc_resolve/src/diagnostics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 37095a543060b..1463f83ad1fe4 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -2852,7 +2852,7 @@ fn show_candidates( } else { // Get the unique item kinds and if there's only one, we use the right kind name // instead of the more generic "items". - // Ordering is not important here. + // Ordering is not important if there's only one element in the set. #[allow(rustc::potential_query_instability)] let mut kinds = accessible_path_strings .iter() From 9652373d348e300eae68b112118117f5b92f7151 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0smail=20Ar=C4=B1l=C4=B1k?= Date: Sun, 27 Oct 2024 09:58:25 +0300 Subject: [PATCH 07/15] Clarify reasoning behind allowing this lint in a comment Co-authored-by: Vadim Petrochenkov --- compiler/rustc_resolve/src/late.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 606b3695903a4..e21f7be9a09ef 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -2258,7 +2258,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> { } })); } - // Ordering is not important here. + // Ordering is not important if there's only one element in the set. #[allow(rustc::potential_query_instability)] let mut distinct_iter = distinct.into_iter(); if let Some(res) = distinct_iter.next() { From e592d7336c6b7c379627d271b12e4c51b2a0522d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0smail=20Ar=C4=B1l=C4=B1k?= Date: Sun, 27 Oct 2024 10:03:15 +0300 Subject: [PATCH 08/15] Clarify reasoning behind allowing this lint in a comment Co-authored-by: Vadim Petrochenkov --- compiler/rustc_resolve/src/late/diagnostics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 4ebc2afd68d23..541afea13291c 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -1928,7 +1928,7 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> { let Some(default_trait) = default_trait else { return; }; - // The ordering is not important here. + // The ordering is not important because `any` is used on the iterator. #[allow(rustc::potential_query_instability)] if self .r From 477de93119080d2b5bf0305abb545d7d965bb386 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0smail=20Ar=C4=B1l=C4=B1k?= Date: Sun, 27 Oct 2024 10:03:57 +0300 Subject: [PATCH 09/15] Specify sort location in a comment Co-authored-by: Vadim Petrochenkov --- compiler/rustc_resolve/src/late/diagnostics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 541afea13291c..3f830661ba523 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -2168,7 +2168,7 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> { // Items from the prelude if !module.no_implicit_prelude { let extern_prelude = self.r.extern_prelude.clone(); - // It is sorted later so ordering is not important. + // The names are sorted at the bottom of this function. #[allow(rustc::potential_query_instability)] names.extend(extern_prelude.iter().flat_map(|(ident, _)| { self.r From 93ab85f3ad250661d6e6d0107a0b7d44c75f9392 Mon Sep 17 00:00:00 2001 From: ismailarilik Date: Mon, 28 Oct 2024 09:30:45 +0300 Subject: [PATCH 10/15] Sort suggestions in diagnostics to make error reporting deterministic --- compiler/rustc_resolve/src/diagnostics.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 1463f83ad1fe4..b6417b654f206 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -1366,7 +1366,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ); if lookup_ident.span.at_least_rust_2018() { - // It will be sorted later. + // Extended suggestions will be sorted at the end of this function. #[allow(rustc::potential_query_instability)] for ident in self.extern_prelude.clone().into_keys() { if ident.span.from_expansion() { @@ -1422,6 +1422,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } } + // Make sure error reporting is deterministic. + suggestions.sort_by_key(|suggestion| suggestion.did.unwrap().index); suggestions } From 5d14c349afd52809b0dc35815cc53992c0b3fef3 Mon Sep 17 00:00:00 2001 From: ismailarilik Date: Mon, 28 Oct 2024 09:45:13 +0300 Subject: [PATCH 11/15] Revert unused_arms to HashMap; they are already sorted so no need to use IndexMap for it --- compiler/rustc_resolve/src/lib.rs | 2 +- compiler/rustc_resolve/src/macros.rs | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 7d06c0b692047..f1d5f3e2ffa18 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -1122,7 +1122,7 @@ pub struct Resolver<'ra, 'tcx> { ast_transform_scopes: FxHashMap>, unused_macros: FxHashMap, /// A map from the macro to all its potentially unused arms. - unused_macro_rules: FxIndexMap>, + unused_macro_rules: FxIndexMap>, proc_macro_stubs: FxHashSet, /// Traces collected during macro resolution and validated when it's complete. single_segment_macro_resolutions: diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index 4ff70ce39a999..6305c0094b481 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -341,7 +341,7 @@ impl<'ra, 'tcx> ResolverExpand for Resolver<'ra, 'tcx> { fn record_macro_rule_usage(&mut self, id: NodeId, rule_i: usize) { let did = self.local_def_id(id); if let Some(rules) = self.unused_macro_rules.get_mut(&did) { - rules.shift_remove(&rule_i); + rules.remove(&rule_i); } } @@ -360,6 +360,8 @@ impl<'ra, 'tcx> ResolverExpand for Resolver<'ra, 'tcx> { } for (&def_id, unused_arms) in self.unused_macro_rules.iter() { + // It is already sorted below. + #[allow(rustc::potential_query_instability)] let mut unused_arms = unused_arms.iter().collect::>(); unused_arms.sort_by_key(|&(&arm_i, _)| arm_i); From f09b6a80351551e04ba5d9dc7da9e3d7245fe725 Mon Sep 17 00:00:00 2001 From: ismailarilik Date: Sun, 3 Nov 2024 13:33:12 +0300 Subject: [PATCH 12/15] Try to sort `single_imports` before usage to have a stable result The `FxIndexSet` solution seems costly. --- compiler/rustc_resolve/src/ident.rs | 7 ++++++- compiler/rustc_resolve/src/imports.rs | 8 ++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_resolve/src/ident.rs b/compiler/rustc_resolve/src/ident.rs index b80e0e196ca08..0c06cc2e6a1db 100644 --- a/compiler/rustc_resolve/src/ident.rs +++ b/compiler/rustc_resolve/src/ident.rs @@ -984,9 +984,14 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { // --- From now on we either have a glob resolution or no resolution. --- + // It is sorted before usage so ordering is not important. + #[allow(rustc::potential_query_instability)] + let mut single_imports: Vec<_> = resolution.single_imports.clone().into_iter().collect(); + single_imports.sort_by_key(|import| import.0.id()); + // Check if one of single imports can still define the name, // if it can then our result is not determined and can be invalidated. - for single_import in &resolution.single_imports { + for single_import in &single_imports { if ignore_import == Some(*single_import) { // This branch handles a cycle in single imports. // diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index f767934e10d4d..51fbcb8ebb805 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -4,7 +4,7 @@ use std::cell::Cell; use std::mem; use rustc_ast::NodeId; -use rustc_data_structures::fx::{FxHashSet, FxIndexSet}; +use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::intern::Interned; use rustc_errors::codes::*; use rustc_errors::{Applicability, MultiSpan, pluralize, struct_span_code_err}; @@ -220,7 +220,7 @@ impl<'ra> ImportData<'ra> { pub(crate) struct NameResolution<'ra> { /// Single imports that may define the name in the namespace. /// Imports are arena-allocated, so it's ok to use pointers as keys. - pub single_imports: FxIndexSet>, + pub single_imports: FxHashSet>, /// The least shadowable known binding for this name, or None if there are no known bindings. pub binding: Option>, pub shadowed_glob: Option>, @@ -482,7 +482,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let key = BindingKey::new(target, ns); let _ = this.try_define(import.parent_scope.module, key, dummy_binding, false); this.update_resolution(import.parent_scope.module, key, false, |_, resolution| { - resolution.single_imports.shift_remove(&import); + resolution.single_imports.remove(&import); }) }); self.record_use(target, dummy_binding, Used::Other); @@ -837,7 +837,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } let key = BindingKey::new(target, ns); this.update_resolution(parent, key, false, |_, resolution| { - resolution.single_imports.shift_remove(&import); + resolution.single_imports.remove(&import); }); } } From 0ecb033d56ac15485ceee12b4aae15307b84d771 Mon Sep 17 00:00:00 2001 From: ismailarilik Date: Sun, 3 Nov 2024 16:17:43 +0300 Subject: [PATCH 13/15] Try to sort bindings before usage to have a stable result The `FxIndexMap` solution seems costly. --- compiler/rustc_resolve/src/late.rs | 16 ++++++---- .../rustc_resolve/src/late/diagnostics.rs | 30 ++++++++++++++++--- 2 files changed, 36 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index e21f7be9a09ef..3e74ab19fa0de 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -259,7 +259,7 @@ impl RibKind<'_> { /// resolving, the name is looked up from inside out. #[derive(Debug)] pub(crate) struct Rib<'ra, R = Res> { - pub bindings: FxIndexMap, + pub bindings: FxHashMap, pub kind: RibKind<'ra>, } @@ -1548,7 +1548,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> { // Allow all following defaults to refer to this type parameter. forward_ty_ban_rib .bindings - .shift_remove(&Ident::with_dummy_span(param.ident.name)); + .remove(&Ident::with_dummy_span(param.ident.name)); } GenericParamKind::Const { ref ty, kw_span: _, ref default } => { // Const parameters can't have param bounds. @@ -1576,7 +1576,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> { // Allow all following defaults to refer to this const parameter. forward_const_ban_rib .bindings - .shift_remove(&Ident::with_dummy_span(param.ident.name)); + .remove(&Ident::with_dummy_span(param.ident.name)); } } } @@ -2795,8 +2795,12 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> { break; } - seen_bindings - .extend(parent_rib.bindings.keys().map(|ident| (*ident, ident.span))); + // It is sorted before usage so ordering is not important here. + #[allow(rustc::potential_query_instability)] + let mut idents: Vec<_> = parent_rib.bindings.keys().into_iter().collect(); + idents.sort_by_key(|&ident| ident.span); + + seen_bindings.extend(idents.into_iter().map(|ident| (*ident, ident.span))); } } @@ -3868,7 +3872,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> { } } - fn innermost_rib_bindings(&mut self, ns: Namespace) -> &mut FxIndexMap { + fn innermost_rib_bindings(&mut self, ns: Namespace) -> &mut FxHashMap { &mut self.ribs[ns].last_mut().unwrap().bindings } diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 3f830661ba523..24bd116711922 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -762,7 +762,12 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> { if let Some(rib) = &self.last_block_rib && let RibKind::Normal = rib.kind { - for (ident, &res) in &rib.bindings { + // It is sorted before usage so ordering is not important here. + #[allow(rustc::potential_query_instability)] + let mut bindings: Vec<_> = rib.bindings.clone().into_iter().collect(); + bindings.sort_by_key(|(ident, _)| ident.span); + + for (ident, res) in &bindings { if let Res::Local(_) = res && path.len() == 1 && ident.span.eq_ctxt(path[0].ident.span) @@ -944,7 +949,12 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> { if let Some(err_code) = err.code { if err_code == E0425 { for label_rib in &self.label_ribs { - for (label_ident, node_id) in &label_rib.bindings { + // It is sorted before usage so ordering is not important here. + #[allow(rustc::potential_query_instability)] + let mut bindings: Vec<_> = label_rib.bindings.clone().into_iter().collect(); + bindings.sort_by_key(|(ident, _)| ident.span); + + for (label_ident, node_id) in &bindings { let ident = path.last().unwrap().ident; if format!("'{ident}") == label_ident.to_string() { err.span_label(label_ident.span, "a label with a similar name exists"); @@ -2142,6 +2152,8 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> { }; // Locals and type parameters + // `names` is sorted below so ordering is not important here. + #[allow(rustc::potential_query_instability)] for (ident, &res) in &rib.bindings { if filter_fn(res) && ident.span.ctxt() == rib_ctxt { names.push(TypoSuggestion::typo_from_ident(*ident, res)); @@ -2605,18 +2617,28 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> { let within_scope = self.is_label_valid_from_rib(rib_index); let rib = &self.label_ribs[rib_index]; - let names = rib + // `names` is sorted below so ordering is not important here. + #[allow(rustc::potential_query_instability)] + let mut names = rib .bindings .iter() .filter(|(id, _)| id.span.eq_ctxt(label.span)) .map(|(id, _)| id.name) .collect::>(); + // Make sure error reporting is deterministic. + names.sort(); + find_best_match_for_name(&names, label.name, None).map(|symbol| { + // It is sorted before usage so ordering is not important here. + #[allow(rustc::potential_query_instability)] + let mut bindings: Vec<_> = rib.bindings.clone().into_iter().collect(); + bindings.sort_by_key(|(ident, _)| ident.span); + // Upon finding a similar name, get the ident that it was from - the span // contained within helps make a useful diagnostic. In addition, determine // whether this candidate is within scope. - let (ident, _) = rib.bindings.iter().find(|(ident, _)| ident.name == symbol).unwrap(); + let (ident, _) = bindings.iter().find(|(ident, _)| ident.name == symbol).unwrap(); (*ident, within_scope) }) } From 2ff01e8b3c7f79adb31a25aaca174596b9a59a9d Mon Sep 17 00:00:00 2001 From: ismailarilik Date: Mon, 4 Nov 2024 09:15:40 +0300 Subject: [PATCH 14/15] Use `swap_remove` instead of `shift_remove` `shift_remove` is very expensive --- compiler/rustc_resolve/src/ident.rs | 7 +---- compiler/rustc_resolve/src/imports.rs | 8 ++--- compiler/rustc_resolve/src/late.rs | 16 ++++------ .../rustc_resolve/src/late/diagnostics.rs | 30 +++---------------- 4 files changed, 15 insertions(+), 46 deletions(-) diff --git a/compiler/rustc_resolve/src/ident.rs b/compiler/rustc_resolve/src/ident.rs index 0c06cc2e6a1db..b80e0e196ca08 100644 --- a/compiler/rustc_resolve/src/ident.rs +++ b/compiler/rustc_resolve/src/ident.rs @@ -984,14 +984,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { // --- From now on we either have a glob resolution or no resolution. --- - // It is sorted before usage so ordering is not important. - #[allow(rustc::potential_query_instability)] - let mut single_imports: Vec<_> = resolution.single_imports.clone().into_iter().collect(); - single_imports.sort_by_key(|import| import.0.id()); - // Check if one of single imports can still define the name, // if it can then our result is not determined and can be invalidated. - for single_import in &single_imports { + for single_import in &resolution.single_imports { if ignore_import == Some(*single_import) { // This branch handles a cycle in single imports. // diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index 51fbcb8ebb805..7db556ff48580 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -4,7 +4,7 @@ use std::cell::Cell; use std::mem; use rustc_ast::NodeId; -use rustc_data_structures::fx::FxHashSet; +use rustc_data_structures::fx::{FxHashSet, FxIndexSet}; use rustc_data_structures::intern::Interned; use rustc_errors::codes::*; use rustc_errors::{Applicability, MultiSpan, pluralize, struct_span_code_err}; @@ -220,7 +220,7 @@ impl<'ra> ImportData<'ra> { pub(crate) struct NameResolution<'ra> { /// Single imports that may define the name in the namespace. /// Imports are arena-allocated, so it's ok to use pointers as keys. - pub single_imports: FxHashSet>, + pub single_imports: FxIndexSet>, /// The least shadowable known binding for this name, or None if there are no known bindings. pub binding: Option>, pub shadowed_glob: Option>, @@ -482,7 +482,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let key = BindingKey::new(target, ns); let _ = this.try_define(import.parent_scope.module, key, dummy_binding, false); this.update_resolution(import.parent_scope.module, key, false, |_, resolution| { - resolution.single_imports.remove(&import); + resolution.single_imports.swap_remove(&import); }) }); self.record_use(target, dummy_binding, Used::Other); @@ -837,7 +837,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } let key = BindingKey::new(target, ns); this.update_resolution(parent, key, false, |_, resolution| { - resolution.single_imports.remove(&import); + resolution.single_imports.swap_remove(&import); }); } } diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 3e74ab19fa0de..50cab538f143b 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -259,7 +259,7 @@ impl RibKind<'_> { /// resolving, the name is looked up from inside out. #[derive(Debug)] pub(crate) struct Rib<'ra, R = Res> { - pub bindings: FxHashMap, + pub bindings: FxIndexMap, pub kind: RibKind<'ra>, } @@ -1548,7 +1548,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> { // Allow all following defaults to refer to this type parameter. forward_ty_ban_rib .bindings - .remove(&Ident::with_dummy_span(param.ident.name)); + .swap_remove(&Ident::with_dummy_span(param.ident.name)); } GenericParamKind::Const { ref ty, kw_span: _, ref default } => { // Const parameters can't have param bounds. @@ -1576,7 +1576,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> { // Allow all following defaults to refer to this const parameter. forward_const_ban_rib .bindings - .remove(&Ident::with_dummy_span(param.ident.name)); + .swap_remove(&Ident::with_dummy_span(param.ident.name)); } } } @@ -2795,12 +2795,8 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> { break; } - // It is sorted before usage so ordering is not important here. - #[allow(rustc::potential_query_instability)] - let mut idents: Vec<_> = parent_rib.bindings.keys().into_iter().collect(); - idents.sort_by_key(|&ident| ident.span); - - seen_bindings.extend(idents.into_iter().map(|ident| (*ident, ident.span))); + seen_bindings + .extend(parent_rib.bindings.keys().map(|ident| (*ident, ident.span))); } } @@ -3872,7 +3868,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> { } } - fn innermost_rib_bindings(&mut self, ns: Namespace) -> &mut FxHashMap { + fn innermost_rib_bindings(&mut self, ns: Namespace) -> &mut FxIndexMap { &mut self.ribs[ns].last_mut().unwrap().bindings } diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 24bd116711922..3f830661ba523 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -762,12 +762,7 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> { if let Some(rib) = &self.last_block_rib && let RibKind::Normal = rib.kind { - // It is sorted before usage so ordering is not important here. - #[allow(rustc::potential_query_instability)] - let mut bindings: Vec<_> = rib.bindings.clone().into_iter().collect(); - bindings.sort_by_key(|(ident, _)| ident.span); - - for (ident, res) in &bindings { + for (ident, &res) in &rib.bindings { if let Res::Local(_) = res && path.len() == 1 && ident.span.eq_ctxt(path[0].ident.span) @@ -949,12 +944,7 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> { if let Some(err_code) = err.code { if err_code == E0425 { for label_rib in &self.label_ribs { - // It is sorted before usage so ordering is not important here. - #[allow(rustc::potential_query_instability)] - let mut bindings: Vec<_> = label_rib.bindings.clone().into_iter().collect(); - bindings.sort_by_key(|(ident, _)| ident.span); - - for (label_ident, node_id) in &bindings { + for (label_ident, node_id) in &label_rib.bindings { let ident = path.last().unwrap().ident; if format!("'{ident}") == label_ident.to_string() { err.span_label(label_ident.span, "a label with a similar name exists"); @@ -2152,8 +2142,6 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> { }; // Locals and type parameters - // `names` is sorted below so ordering is not important here. - #[allow(rustc::potential_query_instability)] for (ident, &res) in &rib.bindings { if filter_fn(res) && ident.span.ctxt() == rib_ctxt { names.push(TypoSuggestion::typo_from_ident(*ident, res)); @@ -2617,28 +2605,18 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> { let within_scope = self.is_label_valid_from_rib(rib_index); let rib = &self.label_ribs[rib_index]; - // `names` is sorted below so ordering is not important here. - #[allow(rustc::potential_query_instability)] - let mut names = rib + let names = rib .bindings .iter() .filter(|(id, _)| id.span.eq_ctxt(label.span)) .map(|(id, _)| id.name) .collect::>(); - // Make sure error reporting is deterministic. - names.sort(); - find_best_match_for_name(&names, label.name, None).map(|symbol| { - // It is sorted before usage so ordering is not important here. - #[allow(rustc::potential_query_instability)] - let mut bindings: Vec<_> = rib.bindings.clone().into_iter().collect(); - bindings.sort_by_key(|(ident, _)| ident.span); - // Upon finding a similar name, get the ident that it was from - the span // contained within helps make a useful diagnostic. In addition, determine // whether this candidate is within scope. - let (ident, _) = bindings.iter().find(|(ident, _)| ident.name == symbol).unwrap(); + let (ident, _) = rib.bindings.iter().find(|(ident, _)| ident.name == symbol).unwrap(); (*ident, within_scope) }) } From 0944d8c1ea3bdcc40b14605a4c884acfb0f50853 Mon Sep 17 00:00:00 2001 From: ismailarilik Date: Mon, 4 Nov 2024 09:25:12 +0300 Subject: [PATCH 15/15] Sort `idents` before usage instead of `suggestions` to have a stable order As requested by @petrochenkov --- compiler/rustc_resolve/src/diagnostics.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index b6417b654f206..b5ae6ed4cda2f 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -1366,9 +1366,12 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ); if lookup_ident.span.at_least_rust_2018() { - // Extended suggestions will be sorted at the end of this function. + // `idents` is sorted before usage so ordering is not important here. #[allow(rustc::potential_query_instability)] - for ident in self.extern_prelude.clone().into_keys() { + let mut idents: Vec<_> = self.extern_prelude.clone().into_keys().collect(); + idents.sort_by_key(|ident| ident.span); + + for ident in idents { if ident.span.from_expansion() { // Idents are adjusted to the root context before being // resolved in the extern prelude, so reporting this to the @@ -1422,8 +1425,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } } - // Make sure error reporting is deterministic. - suggestions.sort_by_key(|suggestion| suggestion.did.unwrap().index); suggestions }