14 files changed, 109 insertions, 113 deletions

diff --git a/crates/ra_hir_def/src/adt.rs b/crates/ra_hir_def/src/adt.rs
index 20e9a1eb5..c9f30923e 100644
--- a/crates/ra_hir_def/src/adt.rs
+++ b/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())),
             },
-        )
+        );
     }
 }
 
@@ -109,10 +109,18 @@ impl VariantData {
         }
     }
 
-    pub fn fields(&self) -> Option<&Arena<LocalStructFieldId, StructFieldData>> {
+    pub fn fields(&self) -> &Arena<LocalStructFieldId, StructFieldData> {
+        const EMPTY: &Arena<LocalStructFieldId, StructFieldData> = &Arena::new();
         match &self {
-            VariantData::Record(fields) | VariantData::Tuple(fields) => Some(fields),
-            _ => None,
+            VariantData::Record(fields) | VariantData::Tuple(fields) => fields,
+            _ => EMPTY,
+        }
+    }
+
+    pub fn is_unit(&self) -> bool {
+        match self {
+            VariantData::Unit => true,
+            _ => false,
         }
     }
 }
@@ -160,7 +168,7 @@ fn lower_struct(
                         name: Name::new_tuple_field(i),
                         type_ref: TypeRef::from_ast_opt(fd.type_ref()),
                     },
-                )
+                );
             }
             StructKind::Tuple
         }
@@ -172,7 +180,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
         }
diff --git a/crates/ra_hir_def/src/body.rs b/crates/ra_hir_def/src/body.rs
index c06997cf1..d77ccb272 100644
--- a/crates/ra_hir_def/src/body.rs
+++ b/crates/ra_hir_def/src/body.rs
@@ -1,4 +1,5 @@
-//! FIXME: write short doc here
+//! Defines `Body`: a lowered representation of bodies of functions, statics and
+//! consts.
 mod lower;
 pub mod scope;
 
@@ -20,7 +21,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 +29,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 +102,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 +183,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 {
diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs
index f4640dfa4..331736cb2 100644
--- a/crates/ra_hir_def/src/body/lower.rs
+++ b/crates/ra_hir_def/src/body/lower.rs
@@ -1,4 +1,5 @@
-//! FIXME: write short doc here
+//! Transforms `ast::Expr` into an equivalent `hir_def::expr::Expr`
+//! representation.
 
 use hir_expand::{
     either::Either,
diff --git a/crates/ra_hir_def/src/body/scope.rs b/crates/ra_hir_def/src/body/scope.rs
index 20d707bc4..625aa39dd 100644
--- a/crates/ra_hir_def/src/body/scope.rs
+++ b/crates/ra_hir_def/src/body/scope.rs
@@ -1,4 +1,4 @@
-//! FIXME: write short doc here
+//! Name resolution for expressions.
 use std::sync::Arc;
 
 use hir_expand::name::Name;
@@ -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
     }
 
diff --git a/crates/ra_hir_def/src/lang_item.rs b/crates/ra_hir_def/src/lang_item.rs
index 3b9fb0328..f15c23db9 100644
--- a/crates/ra_hir_def/src/lang_item.rs
+++ b/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)
diff --git a/crates/ra_hir_def/src/lib.rs b/crates/ra_hir_def/src/lib.rs
index f63c3dd64..8e8c2d749 100644
--- a/crates/ra_hir_def/src/lib.rs
+++ b/crates/ra_hir_def/src/lib.rs
@@ -192,12 +192,6 @@ pub struct LocalEnumVariantId(RawId);
 impl_arena_id!(LocalEnumVariantId);
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
-pub enum VariantId {
-    EnumVariantId(EnumVariantId),
-    StructId(StructId),
-}
-
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct StructFieldId {
     pub parent: VariantId,
     pub local_id: LocalStructFieldId,
@@ -437,6 +431,13 @@ impl_froms!(
     ImplId
 );
 
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub enum VariantId {
+    EnumVariantId(EnumVariantId),
+    StructId(StructId),
+}
+impl_froms!(VariantId: EnumVariantId, StructId);
+
 trait Intern {
     type ID;
     fn intern(self, db: &impl db::DefDatabase) -> Self::ID;
@@ -481,6 +482,16 @@ impl HasModule for ConstLoc {
     }
 }
 
+impl HasModule for AdtId {
+    fn module(&self, db: &impl db::DefDatabase) -> ModuleId {
+        match self {
+            AdtId::StructId(it) => it.0.module(db),
+            AdtId::UnionId(it) => it.0.module(db),
+            AdtId::EnumId(it) => it.module(db),
+        }
+    }
+}
+
 impl HasModule for StaticLoc {
     fn module(&self, _db: &impl db::DefDatabase) -> ModuleId {
         self.container
diff --git a/crates/ra_hir_def/src/nameres.rs b/crates/ra_hir_def/src/nameres.rs
index 5919771b0..2359386c2 100644
--- a/crates/ra_hir_def/src/nameres.rs
+++ b/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()
diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs
index df01a20e1..41becf8df 100644
--- a/crates/ra_hir_def/src/nameres/collector.rs
+++ b/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,
diff --git a/crates/ra_hir_def/src/nameres/raw.rs b/crates/ra_hir_def/src/nameres/raw.rs
index 2ec84f2cc..401af031c 100644
--- a/crates/ra_hir_def/src/nameres/raw.rs
+++ b/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))
     }
 
diff --git a/crates/ra_hir_def/src/nameres/tests.rs b/crates/ra_hir_def/src/nameres/tests.rs
index f0b86af7c..f502f1cb3 100644
--- a/crates/ra_hir_def/src/nameres/tests.rs
+++ b/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) {
diff --git a/crates/ra_hir_def/src/path.rs b/crates/ra_hir_def/src/path.rs
index 7b2723d57..0e606fd0e 100644
--- a/crates/ra_hir_def/src/path.rs
+++ b/crates/ra_hir_def/src/path.rs
@@ -195,7 +195,7 @@ impl Path {
     }
 
     /// Converts an `ast::NameRef` into a single-identifier `Path`.
-    pub fn from_name_ref(name_ref: &ast::NameRef) -> Path {
+    pub(crate) fn from_name_ref(name_ref: &ast::NameRef) -> Path {
         name_ref.as_name().into()
     }
 
diff --git a/crates/ra_hir_def/src/per_ns.rs b/crates/ra_hir_def/src/per_ns.rs
index 06ef6c9fc..00e866bf9 100644
--- a/crates/ra_hir_def/src/per_ns.rs
+++ b/crates/ra_hir_def/src/per_ns.rs
@@ -1,4 +1,7 @@
-//! FIXME: write short doc here
+//! In rust, it is possible to have a value, a type and a macro with the same
+//! name without conflicts.
+//!
+//! `PerNs` (per namespace) captures this.
 
 use hir_expand::MacroDefId;
 
diff --git a/crates/ra_hir_def/src/resolver.rs b/crates/ra_hir_def/src/resolver.rs
index 7182b8a4d..95b3c926d 100644
--- a/crates/ra_hir_def/src/resolver.rs
+++ b/crates/ra_hir_def/src/resolver.rs
@@ -18,8 +18,8 @@ use crate::{
     path::{Path, PathKind},
     per_ns::PerNs,
     AdtId, AstItemDef, ConstId, ContainerId, DefWithBodyId, EnumId, EnumVariantId, FunctionId,
-    GenericDefId, ImplId, LocalModuleId, Lookup, ModuleDefId, ModuleId, StaticId, StructId,
-    TraitId, TypeAliasId,
+    GenericDefId, HasModule, ImplId, LocalModuleId, Lookup, ModuleDefId, ModuleId, StaticId,
+    StructId, TraitId, TypeAliasId,
 };
 
 #[derive(Debug, Clone, Default)]
@@ -29,20 +29,20 @@ pub struct Resolver {
 
 // FIXME how to store these best
 #[derive(Debug, Clone)]
-pub(crate) struct ModuleItemMap {
+struct ModuleItemMap {
     crate_def_map: Arc<CrateDefMap>,
     module_id: LocalModuleId,
 }
 
 #[derive(Debug, Clone)]
-pub(crate) struct ExprScope {
+struct ExprScope {
     owner: DefWithBodyId,
     expr_scopes: Arc<ExprScopes>,
     scope_id: ScopeId,
 }
 
 #[derive(Debug, Clone)]
-pub(crate) enum Scope {
+enum Scope {
     /// All the items and imported names of a module
     ModuleScope(ModuleItemMap),
     /// Brings the generic parameters of an item into scope
@@ -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));
@@ -503,13 +503,7 @@ impl HasResolver for TraitId {
 impl<T: Into<AdtId>> HasResolver for T {
     fn resolver(self, db: &impl DefDatabase) -> Resolver {
         let def = self.into();
-        let module = match def {
-            AdtId::StructId(it) => it.0.module(db),
-            AdtId::UnionId(it) => it.0.module(db),
-            AdtId::EnumId(it) => it.module(db),
-        };
-
-        module
+        def.module(db)
             .resolver(db)
             .push_generic_params_scope(db, def.into())
             .push_scope(Scope::AdtScope(def))
diff --git a/crates/ra_hir_def/src/trace.rs b/crates/ra_hir_def/src/trace.rs
index fc26f5a48..2bcd707bc 100644
--- a/crates/ra_hir_def/src/trace.rs
+++ b/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(mut self) -> Arena<ID, T> {
+        self.arena.take().unwrap()
     }
 
-    pub(crate) fn into_arena(self) -> Arena<ID, T> {
-        assert!(self.for_arena);
-        self.arena
+    pub(crate) fn into_map(mut self) -> ArenaMap<ID, V> {
+        self.map.take().unwrap()
     }
 
-    pub(crate) fn into_map(self) -> ArenaMap<ID, V> {
-        assert!(!self.for_arena);
-        self.map
+    pub(crate) fn into_arena_and_map(mut self) -> (Arena<ID, T>, ArenaMap<ID, V>) {
+        (self.arena.take().unwrap(), self.map.take().unwrap())
     }
 }