Merge #2382

2382: Remove ids module r=matklad a=matklad



Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>

crates/ra_db/src/lib.rs

@@ -15,6 +15,20 @@ pub use crate::{
 pub use relative_path::{RelativePath, RelativePathBuf};
 pub use salsa;
 
+#[macro_export]
+macro_rules! impl_intern_key {
+    ($name:ident) => {
+        impl $crate::salsa::InternKey for $name {
+            fn from_intern_id(v: $crate::salsa::InternId) -> Self {
+                $name(v)
+            }
+            fn as_intern_id(&self) -> $crate::salsa::InternId {
+                self.0
+            }
+        }
+    };
+}
+
 pub trait CheckCanceled {
     /// Aborts current query if there are pending changes.
     ///

crates/ra_hir/src/code_model.rs

@@ -12,13 +12,14 @@ use hir_def::{
     per_ns::PerNs,
     resolver::{HasResolver, TypeNs},
     type_ref::TypeRef,
-    ContainerId, HasModule, ImplId, LocalEnumVariantId, LocalImportId, LocalModuleId,
-    LocalStructFieldId, Lookup, ModuleId, UnionId,
+    AstItemDef, ConstId, ContainerId, EnumId, FunctionId, HasModule, ImplId, LocalEnumVariantId,
+    LocalImportId, LocalModuleId, LocalStructFieldId, Lookup, ModuleId, StaticId, StructId,
+    TraitId, TypeAliasId, UnionId,
 };
 use hir_expand::{
     diagnostics::DiagnosticSink,
     name::{self, AsName},
-    AstId,
+    AstId, MacroDefId,
 };
 use ra_db::{CrateId, Edition, FileId, FilePosition};
 use ra_syntax::{ast, AstNode, SyntaxNode};
@@ -26,10 +27,6 @@ use ra_syntax::{ast, AstNode, SyntaxNode};
 use crate::{
     db::{DefDatabase, HirDatabase},
     expr::{BindingAnnotation, Body, BodySourceMap, ExprValidator, Pat, PatId},
-    ids::{
-        AstItemDef, ConstId, EnumId, FunctionId, MacroDefId, StaticId, StructId, TraitId,
-        TypeAliasId,
-    },
     ty::{InferenceResult, Namespace, TraitRef},
     Either, HasSource, Name, Source, Ty,
 };

crates/ra_hir/src/code_model/src.rs

@@ -1,12 +1,11 @@
 //! FIXME: write short doc here
 
-use hir_def::{HasChildSource, HasSource as _, Lookup, VariantId};
+use hir_def::{AstItemDef, HasChildSource, HasSource as _, Lookup, VariantId};
 use hir_expand::either::Either;
 use ra_syntax::ast::{self, AstNode};
 
 use crate::{
     db::{DefDatabase, HirDatabase},
-    ids::AstItemDef,
     Const, Enum, EnumVariant, FieldSource, Function, HasBody, Import, MacroDef, Module,
     ModuleSource, Static, Struct, StructField, Trait, TypeAlias, Union,
 };

crates/ra_hir/src/db.rs

@@ -5,7 +5,6 @@ use std::sync::Arc;
 use ra_db::salsa;
 
 use crate::{
-    ids,
     ty::{
         method_resolution::CrateImplBlocks,
         traits::{AssocTyValue, Impl},
@@ -71,11 +70,14 @@ pub trait HirDatabase: DefDatabase {
 
     // Interned IDs for Chalk integration
     #[salsa::interned]
-    fn intern_type_ctor(&self, type_ctor: TypeCtor) -> ids::TypeCtorId;
+    fn intern_type_ctor(&self, type_ctor: TypeCtor) -> crate::ty::TypeCtorId;
     #[salsa::interned]
-    fn intern_chalk_impl(&self, impl_: Impl) -> ids::GlobalImplId;
+    fn intern_chalk_impl(&self, impl_: Impl) -> crate::ty::traits::GlobalImplId;
     #[salsa::interned]
-    fn intern_assoc_ty_value(&self, assoc_ty_value: AssocTyValue) -> ids::AssocTyValueId;
+    fn intern_assoc_ty_value(
+        &self,
+        assoc_ty_value: AssocTyValue,
+    ) -> crate::ty::traits::AssocTyValueId;
 
     #[salsa::invoke(crate::ty::traits::chalk::associated_ty_data_query)]
     fn associated_ty_data(

crates/ra_hir/src/debug.rs

@@ -22,9 +22,10 @@
 
 use std::fmt;
 
+use hir_expand::HirFileId;
 use ra_db::{CrateId, FileId};
 
-use crate::{db::HirDatabase, Crate, HirFileId, Module, Name};
+use crate::{db::HirDatabase, Crate, Module, Name};
 
 impl Crate {
     pub fn debug(self, db: &impl HirDebugDatabase) -> impl fmt::Debug + '_ {

crates/ra_hir/src/diagnostics.rs

@@ -2,9 +2,10 @@
 
 use std::any::Any;
 
+use hir_expand::HirFileId;
 use ra_syntax::{ast, AstNode, AstPtr, SyntaxNodePtr};
 
-use crate::{db::AstDatabase, HirFileId, Name, Source};
+use crate::{db::AstDatabase, Name, Source};
 
 pub use hir_def::diagnostics::UnresolvedModule;
 pub use hir_expand::diagnostics::{AstDiagnostic, Diagnostic, DiagnosticSink};

crates/ra_hir/src/from_source.rs

@@ -1,6 +1,6 @@
 //! FIXME: write short doc here
 
-use hir_def::{ModuleId, StructId, StructOrUnionId, UnionId};
+use hir_def::{AstItemDef, LocationCtx, ModuleId, StructId, StructOrUnionId, UnionId};
 use hir_expand::{name::AsName, AstId, MacroDefId, MacroDefKind};
 use ra_syntax::{
     ast::{self, AstNode, NameOwner},
@@ -9,7 +9,6 @@ use ra_syntax::{
 
 use crate::{
     db::{AstDatabase, DefDatabase, HirDatabase},
-    ids::{AstItemDef, LocationCtx},
     AssocItem, Const, DefWithBody, Enum, EnumVariant, FieldSource, Function, HasBody, HasSource,
     ImplBlock, Local, MacroDef, Module, ModuleDef, ModuleSource, Source, Static, Struct,
     StructField, Trait, TypeAlias, Union, VariantDef,

crates/ra_hir/src/ids.rs

@@ -1,45 +0,0 @@
-//! hir makes heavy use of ids: integer (u32) handlers to various things. You
-//! can think of id as a pointer (but without a lifetime) or a file descriptor
-//! (but for hir objects).
-//!
-//! This module defines a bunch of ids we are using. The most important ones are
-//! probably `HirFileId` and `DefId`.
-
-use ra_db::salsa;
-
-pub use hir_def::{
-    AstItemDef, ConstId, EnumId, FunctionId, ItemLoc, LocationCtx, StaticId, StructId, TraitId,
-    TypeAliasId,
-};
-pub use hir_expand::{HirFileId, MacroCallId, MacroCallLoc, MacroDefId, MacroFile, MacroFileKind};
-
-macro_rules! impl_intern_key {
-    ($name:ident) => {
-        impl salsa::InternKey for $name {
-            fn from_intern_id(v: salsa::InternId) -> Self {
-                $name(v)
-            }
-            fn as_intern_id(&self) -> salsa::InternId {
-                self.0
-            }
-        }
-    };
-}
-
-/// This exists just for Chalk, because Chalk just has a single `StructId` where
-/// we have different kinds of ADTs, primitive types and special type
-/// constructors like tuples and function pointers.
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
-pub struct TypeCtorId(salsa::InternId);
-impl_intern_key!(TypeCtorId);
-
-/// This exists just for Chalk, because our ImplIds are only unique per module.
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
-pub struct GlobalImplId(salsa::InternId);
-impl_intern_key!(GlobalImplId);
-
-/// This exists just for Chalk, because it needs a unique ID for each associated
-/// type value in an impl (even synthetic ones).
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
-pub struct AssocTyValueId(salsa::InternId);
-impl_intern_key!(AssocTyValueId);

crates/ra_hir/src/lib.rs

@@ -31,7 +31,6 @@ pub mod debug;
 pub mod db;
 pub mod source_binder;
 
-mod ids;
 mod ty;
 mod impl_block;
 mod expr;
@@ -58,7 +57,6 @@ pub use crate::{
     },
     expr::ExprScopes,
     from_source::FromSource,
-    ids::{HirFileId, MacroCallId, MacroCallLoc, MacroDefId, MacroFile},
     source_binder::{PathResolution, ScopeEntryWithSyntax, SourceAnalyzer},
     ty::{
         display::HirDisplay,
@@ -73,4 +71,6 @@ pub use hir_def::{
     path::{Path, PathKind},
     type_ref::Mutability,
 };
-pub use hir_expand::{either::Either, name::Name, Source};
+pub use hir_expand::{
+    either::Either, name::Name, HirFileId, MacroCallId, MacroCallLoc, MacroDefId, MacroFile, Source,
+};

crates/ra_hir/src/source_binder.rs

@@ -11,9 +11,11 @@ use hir_def::{
     expr::{ExprId, PatId},
     path::known,
     resolver::{self, resolver_for_scope, HasResolver, Resolver, TypeNs, ValueNs},
-    DefWithBodyId,
+    DefWithBodyId, LocationCtx,
+};
+use hir_expand::{
+    name::AsName, AstId, HirFileId, MacroCallId, MacroCallLoc, MacroFileKind, Source,
 };
-use hir_expand::{name::AsName, AstId, MacroCallId, MacroCallLoc, MacroFileKind, Source};
 use ra_syntax::{
     ast::{self, AstNode},
     match_ast, AstPtr,
@@ -24,11 +26,10 @@ use ra_syntax::{
 use crate::{
     db::HirDatabase,
     expr::{BodySourceMap, ExprScopes, ScopeId},
-    ids::LocationCtx,
     ty::method_resolution::{self, implements_trait},
     Adt, AssocItem, Const, DefWithBody, Either, Enum, EnumVariant, FromSource, Function,
-    GenericParam, HasBody, HirFileId, Local, MacroDef, Module, Name, Path, ScopeDef, Static,
-    Struct, Trait, Ty, TypeAlias,
+    GenericParam, HasBody, Local, MacroDef, Module, Name, Path, ScopeDef, Static, Struct, Trait,
+    Ty, TypeAlias,
 };
 
 fn try_get_resolver_for_node(db: &impl HirDatabase, node: Source<&SyntaxNode>) -> Option<Resolver> {
@@ -544,7 +545,7 @@ fn adjust(
 }
 
 /// Given a `ast::MacroCall`, return what `MacroKindFile` it belongs to.
-/// FIXME: Not completed  
+/// FIXME: Not completed
 fn to_macro_file_kind(macro_call: &ast::MacroCall) -> MacroFileKind {
     let syn = macro_call.syntax();
     let parent = match syn.parent() {

crates/ra_hir/src/ty.rs

@@ -18,6 +18,7 @@ use std::sync::Arc;
 use std::{fmt, iter, mem};
 
 use hir_def::{generics::GenericParams, AdtId};
+use ra_db::{impl_intern_key, salsa};
 
 use crate::{
     db::HirDatabase, expr::ExprId, util::make_mut_slice, Adt, Crate, DefWithBody, FloatTy,
@@ -114,6 +115,13 @@ pub enum TypeCtor {
     Closure { def: DefWithBody, expr: ExprId },
 }
 
+/// This exists just for Chalk, because Chalk just has a single `StructId` where
+/// we have different kinds of ADTs, primitive types and special type
+/// constructors like tuples and function pointers.
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub struct TypeCtorId(salsa::InternId);
+impl_intern_key!(TypeCtorId);
+
 impl TypeCtor {
     pub fn num_ty_params(self, db: &impl HirDatabase) -> usize {
         match self {

crates/ra_hir/src/ty/traits.rs

@@ -3,7 +3,7 @@ use std::sync::{Arc, Mutex};
 
 use chalk_ir::{cast::Cast, family::ChalkIr};
 use log::debug;
-use ra_db::salsa;
+use ra_db::{impl_intern_key, salsa};
 use ra_prof::profile;
 use rustc_hash::FxHashSet;
 
@@ -304,6 +304,10 @@ pub enum Impl {
     /// Closure types implement the Fn traits synthetically.
     ClosureFnTraitImpl(ClosureFnTraitImplData),
 }
+/// This exists just for Chalk, because our ImplIds are only unique per module.
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub struct GlobalImplId(salsa::InternId);
+impl_intern_key!(GlobalImplId);
 
 /// An associated type value. Usually this comes from a `type` declaration
 /// inside an impl block, but for built-in impls we have to synthesize it.
@@ -315,3 +319,8 @@ pub enum AssocTyValue {
     /// The output type of the Fn trait implementation.
     ClosureFnTraitImplOutput(ClosureFnTraitImplData),
 }
+/// This exists just for Chalk, because it needs a unique ID for each associated
+/// type value in an impl (even synthetic ones).
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub struct AssocTyValueId(salsa::InternId);
+impl_intern_key!(AssocTyValueId);

crates/ra_hir/src/ty/traits/chalk.rs

@@ -847,38 +847,38 @@ fn id_to_chalk<T: InternKey>(salsa_id: T) -> chalk_ir::RawId {
     chalk_ir::RawId { index: salsa_id.as_intern_id().as_u32() }
 }
 
-impl From<chalk_ir::StructId> for crate::ids::TypeCtorId {
+impl From<chalk_ir::StructId> for crate::ty::TypeCtorId {
     fn from(struct_id: chalk_ir::StructId) -> Self {
         id_from_chalk(struct_id.0)
     }
 }
 
-impl From<crate::ids::TypeCtorId> for chalk_ir::StructId {
-    fn from(type_ctor_id: crate::ids::TypeCtorId) -> Self {
+impl From<crate::ty::TypeCtorId> for chalk_ir::StructId {
+    fn from(type_ctor_id: crate::ty::TypeCtorId) -> Self {
         chalk_ir::StructId(id_to_chalk(type_ctor_id))
     }
 }
 
-impl From<chalk_ir::ImplId> for crate::ids::GlobalImplId {
+impl From<chalk_ir::ImplId> for crate::ty::traits::GlobalImplId {
     fn from(impl_id: chalk_ir::ImplId) -> Self {
         id_from_chalk(impl_id.0)
     }
 }
 
-impl From<crate::ids::GlobalImplId> for chalk_ir::ImplId {
-    fn from(impl_id: crate::ids::GlobalImplId) -> Self {
+impl From<crate::ty::traits::GlobalImplId> for chalk_ir::ImplId {
+    fn from(impl_id: crate::ty::traits::GlobalImplId) -> Self {
         chalk_ir::ImplId(id_to_chalk(impl_id))
     }
 }
 
-impl From<chalk_rust_ir::AssociatedTyValueId> for crate::ids::AssocTyValueId {
+impl From<chalk_rust_ir::AssociatedTyValueId> for crate::ty::traits::AssocTyValueId {
     fn from(id: chalk_rust_ir::AssociatedTyValueId) -> Self {
         id_from_chalk(id.0)
     }
 }
 
-impl From<crate::ids::AssocTyValueId> for chalk_rust_ir::AssociatedTyValueId {
-    fn from(assoc_ty_value_id: crate::ids::AssocTyValueId) -> Self {
+impl From<crate::ty::traits::AssocTyValueId> for chalk_rust_ir::AssociatedTyValueId {
+    fn from(assoc_ty_value_id: crate::ty::traits::AssocTyValueId) -> Self {
         chalk_rust_ir::AssociatedTyValueId(id_to_chalk(assoc_ty_value_id))
     }
 }

crates/ra_hir_def/src/lib.rs

@@ -35,7 +35,7 @@ use std::hash::{Hash, Hasher};
 
 use hir_expand::{ast_id_map::FileAstId, db::AstDatabase, AstId, HirFileId, MacroDefId, Source};
 use ra_arena::{impl_arena_id, map::ArenaMap, RawId};
-use ra_db::{salsa, CrateId};
+use ra_db::{impl_intern_key, salsa, CrateId};
 use ra_syntax::{ast, AstNode};
 
 use crate::{builtin_type::BuiltinType, db::InternDatabase};
@@ -56,19 +56,6 @@ pub struct ModuleId {
 pub struct LocalModuleId(RawId);
 impl_arena_id!(LocalModuleId);
 
-macro_rules! impl_intern_key {
-    ($name:ident) => {
-        impl salsa::InternKey for $name {
-            fn from_intern_id(v: salsa::InternId) -> Self {
-                $name(v)
-            }
-            fn as_intern_id(&self) -> salsa::InternId {
-                self.0
-            }
-        }
-    };
-}
-
 #[derive(Debug)]
 pub struct ItemLoc<N: AstNode> {
     pub(crate) module: ModuleId,

crates/ra_hir_def/src/nameres.rs

@@ -66,7 +66,7 @@ use ra_arena::Arena;
 use ra_db::{CrateId, Edition, FileId};
 use ra_prof::profile;
 use ra_syntax::ast;
-use rustc_hash::{FxHashMap, FxHashSet};
+use rustc_hash::FxHashMap;
 
 use crate::{
     builtin_type::BuiltinType,
@@ -90,18 +90,6 @@ pub struct CrateDefMap {
     root: LocalModuleId,
     modules: Arena<LocalModuleId, ModuleData>,
 
-    /// Some macros are not well-behavior, which leads to infinite loop
-    /// e.g. macro_rules! foo { ($ty:ty) => { foo!($ty); } }
-    /// We mark it down and skip it in collector
-    ///
-    /// FIXME:
-    /// Right now it only handle a poison macro in a single crate,
-    /// such that if other crate try to call that macro,
-    /// the whole process will do again until it became poisoned in that crate.
-    /// We should handle this macro set globally
-    /// However, do we want to put it as a global variable?
-    poison_macros: FxHashSet<MacroDefId>,
-
     diagnostics: Vec<DefDiagnostic>,
 }
 
@@ -234,7 +222,6 @@ impl CrateDefMap {
                 prelude: None,
                 root,
                 modules,
-                poison_macros: FxHashSet::default(),
                 diagnostics: Vec::new(),
             }
         };
@@ -267,16 +254,6 @@ impl CrateDefMap {
         self.diagnostics.iter().for_each(|it| it.add_to(db, module, sink))
     }
 
-    pub fn resolve_path(
-        &self,
-        db: &impl DefDatabase,
-        original_module: LocalModuleId,
-        path: &Path,
-    ) -> (PerNs, Option<usize>) {
-        let res = self.resolve_path_fp_with_macro(db, ResolveMode::Other, original_module, path);
-        (res.resolved_def, res.segment_index)
-    }
-
     pub fn modules(&self) -> impl Iterator<Item = LocalModuleId> + '_ {
         self.modules.iter().map(|(id, _data)| id)
     }
@@ -287,6 +264,16 @@ impl CrateDefMap {
             .filter(move |(_id, data)| data.definition == Some(file_id))
             .map(|(id, _data)| id)
     }
+
+    pub(crate) fn resolve_path(
+        &self,
+        db: &impl DefDatabase,
+        original_module: LocalModuleId,
+        path: &Path,
+    ) -> (PerNs, Option<usize>) {
+        let res = self.resolve_path_fp_with_macro(db, ResolveMode::Other, original_module, path);
+        (res.resolved_def, res.segment_index)
+    }
 }
 
 impl ModuleData {

crates/ra_hir_def/src/nameres/collector.rs

@@ -8,7 +8,7 @@ use hir_expand::{
 use ra_cfg::CfgOptions;
 use ra_db::{CrateId, FileId};
 use ra_syntax::{ast, SmolStr};
-use rustc_hash::FxHashMap;
+use rustc_hash::{FxHashMap, FxHashSet};
 use test_utils::tested_by;
 
 use crate::{
@@ -57,6 +57,7 @@ pub(super) fn collect_defs(db: &impl DefDatabase, mut def_map: CrateDefMap) -> C
         unexpanded_macros: Vec::new(),
         mod_dirs: FxHashMap::default(),
         macro_stack_monitor: MacroStackMonitor::default(),
+        poison_macros: FxHashSet::default(),
         cfg_options,
     };
     collector.collect();
@@ -103,6 +104,17 @@ struct DefCollector<'a, DB> {
     /// Some macro use `$tt:tt which mean we have to handle the macro perfectly
     /// To prevent stack overflow, we add a deep counter here for prevent that.
     macro_stack_monitor: MacroStackMonitor,
+    /// Some macros are not well-behavior, which leads to infinite loop
+    /// e.g. macro_rules! foo { ($ty:ty) => { foo!($ty); } }
+    /// We mark it down and skip it in collector
+    ///
+    /// FIXME:
+    /// Right now it only handle a poison macro in a single crate,
+    /// such that if other crate try to call that macro,
+    /// the whole process will do again until it became poisoned in that crate.
+    /// We should handle this macro set globally
+    /// However, do we want to put it as a global variable?
+    poison_macros: FxHashSet<MacroDefId>,
 
     cfg_options: &'a CfgOptions,
 }
@@ -489,7 +501,7 @@ where
         macro_call_id: MacroCallId,
         macro_def_id: MacroDefId,
     ) {
-        if self.def_map.poison_macros.contains(&macro_def_id) {
+        if self.poison_macros.contains(&macro_def_id) {
             return;
         }
 
@@ -509,7 +521,7 @@ where
             .collect(raw_items.items());
         } else {
             log::error!("Too deep macro expansion: {:?}", macro_call_id);
-            self.def_map.poison_macros.insert(macro_def_id);
+            self.poison_macros.insert(macro_def_id);
         }
 
         self.macro_stack_monitor.decrease(macro_def_id);
@@ -807,7 +819,7 @@ mod tests {
         db: &impl DefDatabase,
         def_map: CrateDefMap,
         monitor: MacroStackMonitor,
-    ) -> CrateDefMap {
+    ) -> (CrateDefMap, FxHashSet<MacroDefId>) {
         let mut collector = DefCollector {
             db,
             def_map,
@@ -816,13 +828,18 @@ mod tests {
             unexpanded_macros: Vec::new(),
             mod_dirs: FxHashMap::default(),
             macro_stack_monitor: monitor,
+            poison_macros: FxHashSet::default(),
             cfg_options: &CfgOptions::default(),
         };
         collector.collect();
-        collector.finish()
+        (collector.def_map, collector.poison_macros)
     }
 
-    fn do_limited_resolve(code: &str, limit: u32, poison_limit: u32) -> CrateDefMap {
+    fn do_limited_resolve(
+        code: &str,
+        limit: u32,
+        poison_limit: u32,
+    ) -> (CrateDefMap, FxHashSet<MacroDefId>) {
         let (db, _file_id) = TestDB::with_single_file(&code);
         let krate = db.test_crate();
 
@@ -837,7 +854,6 @@ mod tests {
                 prelude: None,
                 root,
                 modules,
-                poison_macros: FxHashSet::default(),
                 diagnostics: Vec::new(),
             }
         };
@@ -867,7 +883,7 @@ foo!(KABOOM);
 
     #[test]
     fn test_macro_expand_poisoned() {
-        let def = do_limited_resolve(
+        let (_, poison_macros) = do_limited_resolve(
             r#"
         macro_rules! foo {
             ($ty:ty) => { foo!($ty); }
@@ -878,12 +894,12 @@ foo!(KABOOM);
             16,
         );
 
-        assert_eq!(def.poison_macros.len(), 1);
+        assert_eq!(poison_macros.len(), 1);
     }
 
     #[test]
     fn test_macro_expand_normal() {
-        let def = do_limited_resolve(
+        let (_, poison_macros) = do_limited_resolve(
             r#"
         macro_rules! foo {
             ($ident:ident) => { struct $ident {} }
@@ -894,6 +910,6 @@ foo!(Bar);
             16,
         );
 
-        assert_eq!(def.poison_macros.len(), 0);
+        assert_eq!(poison_macros.len(), 0);
     }
 }