From de9e964e4ac21897bd48adbe37f379d74422919f Mon Sep 17 00:00:00 2001
From: Paul Daniel Faria <Nashenas88@users.noreply.github.com>
Date: Thu, 25 Jun 2020 12:42:12 -0400
Subject: [PATCH] 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(-)
diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs
index 3ced648e56f..d749c828da0 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<HirFileId, Arc<ItemTree>>,
+ import_types: FxHashMap<Name, ImportType>,
}
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 0184b6af9e5..511c08a8d78 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<Name, PerNs>,
@@ -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, ImportType>,
+ 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 93f58e2c761..f7b99e0bef7 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<LocalModuleId, ModDir>,
cfg_options: &'a CfgOptions,
proc_macros: Vec<(Name, ProcMacroExpander)>,
+ import_types: FxHashMap<Name, ImportType>,
}
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::<Vec<_>>();
- 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::<Vec<_>>();
- 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::<Vec<_>>();
- 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 e5cbca71d61..74665c58851 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 }
- }
}