Merge #2645

2645: Simplify r=matklad a=matklad



Co-authored-by: Aleksey Kladov <[email protected]>

7 files changed, 108 insertions, 140 deletions

diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs
index 4cd28eb4e..bcfc0d03e 100644
--- a/crates/ra_hir/src/code_model.rs
+++ b/crates/ra_hir/src/code_model.rs
@@ -184,7 +184,7 @@ impl Module {
         db.crate_def_map(self.id.krate)[self.id.local_id]
             .scope
             .entries()
-            .map(|(name, res)| (name.clone(), res.def.into()))
+            .map(|(name, def)| (name.clone(), def.into()))
             .collect()
     }
 
diff --git a/crates/ra_hir_def/src/body/scope.rs b/crates/ra_hir_def/src/body/scope.rs
index ab6599b23..a63552327 100644
--- a/crates/ra_hir_def/src/body/scope.rs
+++ b/crates/ra_hir_def/src/body/scope.rs
@@ -183,8 +183,8 @@ mod tests {
         let crate_def_map = db.crate_def_map(krate);
 
         let module = crate_def_map.modules_for_file(file_id).next().unwrap();
-        let (_, res) = crate_def_map[module].scope.entries().next().unwrap();
-        match res.def.take_values().unwrap() {
+        let (_, def) = crate_def_map[module].scope.entries().next().unwrap();
+        match def.take_values().unwrap() {
             ModuleDefId::FunctionId(it) => it,
             _ => panic!(),
         }
diff --git a/crates/ra_hir_def/src/item_scope.rs b/crates/ra_hir_def/src/item_scope.rs
index 9e082c5f7..f1adc3b58 100644
--- a/crates/ra_hir_def/src/item_scope.rs
+++ b/crates/ra_hir_def/src/item_scope.rs
@@ -5,11 +5,12 @@ use hir_expand::name::Name;
 use once_cell::sync::Lazy;
 use rustc_hash::FxHashMap;
 
-use crate::{per_ns::PerNs, BuiltinType, ImplId, MacroDefId, ModuleDefId, TraitId};
+use crate::{per_ns::PerNs, AdtId, BuiltinType, ImplId, MacroDefId, ModuleDefId, TraitId};
 
 #[derive(Debug, Default, PartialEq, Eq)]
 pub struct ItemScope {
-    items: FxHashMap<Name, Resolution>,
+    visible: FxHashMap<Name, PerNs>,
+    defs: Vec<ModuleDefId>,
     impls: Vec<ImplId>,
     /// Macros visible in current module in legacy textual scope
     ///
@@ -26,12 +27,10 @@ pub struct ItemScope {
     legacy_macros: FxHashMap<Name, MacroDefId>,
 }
 
-static BUILTIN_SCOPE: Lazy<FxHashMap<Name, Resolution>> = Lazy::new(|| {
+static BUILTIN_SCOPE: Lazy<FxHashMap<Name, PerNs>> = Lazy::new(|| {
     BuiltinType::ALL
         .iter()
-        .map(|(name, ty)| {
-            (name.clone(), Resolution { def: PerNs::types(ty.clone().into()), import: false })
-        })
+        .map(|(name, ty)| (name.clone(), PerNs::types(ty.clone().into())))
         .collect()
 });
 
@@ -47,17 +46,13 @@ pub(crate) enum BuiltinShadowMode {
 /// Legacy macros can only be accessed through special methods like `get_legacy_macros`.
 /// Other methods will only resolve values, types and module scoped macros only.
 impl ItemScope {
-    pub fn entries<'a>(&'a self) -> impl Iterator<Item = (&'a Name, &'a Resolution)> + 'a {
+    pub fn entries<'a>(&'a self) -> impl Iterator<Item = (&'a Name, PerNs)> + 'a {
         //FIXME: shadowing
-        self.items.iter().chain(BUILTIN_SCOPE.iter())
+        self.visible.iter().chain(BUILTIN_SCOPE.iter()).map(|(n, def)| (n, *def))
     }
 
     pub fn declarations(&self) -> impl Iterator<Item = ModuleDefId> + '_ {
-        self.entries()
-            .filter_map(|(_name, res)| if !res.import { Some(res.def) } else { None })
-            .flat_map(|per_ns| {
-                per_ns.take_types().into_iter().chain(per_ns.take_values().into_iter())
-            })
+        self.defs.iter().copied()
     }
 
     pub fn impls(&self) -> impl Iterator<Item = ImplId> + ExactSizeIterator + '_ {
@@ -66,9 +61,7 @@ impl ItemScope {
 
     /// Iterate over all module scoped macros
     pub(crate) fn macros<'a>(&'a self) -> impl Iterator<Item = (&'a Name, MacroDefId)> + 'a {
-        self.items
-            .iter()
-            .filter_map(|(name, res)| res.def.take_macros().map(|macro_| (name, macro_)))
+        self.visible.iter().filter_map(|(name, def)| def.take_macros().map(|macro_| (name, macro_)))
     }
 
     /// Iterate over all legacy textual scoped macros visible at the end of the module
@@ -77,13 +70,13 @@ impl ItemScope {
     }
 
     /// Get a name from current module scope, legacy macros are not included
-    pub(crate) fn get(&self, name: &Name, shadow: BuiltinShadowMode) -> Option<&Resolution> {
+    pub(crate) fn get(&self, name: &Name, shadow: BuiltinShadowMode) -> Option<&PerNs> {
         match shadow {
-            BuiltinShadowMode::Module => self.items.get(name).or_else(|| BUILTIN_SCOPE.get(name)),
+            BuiltinShadowMode::Module => self.visible.get(name).or_else(|| BUILTIN_SCOPE.get(name)),
             BuiltinShadowMode::Other => {
-                let item = self.items.get(name);
-                if let Some(res) = item {
-                    if let Some(ModuleDefId::ModuleId(_)) = res.def.take_types() {
+                let item = self.visible.get(name);
+                if let Some(def) = item {
+                    if let Some(ModuleDefId::ModuleId(_)) = def.take_types() {
                         return BUILTIN_SCOPE.get(name).or(item);
                     }
                 }
@@ -94,12 +87,16 @@ impl ItemScope {
     }
 
     pub(crate) fn traits<'a>(&'a self) -> impl Iterator<Item = TraitId> + 'a {
-        self.items.values().filter_map(|r| match r.def.take_types() {
+        self.visible.values().filter_map(|def| match def.take_types() {
             Some(ModuleDefId::TraitId(t)) => Some(t),
             _ => None,
         })
     }
 
+    pub(crate) fn define_def(&mut self, def: ModuleDefId) {
+        self.defs.push(def)
+    }
+
     pub(crate) fn get_legacy_macro(&self, name: &Name) -> Option<MacroDefId> {
         self.legacy_macros.get(name).copied()
     }
@@ -112,34 +109,28 @@ impl ItemScope {
         self.legacy_macros.insert(name, mac);
     }
 
-    pub(crate) fn push_res(&mut self, name: Name, res: &Resolution, import: bool) -> bool {
+    pub(crate) fn push_res(&mut self, name: Name, def: &PerNs) -> bool {
         let mut changed = false;
-        let existing = self.items.entry(name.clone()).or_default();
+        let existing = self.visible.entry(name.clone()).or_default();
 
-        if existing.def.types.is_none() && res.def.types.is_some() {
-            existing.def.types = res.def.types;
-            existing.import = import || res.import;
+        if existing.types.is_none() && def.types.is_some() {
+            existing.types = def.types;
             changed = true;
         }
-        if existing.def.values.is_none() && res.def.values.is_some() {
-            existing.def.values = res.def.values;
-            existing.import = import || res.import;
+        if existing.values.is_none() && def.values.is_some() {
+            existing.values = def.values;
             changed = true;
         }
-        if existing.def.macros.is_none() && res.def.macros.is_some() {
-            existing.def.macros = res.def.macros;
-            existing.import = import || res.import;
+        if existing.macros.is_none() && def.macros.is_some() {
+            existing.macros = def.macros;
             changed = true;
         }
 
-        if existing.def.is_none() && res.def.is_none() && !existing.import && res.import {
-            existing.import = res.import;
-        }
         changed
     }
 
-    pub(crate) fn collect_resolutions(&self) -> Vec<(Name, Resolution)> {
-        self.items.iter().map(|(name, res)| (name.clone(), res.clone())).collect()
+    pub(crate) fn collect_resolutions(&self) -> Vec<(Name, PerNs)> {
+        self.visible.iter().map(|(name, res)| (name.clone(), res.clone())).collect()
     }
 
     pub(crate) fn collect_legacy_macros(&self) -> FxHashMap<Name, MacroDefId> {
@@ -147,9 +138,20 @@ impl ItemScope {
     }
 }
 
-#[derive(Debug, Clone, PartialEq, Eq, Default)]
-pub struct Resolution {
-    /// None for unresolved
-    pub def: PerNs,
-    pub(crate) import: bool,
+impl From<ModuleDefId> for PerNs {
+    fn from(def: ModuleDefId) -> PerNs {
+        match def {
+            ModuleDefId::ModuleId(_) => PerNs::types(def),
+            ModuleDefId::FunctionId(_) => PerNs::values(def),
+            ModuleDefId::AdtId(adt) => match adt {
+                AdtId::StructId(_) | AdtId::UnionId(_) => PerNs::both(def, def),
+                AdtId::EnumId(_) => PerNs::types(def),
+            },
+            ModuleDefId::EnumVariantId(_) => PerNs::both(def, def),
+            ModuleDefId::ConstId(_) | ModuleDefId::StaticId(_) => PerNs::values(def),
+            ModuleDefId::TraitId(_) => PerNs::types(def),
+            ModuleDefId::TypeAliasId(_) => PerNs::types(def),
+            ModuleDefId::BuiltinType(_) => PerNs::types(def),
+        }
+    }
 }
diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs
index 2b194f488..4f1fd4801 100644
--- a/crates/ra_hir_def/src/nameres/collector.rs
+++ b/crates/ra_hir_def/src/nameres/collector.rs
@@ -18,7 +18,6 @@ use test_utils::tested_by;
 use crate::{
     attr::Attrs,
     db::DefDatabase,
-    item_scope::Resolution,
     nameres::{
         diagnostics::DefDiagnostic, mod_resolution::ModDir, path_resolution::ReachedFixedPoint,
         raw, BuiltinShadowMode, CrateDefMap, ModuleData, ModuleOrigin, ResolveMode,
@@ -215,11 +214,7 @@ where
         // In Rust, `#[macro_export]` macros are unconditionally visible at the
         // crate root, even if the parent modules is **not** visible.
         if export {
-            self.update(
-                self.def_map.root,
-                None,
-                &[(name, Resolution { def: PerNs::macros(macro_), import: false })],
-            );
+            self.update(self.def_map.root, &[(name, PerNs::macros(macro_))]);
         }
     }
 
@@ -373,7 +368,7 @@ where
                         // Module scoped macros is included
                         let items = scope.collect_resolutions();
 
-                        self.update(module_id, Some(import_id), &items);
+                        self.update(module_id, &items);
                     } else {
                         // glob import from same crate => we do an initial
                         // import, and then need to propagate any further
@@ -383,7 +378,7 @@ where
                         // Module scoped macros is included
                         let items = scope.collect_resolutions();
 
-                        self.update(module_id, Some(import_id), &items);
+                        self.update(module_id, &items);
                         // 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(|it| *it == (module_id, import_id)) {
@@ -401,14 +396,11 @@ where
                         .map(|(local_id, variant_data)| {
                             let name = variant_data.name.clone();
                             let variant = EnumVariantId { parent: e, local_id };
-                            let res = Resolution {
-                                def: PerNs::both(variant.into(), variant.into()),
-                                import: true,
-                            };
+                            let res = PerNs::both(variant.into(), variant.into());
                             (name, res)
                         })
                         .collect::<Vec<_>>();
-                    self.update(module_id, Some(import_id), &resolutions);
+                    self.update(module_id, &resolutions);
                 }
                 Some(d) => {
                     log::debug!("glob import {:?} from non-module/enum {:?}", import, d);
@@ -430,28 +422,21 @@ where
                         }
                     }
 
-                    let resolution = Resolution { def, import: true };
-                    self.update(module_id, Some(import_id), &[(name, resolution)]);
+                    self.update(module_id, &[(name, def)]);
                 }
                 None => tested_by!(bogus_paths),
             }
         }
     }
 
-    fn update(
-        &mut self,
-        module_id: LocalModuleId,
-        import: Option<raw::Import>,
-        resolutions: &[(Name, Resolution)],
-    ) {
-        self.update_recursive(module_id, import, resolutions, 0)
+    fn update(&mut self, module_id: LocalModuleId, resolutions: &[(Name, PerNs)]) {
+        self.update_recursive(module_id, resolutions, 0)
     }
 
     fn update_recursive(
         &mut self,
         module_id: LocalModuleId,
-        import: Option<raw::Import>,
-        resolutions: &[(Name, Resolution)],
+        resolutions: &[(Name, PerNs)],
         depth: usize,
     ) {
         if depth > 100 {
@@ -461,7 +446,7 @@ where
         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, import.is_some());
+            changed |= scope.push_res(name.clone(), res);
         }
 
         if !changed {
@@ -474,9 +459,9 @@ where
             .flat_map(|v| v.iter())
             .cloned()
             .collect::<Vec<_>>();
-        for (glob_importing_module, glob_import) in glob_imports {
+        for (glob_importing_module, _glob_import) in glob_imports {
             // We pass the glob import so that the tracked import in those modules is that glob import
-            self.update_recursive(glob_importing_module, Some(glob_import), resolutions, depth + 1);
+            self.update_recursive(glob_importing_module, resolutions, depth + 1);
         }
     }
 
@@ -714,13 +699,10 @@ where
             modules[res].scope.define_legacy_macro(name, mac)
         }
         modules[self.module_id].children.insert(name.clone(), res);
-        let resolution = Resolution {
-            def: PerNs::types(
-                ModuleId { krate: self.def_collector.def_map.krate, local_id: res }.into(),
-            ),
-            import: false,
-        };
-        self.def_collector.update(self.module_id, None, &[(name, resolution)]);
+        let module = ModuleId { krate: self.def_collector.def_map.krate, local_id: res };
+        let def: ModuleDefId = module.into();
+        self.def_collector.def_map.modules[self.module_id].scope.define_def(def);
+        self.def_collector.update(self.module_id, &[(name, def.into())]);
         res
     }
 
@@ -734,64 +716,52 @@ where
 
         let name = def.name.clone();
         let container = ContainerId::ModuleId(module);
-        let def: PerNs = match def.kind {
-            raw::DefKind::Function(ast_id) => {
-                let def = FunctionLoc {
-                    container: container.into(),
-                    ast_id: AstId::new(self.file_id, ast_id),
-                }
-                .intern(self.def_collector.db);
-
-                PerNs::values(def.into())
+        let def: ModuleDefId = match def.kind {
+            raw::DefKind::Function(ast_id) => FunctionLoc {
+                container: container.into(),
+                ast_id: AstId::new(self.file_id, ast_id),
             }
+            .intern(self.def_collector.db)
+            .into(),
             raw::DefKind::Struct(ast_id) => {
-                let def = StructLoc { container, ast_id: AstId::new(self.file_id, ast_id) }
-                    .intern(self.def_collector.db);
-                PerNs::both(def.into(), def.into())
+                StructLoc { container, ast_id: AstId::new(self.file_id, ast_id) }
+                    .intern(self.def_collector.db)
+                    .into()
             }
             raw::DefKind::Union(ast_id) => {
-                let def = UnionLoc { container, ast_id: AstId::new(self.file_id, ast_id) }
-                    .intern(self.def_collector.db);
-                PerNs::both(def.into(), def.into())
+                UnionLoc { container, ast_id: AstId::new(self.file_id, ast_id) }
+                    .intern(self.def_collector.db)
+                    .into()
             }
             raw::DefKind::Enum(ast_id) => {
-                let def = EnumLoc { container, ast_id: AstId::new(self.file_id, ast_id) }
-                    .intern(self.def_collector.db);
-                PerNs::types(def.into())
+                EnumLoc { container, ast_id: AstId::new(self.file_id, ast_id) }
+                    .intern(self.def_collector.db)
+                    .into()
             }
             raw::DefKind::Const(ast_id) => {
-                let def = ConstLoc {
-                    container: container.into(),
-                    ast_id: AstId::new(self.file_id, ast_id),
-                }
-                .intern(self.def_collector.db);
-
-                PerNs::values(def.into())
+                ConstLoc { container: container.into(), ast_id: AstId::new(self.file_id, ast_id) }
+                    .intern(self.def_collector.db)
+                    .into()
             }
             raw::DefKind::Static(ast_id) => {
-                let def = StaticLoc { container, ast_id: AstId::new(self.file_id, ast_id) }
-                    .intern(self.def_collector.db);
-
-                PerNs::values(def.into())
+                StaticLoc { container, ast_id: AstId::new(self.file_id, ast_id) }
+                    .intern(self.def_collector.db)
+                    .into()
             }
             raw::DefKind::Trait(ast_id) => {
-                let def = TraitLoc { container, ast_id: AstId::new(self.file_id, ast_id) }
-                    .intern(self.def_collector.db);
-
-                PerNs::types(def.into())
+                TraitLoc { container, ast_id: AstId::new(self.file_id, ast_id) }
+                    .intern(self.def_collector.db)
+                    .into()
             }
-            raw::DefKind::TypeAlias(ast_id) => {
-                let def = TypeAliasLoc {
-                    container: container.into(),
-                    ast_id: AstId::new(self.file_id, ast_id),
-                }
-                .intern(self.def_collector.db);
-
-                PerNs::types(def.into())
+            raw::DefKind::TypeAlias(ast_id) => TypeAliasLoc {
+                container: container.into(),
+                ast_id: AstId::new(self.file_id, ast_id),
             }
+            .intern(self.def_collector.db)
+            .into(),
         };
-        let resolution = Resolution { def, import: false };
-        self.def_collector.update(self.module_id, None, &[(name, resolution)])
+        self.def_collector.def_map.modules[self.module_id].scope.define_def(def);
+        self.def_collector.update(self.module_id, &[(name, def.into())])
     }
 
     fn collect_derives(&mut self, attrs: &Attrs, def: &raw::DefData) {
diff --git a/crates/ra_hir_def/src/nameres/path_resolution.rs b/crates/ra_hir_def/src/nameres/path_resolution.rs
index 2dd779b66..378d49455 100644
--- a/crates/ra_hir_def/src/nameres/path_resolution.rs
+++ b/crates/ra_hir_def/src/nameres/path_resolution.rs
@@ -181,7 +181,7 @@ impl CrateDefMap {
 
                     // Since it is a qualified path here, it should not contains legacy macros
                     match self[module.local_id].scope.get(&segment, prefer_module(i)) {
-                        Some(res) => res.def,
+                        Some(def) => *def,
                         _ => {
                             log::debug!("path segment {:?} not found", segment);
                             return ResolvePathResult::empty(ReachedFixedPoint::No);
@@ -243,8 +243,7 @@ impl CrateDefMap {
         //  - std prelude
         let from_legacy_macro =
             self[module].scope.get_legacy_macro(name).map_or_else(PerNs::none, PerNs::macros);
-        let from_scope =
-            self[module].scope.get(name, shadow).map_or_else(PerNs::none, |res| res.def);
+        let from_scope = self[module].scope.get(name, shadow).copied().unwrap_or_else(PerNs::none);
         let from_extern_prelude =
             self.extern_prelude.get(name).map_or(PerNs::none(), |&it| PerNs::types(it));
         let from_prelude = self.resolve_in_prelude(db, name, shadow);
@@ -258,7 +257,7 @@ impl CrateDefMap {
         shadow: BuiltinShadowMode,
     ) -> PerNs {
         let from_crate_root =
-            self[self.root].scope.get(name, shadow).map_or_else(PerNs::none, |res| res.def);
+            self[self.root].scope.get(name, shadow).copied().unwrap_or_else(PerNs::none);
         let from_extern_prelude = self.resolve_name_in_extern_prelude(name);
 
         from_crate_root.or(from_extern_prelude)
@@ -279,10 +278,7 @@ impl CrateDefMap {
                 keep = db.crate_def_map(prelude.krate);
                 &keep
             };
-            def_map[prelude.local_id]
-                .scope
-                .get(name, shadow)
-                .map_or_else(PerNs::none, |res| res.def)
+            def_map[prelude.local_id].scope.get(name, shadow).copied().unwrap_or_else(PerNs::none)
         } else {
             PerNs::none()
         }
diff --git a/crates/ra_hir_def/src/nameres/tests.rs b/crates/ra_hir_def/src/nameres/tests.rs
index 4e968bcc8..ff474b53b 100644
--- a/crates/ra_hir_def/src/nameres/tests.rs
+++ b/crates/ra_hir_def/src/nameres/tests.rs
@@ -35,19 +35,19 @@ fn render_crate_def_map(map: &CrateDefMap) -> String {
         let mut entries = map.modules[module].scope.collect_resolutions();
         entries.sort_by_key(|(name, _)| name.clone());
 
-        for (name, res) in entries {
+        for (name, def) in entries {
             *buf += &format!("{}:", name);
 
-            if res.def.types.is_some() {
+            if def.types.is_some() {
                 *buf += " t";
             }
-            if res.def.values.is_some() {
+            if def.values.is_some() {
                 *buf += " v";
             }
-            if res.def.macros.is_some() {
+            if def.macros.is_some() {
                 *buf += " m";
             }
-            if res.def.is_none() {
+            if def.is_none() {
                 *buf += " _";
             }
 
diff --git a/crates/ra_hir_def/src/resolver.rs b/crates/ra_hir_def/src/resolver.rs
index 83013fed3..e70049617 100644
--- a/crates/ra_hir_def/src/resolver.rs
+++ b/crates/ra_hir_def/src/resolver.rs
@@ -413,8 +413,8 @@ impl Scope {
                 //         def: m.module.into(),
                 //     }),
                 // );
-                m.crate_def_map[m.module_id].scope.entries().for_each(|(name, res)| {
-                    f(name.clone(), ScopeDef::PerNs(res.def));
+                m.crate_def_map[m.module_id].scope.entries().for_each(|(name, def)| {
+                    f(name.clone(), ScopeDef::PerNs(def));
                 });
                 m.crate_def_map[m.module_id].scope.legacy_macros().for_each(|(name, macro_)| {
                     f(name.clone(), ScopeDef::PerNs(PerNs::macros(macro_)));
@@ -424,8 +424,8 @@ impl Scope {
                 });
                 if let Some(prelude) = m.crate_def_map.prelude {
                     let prelude_def_map = db.crate_def_map(prelude.krate);
-                    prelude_def_map[prelude.local_id].scope.entries().for_each(|(name, res)| {
-                        f(name.clone(), ScopeDef::PerNs(res.def));
+                    prelude_def_map[prelude.local_id].scope.entries().for_each(|(name, def)| {
+                        f(name.clone(), ScopeDef::PerNs(def));
                     });
                 }
             }