Make Module impl methods crate-private, update some comments

3 files changed, 23 insertions, 8 deletions

diff --git a/crates/ra_hir/src/code_model_impl/module.rs b/crates/ra_hir/src/code_model_impl/module.rs
index 2014f1090..73c212de8 100644
--- a/crates/ra_hir/src/code_model_impl/module.rs
+++ b/crates/ra_hir/src/code_model_impl/module.rs
@@ -95,7 +95,7 @@ impl Module {
     }
 
     /// Finds a child module with the specified name.
-    pub fn child_impl(&self, db: &impl HirDatabase, name: &Name) -> Option<Module> {
+    pub(crate) fn child_impl(&self, db: &impl HirDatabase, name: &Name) -> Option<Module> {
         let loc = self.def_id.loc(db);
         let module_tree = db.module_tree(loc.source_root_id);
         let child_id = loc.module_id.child(&module_tree, name)?;
@@ -103,7 +103,7 @@ impl Module {
     }
 
     /// Iterates over all child modules.
-    pub fn children_impl(&self, db: &impl HirDatabase) -> impl Iterator<Item = Module> {
+    pub(crate) fn children_impl(&self, db: &impl HirDatabase) -> impl Iterator<Item = Module> {
         // FIXME this should be implementable without collecting into a vec, but
         // it's kind of hard since the iterator needs to keep a reference to the
         // module tree.
@@ -117,7 +117,7 @@ impl Module {
         children.into_iter()
     }
 
-    pub fn parent_impl(&self, db: &impl HirDatabase) -> Option<Module> {
+    pub(crate) fn parent_impl(&self, db: &impl HirDatabase) -> Option<Module> {
         let loc = self.def_id.loc(db);
         let module_tree = db.module_tree(loc.source_root_id);
         let parent_id = loc.module_id.parent(&module_tree)?;
@@ -125,13 +125,13 @@ impl Module {
     }
 
     /// Returns a `ModuleScope`: a set of items, visible in this module.
-    pub fn scope_impl(&self, db: &impl HirDatabase) -> ModuleScope {
+    pub(crate) fn scope_impl(&self, db: &impl HirDatabase) -> ModuleScope {
         let loc = self.def_id.loc(db);
         let item_map = db.item_map(loc.source_root_id);
         item_map.per_module[&loc.module_id].clone()
     }
 
-    pub fn resolve_path_impl(&self, db: &impl HirDatabase, path: &Path) -> PerNs<DefId> {
+    pub(crate) fn resolve_path_impl(&self, db: &impl HirDatabase, path: &Path) -> PerNs<DefId> {
         let mut curr_per_ns = PerNs::types(
             match path.kind {
                 PathKind::Crate => self.crate_root(db),
@@ -191,7 +191,10 @@ impl Module {
         curr_per_ns
     }
 
-    pub fn problems_impl(&self, db: &impl HirDatabase) -> Vec<(TreeArc<SyntaxNode>, Problem)> {
+    pub(crate) fn problems_impl(
+        &self,
+        db: &impl HirDatabase,
+    ) -> Vec<(TreeArc<SyntaxNode>, Problem)> {
         let loc = self.def_id.loc(db);
         let module_tree = db.module_tree(loc.source_root_id);
         loc.module_id.problems(&module_tree, db)
diff --git a/crates/ra_hir/src/ids.rs b/crates/ra_hir/src/ids.rs
index 0d8e67547..c5408e277 100644
--- a/crates/ra_hir/src/ids.rs
+++ b/crates/ra_hir/src/ids.rs
@@ -151,6 +151,15 @@ pub(crate) enum DefKind {
     Type,
     Item,
 
+    /// The constructor of a struct. E.g. if we have `struct Foo(usize)`, the
+    /// name `Foo` needs to resolve to different types depending on whether we
+    /// are in the types or values namespace: As a type, `Foo` of course refers
+    /// to the struct `Foo`; as a value, `Foo` is a callable type with signature
+    /// `(usize) -> Foo`. The cleanest approach to handle this seems to be to
+    /// have different defs in the two namespaces.
+    ///
+    /// rustc does the same; note that it even creates a struct constructor if
+    /// the struct isn't a tuple struct (see `CtorKind::Fictive` in rustc).
     StructCtor,
 }
 
diff --git a/crates/ra_hir/src/ty.rs b/crates/ra_hir/src/ty.rs
index b9a48929d..8e93a4457 100644
--- a/crates/ra_hir/src/ty.rs
+++ b/crates/ra_hir/src/ty.rs
@@ -242,7 +242,8 @@ pub enum Ty {
     // Opaque(DefId, Substs),
     /// A type parameter; for example, `T` in `fn f<T>(x: T) {}
     Param {
-        /// The index of the parameter.
+        /// The index of the parameter (starting with parameters from the
+        /// surrounding impl, then the current function).
         idx: u32,
         /// The name of the parameter, for displaying.
         name: Name,
@@ -440,7 +441,9 @@ impl Ty {
                 if (idx as usize) < substs.0.len() {
                     substs.0[idx as usize].clone()
                 } else {
-                    // TODO it's yet unclear to me whether we need to shift the indices here
+                    // TODO: does this indicate a bug? i.e. should we always
+                    // have substs for all type params? (they might contain the
+                    // params themselves again...)
                     Ty::Param { idx, name }
                 }
             }