From de9e964e4ac21897bd48adbe37f379d74422919f Mon Sep 17 00:00:00 2001 From: Paul Daniel Faria Date: Thu, 25 Jun 2020 12:42:12 -0400 Subject: Track import type outside of , use enum rather than bool to improve readability --- crates/ra_hir_def/src/body/lower.rs | 6 +++- crates/ra_hir_def/src/item_scope.rs | 51 +++++++++++++++++++++++------- crates/ra_hir_def/src/nameres/collector.rs | 36 +++++++++++++-------- crates/ra_hir_def/src/per_ns.rs | 20 ++++-------- 4 files changed, 73 insertions(+), 40 deletions(-) (limited to 'crates/ra_hir_def') diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs index 3ced648e5..d749c828d 100644 --- a/crates/ra_hir_def/src/body/lower.rs +++ b/crates/ra_hir_def/src/body/lower.rs @@ -26,7 +26,7 @@ use crate::{ dummy_expr_id, ArithOp, Array, BinaryOp, BindingAnnotation, CmpOp, Expr, ExprId, Literal, LogicOp, MatchArm, Ordering, Pat, PatId, RecordFieldPat, RecordLitField, Statement, }, - item_scope::BuiltinShadowMode, + item_scope::{BuiltinShadowMode, ImportType}, item_tree::{FileItemTreeId, ItemTree, ItemTreeNode}, path::{GenericArgs, Path}, type_ref::{Mutability, Rawness, TypeRef}, @@ -81,6 +81,7 @@ pub(super) fn lower( map }, expander, + import_types: FxHashMap::default(), } .collect(params, body) } @@ -93,6 +94,7 @@ struct ExprCollector<'a> { source_map: BodySourceMap, item_trees: FxHashMap>, + import_types: FxHashMap, } impl ExprCollector<'_> { @@ -711,8 +713,10 @@ impl ExprCollector<'_> { _ => true, }; self.body.item_scope.push_res( + &mut self.import_types, name.as_name(), crate::per_ns::PerNs::from_def(def, vis, has_constructor), + ImportType::Named, ); } } diff --git a/crates/ra_hir_def/src/item_scope.rs b/crates/ra_hir_def/src/item_scope.rs index 0184b6af9..511c08a8d 100644 --- a/crates/ra_hir_def/src/item_scope.rs +++ b/crates/ra_hir_def/src/item_scope.rs @@ -12,6 +12,28 @@ use crate::{ Lookup, MacroDefId, ModuleDefId, TraitId, }; +#[derive(Copy, Clone)] +pub(crate) enum ImportType { + Glob, + Named, +} + +impl ImportType { + fn is_glob(&self) -> bool { + match self { + ImportType::Glob => true, + ImportType::Named => false, + } + } + + fn is_named(&self) -> bool { + match self { + ImportType::Glob => false, + ImportType::Named => true, + } + } +} + #[derive(Debug, Default, PartialEq, Eq)] pub struct ItemScope { visible: FxHashMap, @@ -123,23 +145,30 @@ impl ItemScope { self.legacy_macros.insert(name, mac); } - pub(crate) fn push_res(&mut self, name: Name, def: PerNs) -> bool { + pub(crate) fn push_res( + &mut self, + existing_import_map: &mut FxHashMap, + name: Name, + def: PerNs, + def_import_type: ImportType, + ) -> bool { let mut changed = false; - let existing = self.visible.entry(name).or_default(); + let existing = self.visible.entry(name.clone()).or_default(); + let existing_import_type = existing_import_map.entry(name).or_insert(def_import_type); macro_rules! check_changed { - ($changed:ident, ($existing:ident/$def:ident).$field:ident) => { + ($changed:ident, ($existing:ident/$def:ident).$field:ident, $existing_import_type:ident, $def_import_type:ident) => { match ($existing.$field, $def.$field) { (None, Some(_)) => { - $existing.from_glob = $def.from_glob; + *existing_import_type = $def_import_type; $existing.$field = $def.$field; $changed = true; } - // Only update if the new def came from a specific import and the existing - // import came from a glob import. - (Some(_), Some(_)) if $existing.from_glob && !$def.from_glob => { + (Some(_), Some(_)) + if $existing_import_type.is_glob() && $def_import_type.is_named() => + { mark::hit!(import_shadowed); - $existing.from_glob = $def.from_glob; + *$existing_import_type = $def_import_type; $existing.$field = $def.$field; $changed = true; } @@ -148,9 +177,9 @@ impl ItemScope { }; } - check_changed!(changed, (existing / def).types); - check_changed!(changed, (existing / def).values); - check_changed!(changed, (existing / def).macros); + check_changed!(changed, (existing / def).types, existing_import_type, def_import_type); + check_changed!(changed, (existing / def).values, existing_import_type, def_import_type); + check_changed!(changed, (existing / def).macros, existing_import_type, def_import_type); changed } diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs index 93f58e2c7..f7b99e0be 100644 --- a/crates/ra_hir_def/src/nameres/collector.rs +++ b/crates/ra_hir_def/src/nameres/collector.rs @@ -20,6 +20,7 @@ use test_utils::mark; use crate::{ attr::Attrs, db::DefDatabase, + item_scope::ImportType, item_tree::{ self, FileItemTreeId, ItemTree, ItemTreeId, MacroCall, Mod, ModItem, ModKind, StructDefKind, }, @@ -80,6 +81,7 @@ pub(super) fn collect_defs(db: &dyn DefDatabase, mut def_map: CrateDefMap) -> Cr mod_dirs: FxHashMap::default(), cfg_options, proc_macros, + import_types: FxHashMap::default(), }; collector.collect(); collector.finish() @@ -186,6 +188,7 @@ struct DefCollector<'a> { mod_dirs: FxHashMap, cfg_options: &'a CfgOptions, proc_macros: Vec<(Name, ProcMacroExpander)>, + import_types: FxHashMap, } impl DefCollector<'_> { @@ -305,7 +308,7 @@ impl DefCollector<'_> { self.def_map.root, &[(name, PerNs::macros(macro_, Visibility::Public))], Visibility::Public, - false, + ImportType::Named, ); } } @@ -331,7 +334,7 @@ impl DefCollector<'_> { self.def_map.root, &[(name, PerNs::macros(macro_, Visibility::Public))], Visibility::Public, - false, + ImportType::Named, ); } @@ -478,7 +481,7 @@ impl DefCollector<'_> { .filter(|(_, res)| !res.is_none()) .collect::>(); - self.update(module_id, &items, vis, true); + self.update(module_id, &items, vis, ImportType::Glob); } else { // glob import from same crate => we do an initial // import, and then need to propagate any further @@ -500,7 +503,7 @@ impl DefCollector<'_> { .filter(|(_, res)| !res.is_none()) .collect::>(); - self.update(module_id, &items, vis, true); + self.update(module_id, &items, vis, ImportType::Glob); // record the glob import in case we add further items let glob = self.glob_imports.entry(m.local_id).or_default(); if !glob.iter().any(|(mid, _)| *mid == module_id) { @@ -530,7 +533,7 @@ impl DefCollector<'_> { (name, res) }) .collect::>(); - self.update(module_id, &resolutions, vis, true); + self.update(module_id, &resolutions, vis, ImportType::Glob); } Some(d) => { log::debug!("glob import {:?} from non-module/enum {:?}", import, d); @@ -556,7 +559,7 @@ impl DefCollector<'_> { } } - self.update(module_id, &[(name, def)], vis, false); + self.update(module_id, &[(name, def)], vis, ImportType::Named); } None => mark::hit!(bogus_paths), } @@ -568,9 +571,9 @@ impl DefCollector<'_> { module_id: LocalModuleId, resolutions: &[(Name, PerNs)], vis: Visibility, - is_from_glob: bool, + import_type: ImportType, ) { - self.update_recursive(module_id, resolutions, vis, is_from_glob, 0) + self.update_recursive(module_id, resolutions, vis, import_type, 0) } fn update_recursive( @@ -582,7 +585,7 @@ impl DefCollector<'_> { vis: Visibility, // All resolutions are imported with this glob status; the glob status // in the `PerNs` values are ignored and overwritten - is_from_glob: bool, + import_type: ImportType, depth: usize, ) { if depth > 100 { @@ -592,8 +595,12 @@ impl DefCollector<'_> { let scope = &mut self.def_map.modules[module_id].scope; let mut changed = false; for (name, res) in resolutions { - changed |= - scope.push_res(name.clone(), res.with_visibility(vis).from_glob(is_from_glob)); + changed |= scope.push_res( + &mut self.import_types, + name.clone(), + res.with_visibility(vis), + import_type, + ); } if !changed { @@ -616,7 +623,7 @@ impl DefCollector<'_> { glob_importing_module, resolutions, glob_import_vis, - true, + ImportType::Glob, depth + 1, ); } @@ -940,7 +947,7 @@ impl ModCollector<'_, '_> { self.module_id, &[(name.clone(), PerNs::from_def(id, vis, has_constructor))], vis, - false, + ImportType::Named, ) } } @@ -1047,7 +1054,7 @@ impl ModCollector<'_, '_> { self.module_id, &[(name, PerNs::from_def(def, vis, false))], vis, - false, + ImportType::Named, ); res } @@ -1177,6 +1184,7 @@ mod tests { mod_dirs: FxHashMap::default(), cfg_options: &CfgOptions::default(), proc_macros: Default::default(), + import_types: FxHashMap::default(), }; collector.collect(); collector.def_map diff --git a/crates/ra_hir_def/src/per_ns.rs b/crates/ra_hir_def/src/per_ns.rs index e5cbca71d..74665c588 100644 --- a/crates/ra_hir_def/src/per_ns.rs +++ b/crates/ra_hir_def/src/per_ns.rs @@ -9,7 +9,6 @@ use crate::{item_scope::ItemInNs, visibility::Visibility, ModuleDefId}; #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub struct PerNs { - pub from_glob: bool, pub types: Option<(ModuleDefId, Visibility)>, pub values: Option<(ModuleDefId, Visibility)>, pub macros: Option<(MacroDefId, Visibility)>, @@ -17,29 +16,29 @@ pub struct PerNs { impl Default for PerNs { fn default() -> Self { - PerNs { from_glob: false, types: None, values: None, macros: None } + PerNs { types: None, values: None, macros: None } } } impl PerNs { pub fn none() -> PerNs { - PerNs { from_glob: false, types: None, values: None, macros: None } + PerNs { types: None, values: None, macros: None } } pub fn values(t: ModuleDefId, v: Visibility) -> PerNs { - PerNs { from_glob: false, types: None, values: Some((t, v)), macros: None } + PerNs { types: None, values: Some((t, v)), macros: None } } pub fn types(t: ModuleDefId, v: Visibility) -> PerNs { - PerNs { from_glob: false, types: Some((t, v)), values: None, macros: None } + PerNs { types: Some((t, v)), values: None, macros: None } } pub fn both(types: ModuleDefId, values: ModuleDefId, v: Visibility) -> PerNs { - PerNs { from_glob: false, types: Some((types, v)), values: Some((values, v)), macros: None } + PerNs { types: Some((types, v)), values: Some((values, v)), macros: None } } pub fn macros(macro_: MacroDefId, v: Visibility) -> PerNs { - PerNs { from_glob: false, types: None, values: None, macros: Some((macro_, v)) } + PerNs { types: None, values: None, macros: Some((macro_, v)) } } pub fn is_none(&self) -> bool { @@ -64,7 +63,6 @@ impl PerNs { pub fn filter_visibility(self, mut f: impl FnMut(Visibility) -> bool) -> PerNs { PerNs { - from_glob: self.from_glob, types: self.types.filter(|(_, v)| f(*v)), values: self.values.filter(|(_, v)| f(*v)), macros: self.macros.filter(|(_, v)| f(*v)), @@ -73,7 +71,6 @@ impl PerNs { pub fn with_visibility(self, vis: Visibility) -> PerNs { PerNs { - from_glob: self.from_glob, types: self.types.map(|(it, _)| (it, vis)), values: self.values.map(|(it, _)| (it, vis)), macros: self.macros.map(|(it, _)| (it, vis)), @@ -82,7 +79,6 @@ impl PerNs { pub fn or(self, other: PerNs) -> PerNs { PerNs { - from_glob: self.from_glob, types: self.types.or(other.types), values: self.values.or(other.values), macros: self.macros.or(other.macros), @@ -96,8 +92,4 @@ impl PerNs { .chain(self.values.map(|it| ItemInNs::Values(it.0)).into_iter()) .chain(self.macros.map(|it| ItemInNs::Macros(it.0)).into_iter()) } - - pub fn from_glob(self, from_glob: bool) -> PerNs { - PerNs { from_glob, ..self } - } } -- cgit v1.2.3