Merge #2387

2387: Simplify r=matklad a=matklad



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

crates/ra_cli/src/analysis_stats.rs

@@ -109,7 +109,7 @@ pub fn run(
         }
         let body = f.body(db);
         let inference_result = f.infer(db);
-        for (expr_id, _) in body.exprs() {
+        for (expr_id, _) in body.exprs.iter() {
             let ty = &inference_result[expr_id];
             num_exprs += 1;
             if let Ty::Unknown = ty {

crates/ra_hir/src/code_model.rs

@@ -62,7 +62,7 @@ impl Crate {
     }
 
     pub fn root_module(self, db: &impl DefDatabase) -> Option<Module> {
-        let module_id = db.crate_def_map(self.crate_id).root();
+        let module_id = db.crate_def_map(self.crate_id).root;
         Some(Module::new(self, module_id))
     }
 
@@ -195,7 +195,7 @@ impl Module {
     /// in the module tree of any target in `Cargo.toml`.
     pub fn crate_root(self, db: &impl DefDatabase) -> Module {
         let def_map = db.crate_def_map(self.id.krate);
-        self.with_module_id(def_map.root())
+        self.with_module_id(def_map.root)
     }
 
     /// Finds a child module with the specified name.

crates/ra_hir/src/expr.rs

@@ -44,15 +44,15 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
     pub(crate) fn validate_body(&mut self, db: &impl HirDatabase) {
         let body = self.func.body(db);
 
-        for e in body.exprs() {
+        for e in body.exprs.iter() {
             if let (id, Expr::RecordLit { path, fields, spread }) = e {
                 self.validate_record_literal(id, path, fields, *spread, db);
             }
         }
 
-        let body_expr = &body[body.body_expr()];
+        let body_expr = &body[body.body_expr];
         if let Expr::Block { statements: _, tail: Some(t) } = body_expr {
-            self.validate_results_in_tail_expr(body.body_expr(), *t, db);
+            self.validate_results_in_tail_expr(body.body_expr, *t, db);
         }
     }
 

crates/ra_hir/src/test_db.rs

@@ -80,7 +80,7 @@ impl TestDB {
         let crate_graph = self.crate_graph();
         for krate in crate_graph.iter().next() {
             let crate_def_map = self.crate_def_map(krate);
-            for module_id in crate_def_map.modules() {
+            for (module_id, _) in crate_def_map.modules.iter() {
                 let module_id = ModuleId { krate, module_id };
                 let module = crate::Module::from(module_id);
                 module.diagnostics(

crates/ra_hir/src/ty/infer.rs

@@ -565,7 +565,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
 
     fn collect_fn(&mut self, data: &FunctionData) {
         let body = Arc::clone(&self.body); // avoid borrow checker problem
-        for (type_ref, pat) in data.params.iter().zip(body.params()) {
+        for (type_ref, pat) in data.params.iter().zip(body.params.iter()) {
             let ty = self.make_ty(type_ref);
 
             self.infer_pat(*pat, &ty, BindingMode::default());
@@ -574,7 +574,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
     }
 
     fn infer_body(&mut self) {
-        self.infer_expr(self.body.body_expr(), &Expectation::has_type(self.return_ty.clone()));
+        self.infer_expr(self.body.body_expr, &Expectation::has_type(self.return_ty.clone()));
     }
 
     fn resolve_into_iter_item(&self) -> Option<TypeAlias> {

crates/ra_hir_def/src/adt.rs

@@ -95,7 +95,7 @@ fn lower_enum(
                 name: var.name().map(|it| it.as_name()),
                 variant_data: Arc::new(VariantData::new(var.kind())),
             },
-        )
+        );
     }
 }
 
@@ -160,7 +160,7 @@ fn lower_struct(
                         name: Name::new_tuple_field(i),
                         type_ref: TypeRef::from_ast_opt(fd.type_ref()),
                     },
-                )
+                );
             }
             StructKind::Tuple
         }
@@ -172,7 +172,7 @@ fn lower_struct(
                         name: fd.name().map(|n| n.as_name()).unwrap_or_else(Name::missing),
                         type_ref: TypeRef::from_ast_opt(fd.ascribed_type()),
                     },
-                )
+                );
             }
             StructKind::Record
         }

crates/ra_hir_def/src/body.rs

@@ -20,7 +20,7 @@ use crate::{
     DefWithBodyId, HasModule, HasSource, Lookup, ModuleId,
 };
 
-pub struct Expander {
+struct Expander {
     crate_def_map: Arc<CrateDefMap>,
     current_file_id: HirFileId,
     hygiene: Hygiene,
@@ -28,7 +28,7 @@ pub struct Expander {
 }
 
 impl Expander {
-    pub fn new(db: &impl DefDatabase, current_file_id: HirFileId, module: ModuleId) -> Expander {
+    fn new(db: &impl DefDatabase, current_file_id: HirFileId, module: ModuleId) -> Expander {
         let crate_def_map = db.crate_def_map(module.krate);
         let hygiene = Hygiene::new(db, current_file_id);
         Expander { crate_def_map, current_file_id, hygiene, module }
@@ -101,17 +101,17 @@ impl Drop for Mark {
 /// The body of an item (function, const etc.).
 #[derive(Debug, Eq, PartialEq)]
 pub struct Body {
-    exprs: Arena<ExprId, Expr>,
-    pats: Arena<PatId, Pat>,
+    pub exprs: Arena<ExprId, Expr>,
+    pub pats: Arena<PatId, Pat>,
     /// The patterns for the function's parameters. While the parameter types are
     /// part of the function signature, the patterns are not (they don't change
     /// the external type of the function).
     ///
     /// If this `Body` is for the body of a constant, this will just be
     /// empty.
-    params: Vec<PatId>,
+    pub params: Vec<PatId>,
     /// The `ExprId` of the actual body expression.
-    body_expr: ExprId,
+    pub body_expr: ExprId,
 }
 
 pub type ExprPtr = Either<AstPtr<ast::Expr>, AstPtr<ast::RecordField>>;
@@ -182,22 +182,6 @@ impl Body {
     ) -> (Body, BodySourceMap) {
         lower::lower(db, expander, params, body)
     }
-
-    pub fn params(&self) -> &[PatId] {
-        &self.params
-    }
-
-    pub fn body_expr(&self) -> ExprId {
-        self.body_expr
-    }
-
-    pub fn exprs(&self) -> impl Iterator<Item = (ExprId, &Expr)> {
-        self.exprs.iter()
-    }
-
-    pub fn pats(&self) -> impl Iterator<Item = (PatId, &Pat)> {
-        self.pats.iter()
-    }
 }
 
 impl Index<ExprId> for Body {

crates/ra_hir_def/src/body/scope.rs

@@ -54,8 +54,8 @@ impl ExprScopes {
         let mut scopes =
             ExprScopes { scopes: Arena::default(), scope_by_expr: FxHashMap::default() };
         let root = scopes.root_scope();
-        scopes.add_params_bindings(body, root, body.params());
-        compute_expr_scopes(body.body_expr(), body, &mut scopes, root);
+        scopes.add_params_bindings(body, root, &body.params);
+        compute_expr_scopes(body.body_expr, body, &mut scopes, root);
         scopes
     }
 

crates/ra_hir_def/src/lang_item.rs

@@ -39,8 +39,9 @@ impl LangItems {
         let crate_def_map = db.crate_def_map(krate);
 
         crate_def_map
-            .modules()
-            .filter_map(|module_id| db.module_lang_items(ModuleId { krate, module_id }))
+            .modules
+            .iter()
+            .filter_map(|(module_id, _)| db.module_lang_items(ModuleId { krate, module_id }))
             .for_each(|it| lang_items.items.extend(it.items.iter().map(|(k, v)| (k.clone(), *v))));
 
         Arc::new(lang_items)

crates/ra_hir_def/src/nameres.rs

@@ -80,16 +80,16 @@ use crate::{
 /// Contains all top-level defs from a macro-expanded crate
 #[derive(Debug, PartialEq, Eq)]
 pub struct CrateDefMap {
-    krate: CrateId,
-    edition: Edition,
+    pub root: LocalModuleId,
+    pub modules: Arena<LocalModuleId, ModuleData>,
+    pub(crate) krate: CrateId,
     /// The prelude module for this crate. This either comes from an import
     /// marked with the `prelude_import` attribute, or (in the normal case) from
     /// a dependency (`std` or `core`).
-    prelude: Option<ModuleId>,
-    extern_prelude: FxHashMap<Name, ModuleDefId>,
-    root: LocalModuleId,
-    modules: Arena<LocalModuleId, ModuleData>,
+    pub(crate) prelude: Option<ModuleId>,
+    pub(crate) extern_prelude: FxHashMap<Name, ModuleDefId>,
 
+    edition: Edition,
     diagnostics: Vec<DefDiagnostic>,
 }
 
@@ -229,22 +229,6 @@ impl CrateDefMap {
         Arc::new(def_map)
     }
 
-    pub fn krate(&self) -> CrateId {
-        self.krate
-    }
-
-    pub fn root(&self) -> LocalModuleId {
-        self.root
-    }
-
-    pub fn prelude(&self) -> Option<ModuleId> {
-        self.prelude
-    }
-
-    pub fn extern_prelude(&self) -> &FxHashMap<Name, ModuleDefId> {
-        &self.extern_prelude
-    }
-
     pub fn add_diagnostics(
         &self,
         db: &impl DefDatabase,
@@ -254,10 +238,6 @@ impl CrateDefMap {
         self.diagnostics.iter().for_each(|it| it.add_to(db, module, sink))
     }
 
-    pub fn modules(&self) -> impl Iterator<Item = LocalModuleId> + '_ {
-        self.modules.iter().map(|(id, _data)| id)
-    }
-
     pub fn modules_for_file(&self, file_id: FileId) -> impl Iterator<Item = LocalModuleId> + '_ {
         self.modules
             .iter()

crates/ra_hir_def/src/nameres/collector.rs

@@ -1,4 +1,7 @@
-//! FIXME: write short doc here
+//! The core of the module-level name resolution algorithm.
+//!
+//! `DefCollector::collect` contains the fixed-point iteration loop which
+//! resolves imports and expands macros.
 
 use hir_expand::{
     builtin_macro::find_builtin_macro,

crates/ra_hir_def/src/nameres/raw.rs

@@ -22,7 +22,8 @@ use ra_syntax::{
 use test_utils::tested_by;
 
 use crate::{
-    attr::Attrs, db::DefDatabase, path::Path, FileAstId, HirFileId, LocalImportId, Source,
+    attr::Attrs, db::DefDatabase, path::Path, trace::Trace, FileAstId, HirFileId, LocalImportId,
+    Source,
 };
 
 /// `RawItems` is a set of top-level items in a file (except for impls).
@@ -48,10 +49,6 @@ pub struct ImportSourceMap {
 type ImportSourcePtr = Either<AstPtr<ast::UseTree>, AstPtr<ast::ExternCrateItem>>;
 
 impl ImportSourceMap {
-    fn insert(&mut self, import: LocalImportId, ptr: ImportSourcePtr) {
-        self.map.insert(import, ptr)
-    }
-
     pub fn get(&self, import: LocalImportId) -> ImportSourcePtr {
         self.map[import].clone()
     }
@@ -72,7 +69,7 @@ impl RawItems {
         let mut collector = RawItemsCollector {
             raw_items: RawItems::default(),
             source_ast_id_map: db.ast_id_map(file_id),
-            source_map: ImportSourceMap::default(),
+            imports: Trace::new(),
             file_id,
             hygiene: Hygiene::new(db, file_id),
         };
@@ -83,7 +80,11 @@ impl RawItems {
                 collector.process_module(None, item_list);
             }
         }
-        (Arc::new(collector.raw_items), Arc::new(collector.source_map))
+        let mut raw_items = collector.raw_items;
+        let (arena, map) = collector.imports.into_arena_and_map();
+        raw_items.imports = arena;
+        let source_map = ImportSourceMap { map };
+        (Arc::new(raw_items), Arc::new(source_map))
     }
 
     pub(super) fn items(&self) -> &[RawItem] {
@@ -207,8 +208,8 @@ pub(super) struct ImplData {
 
 struct RawItemsCollector {
     raw_items: RawItems,
+    imports: Trace<LocalImportId, ImportData, ImportSourcePtr>,
     source_ast_id_map: Arc<AstIdMap>,
-    source_map: ImportSourceMap,
     file_id: HirFileId,
     hygiene: Hygiene,
 }
@@ -392,8 +393,7 @@ impl RawItemsCollector {
         data: ImportData,
         source: ImportSourcePtr,
     ) {
-        let import = self.raw_items.imports.alloc(data);
-        self.source_map.insert(import, source);
+        let import = self.imports.alloc(|| source, || data);
         self.push_item(current_module, attrs, RawItemKind::Import(import))
     }
 

crates/ra_hir_def/src/nameres/tests.rs

@@ -25,7 +25,7 @@ fn compute_crate_def_map(fixture: &str) -> Arc<CrateDefMap> {
 
 fn render_crate_def_map(map: &CrateDefMap) -> String {
     let mut buf = String::new();
-    go(&mut buf, map, "\ncrate", map.root());
+    go(&mut buf, map, "\ncrate", map.root);
     return buf.trim().to_string();
 
     fn go(buf: &mut String, map: &CrateDefMap, path: &str, module: LocalModuleId) {

crates/ra_hir_def/src/resolver.rs

@@ -321,7 +321,7 @@ impl Resolver {
         let mut traits = FxHashSet::default();
         for scope in &self.scopes {
             if let Scope::ModuleScope(m) = scope {
-                if let Some(prelude) = m.crate_def_map.prelude() {
+                if let Some(prelude) = m.crate_def_map.prelude {
                     let prelude_def_map = db.crate_def_map(prelude.krate);
                     traits.extend(prelude_def_map[prelude.module_id].scope.traits());
                 }
@@ -340,7 +340,7 @@ impl Resolver {
     }
 
     pub fn krate(&self) -> Option<CrateId> {
-        self.module().map(|t| t.0.krate())
+        self.module().map(|t| t.0.krate)
     }
 
     pub fn where_predicates_in_scope<'a>(
@@ -395,10 +395,10 @@ impl Scope {
                 m.crate_def_map[m.module_id].scope.legacy_macros().for_each(|(name, macro_)| {
                     f(name.clone(), ScopeDef::PerNs(PerNs::macros(macro_)));
                 });
-                m.crate_def_map.extern_prelude().iter().for_each(|(name, &def)| {
+                m.crate_def_map.extern_prelude.iter().for_each(|(name, &def)| {
                     f(name.clone(), ScopeDef::PerNs(PerNs::types(def.into())));
                 });
-                if let Some(prelude) = m.crate_def_map.prelude() {
+                if let Some(prelude) = m.crate_def_map.prelude {
                     let prelude_def_map = db.crate_def_map(prelude.krate);
                     prelude_def_map[prelude.module_id].scope.entries().for_each(|(name, res)| {
                         f(name.clone(), ScopeDef::PerNs(res.def));

crates/ra_hir_def/src/trace.rs

@@ -12,38 +12,48 @@
 use ra_arena::{map::ArenaMap, Arena, ArenaId, RawId};
 
 pub(crate) struct Trace<ID: ArenaId, T, V> {
-    for_arena: bool,
-    arena: Arena<ID, T>,
-    map: ArenaMap<ID, V>,
+    arena: Option<Arena<ID, T>>,
+    map: Option<ArenaMap<ID, V>>,
     len: u32,
 }
 
-impl<ID: ra_arena::ArenaId, T, V> Trace<ID, T, V> {
+impl<ID: ra_arena::ArenaId + Copy, T, V> Trace<ID, T, V> {
+    pub(crate) fn new() -> Trace<ID, T, V> {
+        Trace { arena: Some(Arena::default()), map: Some(ArenaMap::default()), len: 0 }
+    }
+
     pub(crate) fn new_for_arena() -> Trace<ID, T, V> {
-        Trace { for_arena: true, arena: Arena::default(), map: ArenaMap::default(), len: 0 }
+        Trace { arena: Some(Arena::default()), map: None, len: 0 }
     }
 
     pub(crate) fn new_for_map() -> Trace<ID, T, V> {
-        Trace { for_arena: false, arena: Arena::default(), map: ArenaMap::default(), len: 0 }
+        Trace { arena: None, map: Some(ArenaMap::default()), len: 0 }
     }
 
-    pub(crate) fn alloc(&mut self, value: impl Fn() -> V, data: impl Fn() -> T) {
-        if self.for_arena {
-            self.arena.alloc(data());
+    pub(crate) fn alloc(&mut self, value: impl FnOnce() -> V, data: impl FnOnce() -> T) -> ID {
+        let id = if let Some(arena) = &mut self.arena {
+            arena.alloc(data())
         } else {
             let id = ID::from_raw(RawId::from(self.len));
             self.len += 1;
-            self.map.insert(id, value());
+            id
+        };
+
+        if let Some(map) = &mut self.map {
+            map.insert(id, value());
         }
+        id
     }
 
-    pub(crate) fn into_arena(self) -> Arena<ID, T> {
-        assert!(self.for_arena);
-        self.arena
+    pub(crate) fn into_arena(mut self) -> Arena<ID, T> {
+        self.arena.take().unwrap()
     }
 
-    pub(crate) fn into_map(self) -> ArenaMap<ID, V> {
-        assert!(!self.for_arena);
-        self.map
+    pub(crate) fn into_map(mut self) -> ArenaMap<ID, V> {
+        self.map.take().unwrap()
+    }
+
+    pub(crate) fn into_arena_and_map(mut self) -> (Arena<ID, T>, ArenaMap<ID, V>) {
+        (self.arena.take().unwrap(), self.map.take().unwrap())
     }
 }