From 4abdaeb67eb3aa85941cee4928b75dbcbd120148 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 4 Jul 2023 18:51:35 +0300 Subject: [PATCH] resolve: Use `Interned` for `Import` --- .../rustc_resolve/src/build_reduced_graph.rs | 12 ++--- compiler/rustc_resolve/src/diagnostics.rs | 24 ++++----- compiler/rustc_resolve/src/ident.rs | 11 ++--- compiler/rustc_resolve/src/imports.rs | 49 ++++++++----------- compiler/rustc_resolve/src/lib.rs | 33 ++++++------- 5 files changed, 58 insertions(+), 71 deletions(-) diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs index 9af06f1a48a3..fdf1e281b0bd 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -6,7 +6,7 @@ //! Imports are also considered items and placed into modules here, but not resolved yet. use crate::def_collector::collect_definitions; -use crate::imports::{Import, ImportKind}; +use crate::imports::{ImportData, ImportKind}; use crate::macros::{MacroRulesBinding, MacroRulesScope, MacroRulesScopeRef}; use crate::Namespace::{self, MacroNS, TypeNS, ValueNS}; use crate::{errors, BindingKey, MacroData, NameBindingData}; @@ -354,7 +354,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> { vis: ty::Visibility, ) { let current_module = self.parent_scope.module; - let import = self.r.arenas.alloc_import(Import { + let import = self.r.arenas.alloc_import(ImportData { kind, parent_scope: self.parent_scope, module_path, @@ -378,7 +378,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> { if !type_ns_only || ns == TypeNS { let key = BindingKey::new(target, ns); let mut resolution = this.resolution(current_module, key).borrow_mut(); - resolution.add_single_import(import); + resolution.single_imports.insert(import); } }); } @@ -848,7 +848,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> { (used, Some(ModuleOrUniformRoot::Module(module)), binding) }) .unwrap_or((true, None, self.r.dummy_binding)); - let import = self.r.arenas.alloc_import(Import { + let import = self.r.arenas.alloc_import(ImportData { kind: ImportKind::ExternCrate { source: orig_name, target: ident, id: item.id }, root_id: item.id, parent_scope: self.parent_scope, @@ -1058,7 +1058,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> { } let macro_use_import = |this: &Self, span| { - this.r.arenas.alloc_import(Import { + this.r.arenas.alloc_import(ImportData { kind: ImportKind::MacroUse, root_id: item.id, parent_scope: this.parent_scope, @@ -1228,7 +1228,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> { self.r.set_binding_parent_module(binding, parent_scope.module); self.r.all_macro_rules.insert(ident.name, res); if is_macro_export { - let import = self.r.arenas.alloc_import(Import { + let import = self.r.arenas.alloc_import(ImportData { kind: ImportKind::MacroExport, root_id: item.id, parent_scope: self.parent_scope, diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 4db4108c6f42..9c1d42514657 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -262,7 +262,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { // See https://github.com/rust-lang/rust/issues/32354 use NameBindingKind::Import; - let can_suggest = |binding: NameBinding<'_>, import: &self::Import<'_>| { + let can_suggest = |binding: NameBinding<'_>, import: self::Import<'_>| { !binding.span.is_dummy() && !matches!(import.kind, ImportKind::MacroUse | ImportKind::MacroExport) }; @@ -272,22 +272,22 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { (Import { import: new, .. }, Import { import: old, .. }) if { (new.has_attributes || old.has_attributes) - && can_suggest(old_binding, old) - && can_suggest(new_binding, new) + && can_suggest(old_binding, *old) + && can_suggest(new_binding, *new) } => { if old.has_attributes { - Some((new, new_binding.span, true)) + Some((*new, new_binding.span, true)) } else { - Some((old, old_binding.span, true)) + Some((*old, old_binding.span, true)) } } // Otherwise prioritize the new binding. - (Import { import, .. }, other) if can_suggest(new_binding, import) => { - Some((import, new_binding.span, other.is_import())) + (Import { import, .. }, other) if can_suggest(new_binding, *import) => { + Some((*import, new_binding.span, other.is_import())) } - (other, Import { import, .. }) if can_suggest(old_binding, import) => { - Some((import, old_binding.span, other.is_import())) + (other, Import { import, .. }) if can_suggest(old_binding, *import) => { + Some((*import, old_binding.span, other.is_import())) } _ => None, }; @@ -341,7 +341,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { &self, err: &mut Diagnostic, name: Symbol, - import: &Import<'_>, + import: Import<'_>, binding_span: Span, ) { let suggested_name = if name.as_str().chars().next().unwrap().is_uppercase() { @@ -413,7 +413,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { fn add_suggestion_for_duplicate_nested_use( &self, err: &mut Diagnostic, - import: &Import<'_>, + import: Import<'_>, binding_span: Span, ) { assert!(import.is_nested()); @@ -2114,7 +2114,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { /// ``` pub(crate) fn check_for_module_export_macro( &mut self, - import: &'a Import<'a>, + import: Import<'a>, module: ModuleOrUniformRoot<'a>, ident: Ident, ) -> Option<(Option, Option)> { diff --git a/compiler/rustc_resolve/src/ident.rs b/compiler/rustc_resolve/src/ident.rs index ba6f50ef1bae..40eee48e475c 100644 --- a/compiler/rustc_resolve/src/ident.rs +++ b/compiler/rustc_resolve/src/ident.rs @@ -20,7 +20,7 @@ use crate::late::{ use crate::macros::{sub_namespace_match, MacroRulesScope}; use crate::BindingKey; use crate::{errors, AmbiguityError, AmbiguityErrorMisc, AmbiguityKind, Determinacy, Finalize}; -use crate::{Import, ImportKind, LexicalScopeBinding, Module, ModuleKind, ModuleOrUniformRoot}; +use crate::{ImportKind, LexicalScopeBinding, Module, ModuleKind, ModuleOrUniformRoot}; use crate::{NameBinding, NameBindingKind, ParentScope, PathResult, PrivacyError, Res}; use crate::{ResolutionError, Resolver, Scope, ScopeSet, Segment, ToNameBinding, Weak}; @@ -913,11 +913,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { } if !restricted_shadowing && binding.expansion != LocalExpnId::ROOT { - if let NameBindingKind::Import { - import: Import { kind: ImportKind::MacroExport, .. }, - .. - } = binding.kind - { + if let NameBindingKind::Import { import, .. } = binding.kind + && matches!(import.kind, ImportKind::MacroExport) { self.macro_expanded_macro_export_errors.insert((path_span, binding.span)); } } @@ -951,7 +948,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { } if let Some(ignored) = ignore_binding && let NameBindingKind::Import { import, .. } = ignored.kind && - ptr::eq(import, &**single_import) { + import == *single_import { // Ignore not just the binding itself, but if it has a shadowed_glob, // ignore that, too, because this loop is supposed to only process // named imports. diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index 5f5b49d8564d..fa8c15abc4b2 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -135,7 +135,7 @@ impl<'a> std::fmt::Debug for ImportKind<'a> { /// One import. #[derive(Debug, Clone)] -pub(crate) struct Import<'a> { +pub(crate) struct ImportData<'a> { pub kind: ImportKind<'a>, /// Node ID of the "root" use item -- this is always the same as `ImportKind`'s `id` @@ -172,7 +172,9 @@ pub(crate) struct Import<'a> { pub used: Cell, } -impl<'a> Import<'a> { +pub(crate) type Import<'a> = Interned<'a, ImportData<'a>>; + +impl<'a> ImportData<'a> { pub(crate) fn is_glob(&self) -> bool { matches!(self.kind, ImportKind::Glob { .. }) } @@ -214,7 +216,7 @@ impl<'a> Import<'a> { pub(crate) struct NameResolution<'a> { /// 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: FxHashSet>, /// The least shadowable known binding for this name, or None if there are no known bindings. pub binding: Option>, pub shadowed_glob: Option>, @@ -231,10 +233,6 @@ impl<'a> NameResolution<'a> { } }) } - - pub(crate) fn add_single_import(&mut self, import: &'a Import<'a>) { - self.single_imports.insert(Interned::new_unchecked(import)); - } } /// An error that may be transformed into a diagnostic later. Used to combine multiple unresolved @@ -250,15 +248,12 @@ struct UnresolvedImportError { // Reexports of the form `pub use foo as bar;` where `foo` is `extern crate foo;` // are permitted for backward-compatibility under a deprecation lint. -fn pub_use_of_private_extern_crate_hack(import: &Import<'_>, binding: NameBinding<'_>) -> bool { +fn pub_use_of_private_extern_crate_hack(import: Import<'_>, binding: NameBinding<'_>) -> bool { match (&import.kind, &binding.kind) { - ( - ImportKind::Single { .. }, - NameBindingKind::Import { - import: Import { kind: ImportKind::ExternCrate { .. }, .. }, - .. - }, - ) => import.expect_vis().is_public(), + (ImportKind::Single { .. }, NameBindingKind::Import { import: binding_import, .. }) => { + matches!(binding_import.kind, ImportKind::ExternCrate { .. }) + && import.expect_vis().is_public() + } _ => false, } } @@ -266,11 +261,7 @@ fn pub_use_of_private_extern_crate_hack(import: &Import<'_>, binding: NameBindin impl<'a, 'tcx> Resolver<'a, 'tcx> { /// Given a binding and an import that resolves to it, /// return the corresponding binding defined by the import. - pub(crate) fn import( - &self, - binding: NameBinding<'a>, - import: &'a Import<'a>, - ) -> NameBinding<'a> { + pub(crate) fn import(&self, binding: NameBinding<'a>, import: Import<'a>) -> NameBinding<'a> { let import_vis = import.expect_vis().to_def_id(); let vis = if binding.vis.is_at_least(import_vis, self.tcx) || pub_use_of_private_extern_crate_hack(import, binding) @@ -411,7 +402,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { None => continue, }; if self.is_accessible_from(binding.vis, scope) { - let imported_binding = self.import(binding, import); + let imported_binding = self.import(binding, *import); let key = BindingKey { ident, ..key }; let _ = self.try_define(import.parent_scope.module, key, imported_binding); } @@ -422,7 +413,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { // Define a dummy resolution containing a `Res::Err` as a placeholder for a failed // or indeterminate resolution, also mark such failed imports as used to avoid duplicate diagnostics. - fn import_dummy_binding(&mut self, import: &'a Import<'a>, is_indeterminate: bool) { + fn import_dummy_binding(&mut self, import: Import<'a>, is_indeterminate: bool) { if let ImportKind::Single { target, ref target_bindings, .. } = import.kind { if !(is_indeterminate || target_bindings.iter().all(|binding| binding.get().is_none())) { @@ -460,7 +451,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { prev_indeterminate_count = indeterminate_count; indeterminate_count = 0; for import in mem::take(&mut self.indeterminate_imports) { - let import_indeterminate_count = self.resolve_import(&import); + let import_indeterminate_count = self.resolve_import(import); indeterminate_count += import_indeterminate_count; match import_indeterminate_count { 0 => self.determined_imports.push(import), @@ -609,7 +600,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { } } - fn throw_unresolved_import_error(&mut self, errors: Vec<(&Import<'_>, UnresolvedImportError)>) { + fn throw_unresolved_import_error(&mut self, errors: Vec<(Import<'_>, UnresolvedImportError)>) { if errors.is_empty() { return; } @@ -701,7 +692,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { /// /// Meanwhile, if resolve successful, the resolved bindings are written /// into the module. - fn resolve_import(&mut self, import: &'a Import<'a>) -> usize { + fn resolve_import(&mut self, import: Import<'a>) -> usize { debug!( "(resolving import for module) resolving import `{}::...` in `{}`", Segment::names_to_string(&import.module_path), @@ -781,7 +772,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { } let key = BindingKey::new(target, ns); this.update_resolution(parent, key, |_, resolution| { - resolution.single_imports.remove(&Interned::new_unchecked(import)); + resolution.single_imports.remove(&import); }); } } @@ -795,7 +786,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { /// /// Optionally returns an unresolved import error. This error is buffered and used to /// consolidate multiple unresolved import errors into a single diagnostic. - fn finalize_import(&mut self, import: &'a Import<'a>) -> Option { + fn finalize_import(&mut self, import: Import<'a>) -> Option { let orig_vis = import.vis.take(); let ignore_binding = match &import.kind { ImportKind::Single { target_bindings, .. } => target_bindings[TypeNS].get(), @@ -1239,7 +1230,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { fn check_for_redundant_imports( &mut self, ident: Ident, - import: &'a Import<'a>, + import: Import<'a>, source_bindings: &PerNS, Determinacy>>>, target_bindings: &PerNS>>>, target: Ident, @@ -1302,7 +1293,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { } } - fn resolve_glob_import(&mut self, import: &'a Import<'a>) { + fn resolve_glob_import(&mut self, import: Import<'a>) { // This function is only called for glob imports. let ImportKind::Glob { id, is_prelude, .. } = import.kind else { unreachable!() }; diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 8fefbe8177e5..8ee0c715a0ea 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -64,7 +64,7 @@ use std::collections::BTreeSet; use std::{fmt, ptr}; use diagnostics::{ImportSuggestion, LabelSuggestion, Suggestion}; -use imports::{Import, ImportKind, NameResolution}; +use imports::{Import, ImportData, ImportKind, NameResolution}; use late::{HasGenericParams, PathSource, PatternSource}; use macros::{MacroRulesBinding, MacroRulesScope, MacroRulesScopeRef}; @@ -518,8 +518,8 @@ struct ModuleData<'a> { /// Whether `#[no_implicit_prelude]` is active. no_implicit_prelude: bool, - glob_importers: RefCell>>, - globs: RefCell>>, + glob_importers: RefCell>>, + globs: RefCell>>, /// Used to memoize the traits in this module for faster searches through all traits in scope. traits: RefCell)]>>>, @@ -681,7 +681,7 @@ impl<'a> ToNameBinding<'a> for NameBinding<'a> { enum NameBindingKind<'a> { Res(Res), Module(Module<'a>), - Import { binding: NameBinding<'a>, import: &'a Import<'a>, used: Cell }, + Import { binding: NameBinding<'a>, import: Import<'a>, used: Cell }, } impl<'a> NameBindingKind<'a> { @@ -807,10 +807,9 @@ impl<'a> NameBindingData<'a> { fn is_extern_crate(&self) -> bool { match self.kind { - NameBindingKind::Import { - import: &Import { kind: ImportKind::ExternCrate { .. }, .. }, - .. - } => true, + NameBindingKind::Import { import, .. } => { + matches!(import.kind, ImportKind::ExternCrate { .. }) + } NameBindingKind::Module(&ModuleData { kind: ModuleKind::Def(DefKind::Mod, def_id, _), .. @@ -919,10 +918,10 @@ pub struct Resolver<'a, 'tcx> { field_visibility_spans: FxHashMap>, /// All imports known to succeed or fail. - determined_imports: Vec<&'a Import<'a>>, + determined_imports: Vec>, /// All non-determined imports. - indeterminate_imports: Vec<&'a Import<'a>>, + indeterminate_imports: Vec>, // Spans for local variables found during pattern resolution. // Used for suggestions during error reporting. @@ -1032,7 +1031,7 @@ pub struct Resolver<'a, 'tcx> { /// Avoid duplicated errors for "name already defined". name_already_seen: FxHashMap, - potentially_unused_imports: Vec<&'a Import<'a>>, + potentially_unused_imports: Vec>, /// Table for mapping struct IDs into struct constructor IDs, /// it's not used during normal resolution, only for better error reporting. @@ -1087,7 +1086,7 @@ pub struct Resolver<'a, 'tcx> { pub struct ResolverArenas<'a> { modules: TypedArena>, local_modules: RefCell>>, - imports: TypedArena>, + imports: TypedArena>, name_resolutions: TypedArena>>, ast_paths: TypedArena, dropless: DroplessArena, @@ -1120,8 +1119,8 @@ impl<'a> ResolverArenas<'a> { fn alloc_name_binding(&'a self, name_binding: NameBindingData<'a>) -> NameBinding<'a> { Interned::new_unchecked(self.dropless.alloc(name_binding)) } - fn alloc_import(&'a self, import: Import<'a>) -> &'a Import<'_> { - self.imports.alloc(import) + fn alloc_import(&'a self, import: ImportData<'a>) -> Import<'a> { + Interned::new_unchecked(self.imports.alloc(import)) } fn alloc_name_resolution(&'a self) -> &'a RefCell> { self.name_resolutions.alloc(Default::default()) @@ -1626,7 +1625,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { self.maybe_unused_trait_imports.insert(def_id); import_ids.push(def_id); } - self.add_to_glob_map(&import, trait_name); + self.add_to_glob_map(*import, trait_name); kind = &binding.kind; } import_ids @@ -1711,13 +1710,13 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { if let Some(id) = import.id() { self.used_imports.insert(id); } - self.add_to_glob_map(&import, ident); + self.add_to_glob_map(import, ident); self.record_use(ident, binding, false); } } #[inline] - fn add_to_glob_map(&mut self, import: &Import<'_>, ident: Ident) { + fn add_to_glob_map(&mut self, import: Import<'_>, ident: Ident) { if let ImportKind::Glob { id, .. } = import.kind { let def_id = self.local_def_id(id); self.glob_map.entry(def_id).or_default().insert(ident.name);