From 06931988c009b7467b6545cad2d6fb3597a29adf Mon Sep 17 00:00:00 2001
From: Camelid <camelidcamel@gmail.com>
Date: Sun, 27 Dec 2020 16:53:57 -0800
Subject: [PATCH 1/2] Document `ModuleData`

* Convert comments on fields to doc comments so they're visible in API
  docs
* Add new documentation
* Get rid of "normal module" terminology
---
 compiler/rustc_resolve/src/lib.rs | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs
index 0de732b2cf97e..ad584a6969738 100644
--- a/compiler/rustc_resolve/src/lib.rs
+++ b/compiler/rustc_resolve/src/lib.rs
@@ -456,28 +456,36 @@ struct BindingKey {
 type Resolutions<'a> = RefCell<FxIndexMap<BindingKey, &'a RefCell<NameResolution<'a>>>>;
 
 /// One node in the tree of modules.
+///
+/// Note that "module" is a loose term here; it does not necessarily mean
+/// a `mod` that you declare in Rust code. It may also be, e.g., a trait
+/// or an enum. See [`ModuleKind`] (accessible through [`ModuleData::kind`]
+/// for all of the kinds of "modules" that resolve deals with.
 pub struct ModuleData<'a> {
+    /// The direct parent module (it may not be a `mod`, however).
     parent: Option<Module<'a>>,
+    /// What kind of module this is, because this may not be a `mod`.
     kind: ModuleKind,
 
-    // The def id of the closest normal module (`mod`) ancestor (including this module).
+    /// The [`DefId`] of the closest `mod` item ancestor (which may be this module), including crate root.
     normal_ancestor_id: DefId,
 
-    // Mapping between names and their (possibly in-progress) resolutions in this module.
-    // Resolutions in modules from other crates are not populated until accessed.
+    /// Mapping between names and their (possibly in-progress) resolutions in this module.
+    /// Resolutions in modules from other crates are not populated until accessed.
     lazy_resolutions: Resolutions<'a>,
-    // True if this is a module from other crate that needs to be populated on access.
+    /// True if this is a module from other crate that needs to be populated on access.
     populate_on_access: Cell<bool>,
 
-    // Macro invocations that can expand into items in this module.
+    /// Macro invocations that can expand into items in this module.
     unexpanded_invocations: RefCell<FxHashSet<ExpnId>>,
 
+    /// Whether `#[no_implicit_prelude]` is active.
     no_implicit_prelude: bool,
 
     glob_importers: RefCell<Vec<&'a Import<'a>>>,
     globs: RefCell<Vec<&'a Import<'a>>>,
 
-    // Used to memoize the traits in this module for faster searches through all traits in scope.
+    /// Used to memoize the traits in this module for faster searches through all traits in scope.
     traits: RefCell<Option<Box<[(Ident, &'a NameBinding<'a>)]>>>,
 
     /// Span of the module itself. Used for error reporting.

From ff75da89b1fd04532251e1c60fca9cfe306ae26e Mon Sep 17 00:00:00 2001
From: Camelid <camelidcamel@gmail.com>
Date: Sun, 27 Dec 2020 16:54:47 -0800
Subject: [PATCH 2/2] Rename to `nearest_parent_mod`

* Rename `ModuleData.normal_ancestor_id` to `nearest_parent_mod`

`normal_ancestor_id` is a very confusing name if you don't already
understand what it means. Adding docs helps, but using a clearer and
more obvious name is also important.

* Rename `Resolver::nearest_mod_parent` to `nearest_parent_mod`

* Add more docs
---
 .../rustc_resolve/src/build_reduced_graph.rs  | 14 +++----
 compiler/rustc_resolve/src/late.rs            |  4 +-
 compiler/rustc_resolve/src/lib.rs             | 42 +++++++++++--------
 compiler/rustc_resolve/src/macros.rs          |  2 +-
 4 files changed, 35 insertions(+), 27 deletions(-)

diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs
index c5f783e84a91e..de3430d1cd758 100644
--- a/compiler/rustc_resolve/src/build_reduced_graph.rs
+++ b/compiler/rustc_resolve/src/build_reduced_graph.rs
@@ -96,7 +96,7 @@ impl<'a> Resolver<'a> {
 
     /// Walks up the tree of definitions starting at `def_id`,
     /// stopping at the first `DefKind::Mod` encountered
-    fn nearest_mod_parent(&mut self, def_id: DefId) -> Module<'a> {
+    fn nearest_parent_mod(&mut self, def_id: DefId) -> Module<'a> {
         let def_key = self.cstore().def_key(def_id);
 
         let mut parent_id = DefId {
@@ -137,7 +137,7 @@ impl<'a> Resolver<'a> {
                 .get_opt_name()
                 .expect("given a DefId that wasn't a module");
 
-            let parent = Some(self.nearest_mod_parent(def_id));
+            let parent = Some(self.nearest_parent_mod(def_id));
             (name, parent)
         };
 
@@ -179,7 +179,7 @@ impl<'a> Resolver<'a> {
             // so this hopefully won't be a problem.
             //
             // See https://github.com/rust-lang/rust/pull/77984#issuecomment-712445508
-            self.nearest_mod_parent(def_id)
+            self.nearest_parent_mod(def_id)
         }
     }
 
@@ -266,7 +266,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
                 } else {
                     // If it's not in an enum, its visibility is restricted to the `mod` item
                     // that it's defined in.
-                    Ok(ty::Visibility::Restricted(self.parent_scope.module.normal_ancestor_id))
+                    Ok(ty::Visibility::Restricted(self.parent_scope.module.nearest_parent_mod))
                 }
             }
             ast::VisibilityKind::Restricted { ref path, id, .. } => {
@@ -803,7 +803,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
                 let module = self.r.new_module(
                     parent,
                     module_kind,
-                    parent.normal_ancestor_id,
+                    parent.nearest_parent_mod,
                     expansion,
                     item.span,
                 );
@@ -878,7 +878,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
                 let module = self.r.new_module(
                     parent,
                     module_kind,
-                    parent.normal_ancestor_id,
+                    parent.nearest_parent_mod,
                     expansion,
                     item.span,
                 );
@@ -921,7 +921,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
             let module = self.r.new_module(
                 parent,
                 ModuleKind::Block(block.id),
-                parent.normal_ancestor_id,
+                parent.nearest_parent_mod,
                 expansion,
                 block.span,
             );
diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs
index fbe99a3115054..2e738ce8daccd 100644
--- a/compiler/rustc_resolve/src/late.rs
+++ b/compiler/rustc_resolve/src/late.rs
@@ -1775,7 +1775,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
             if this.should_report_errs() {
                 let (err, candidates) = this.smart_resolve_report_errors(path, span, source, res);
 
-                let def_id = this.parent_scope.module.normal_ancestor_id;
+                let def_id = this.parent_scope.module.nearest_parent_mod;
                 let instead = res.is_some();
                 let suggestion =
                     if res.is_none() { this.report_missing_type_error(path) } else { None };
@@ -1843,7 +1843,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
 
             drop(parent_err);
 
-            let def_id = this.parent_scope.module.normal_ancestor_id;
+            let def_id = this.parent_scope.module.nearest_parent_mod;
 
             if this.should_report_errs() {
                 this.r.use_injections.push(UseError {
diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs
index ad584a6969738..dba30f6664007 100644
--- a/compiler/rustc_resolve/src/lib.rs
+++ b/compiler/rustc_resolve/src/lib.rs
@@ -422,7 +422,9 @@ enum ModuleKind {
     ///
     /// This could be:
     ///
-    /// * A normal module ‒ either `mod from_file;` or `mod from_block { }`.
+    /// * A normal module – either `mod from_file;` or `mod from_block { }` –
+    ///   or the crate root (which is conceptually a top-level module).
+    ///   Note that the crate root's [name][Self::name] will be [`kw::Empty`].
     /// * A trait or an enum (it implicitly contains associated types, methods and variant
     ///   constructors).
     Def(DefKind, DefId, Symbol),
@@ -457,18 +459,24 @@ type Resolutions<'a> = RefCell<FxIndexMap<BindingKey, &'a RefCell<NameResolution
 
 /// One node in the tree of modules.
 ///
-/// Note that "module" is a loose term here; it does not necessarily mean
-/// a `mod` that you declare in Rust code. It may also be, e.g., a trait
-/// or an enum. See [`ModuleKind`] (accessible through [`ModuleData::kind`]
-/// for all of the kinds of "modules" that resolve deals with.
+/// Note that a "module" in resolve is broader than a `mod` that you declare in Rust code. It may be one of these:
+///
+/// * `mod`
+/// * crate root (aka, top-level anonymous module)
+/// * `enum`
+/// * `trait`
+/// * curly-braced block with statements
+///
+/// You can use [`ModuleData::kind`] to determine the kind of module this is.
 pub struct ModuleData<'a> {
     /// The direct parent module (it may not be a `mod`, however).
     parent: Option<Module<'a>>,
     /// What kind of module this is, because this may not be a `mod`.
     kind: ModuleKind,
 
-    /// The [`DefId`] of the closest `mod` item ancestor (which may be this module), including crate root.
-    normal_ancestor_id: DefId,
+    /// The [`DefId`] of the nearest `mod` item ancestor (which may be this module).
+    /// This may be the crate root.
+    nearest_parent_mod: DefId,
 
     /// Mapping between names and their (possibly in-progress) resolutions in this module.
     /// Resolutions in modules from other crates are not populated until accessed.
@@ -500,16 +508,16 @@ impl<'a> ModuleData<'a> {
     fn new(
         parent: Option<Module<'a>>,
         kind: ModuleKind,
-        normal_ancestor_id: DefId,
+        nearest_parent_mod: DefId,
         expansion: ExpnId,
         span: Span,
     ) -> Self {
         ModuleData {
             parent,
             kind,
-            normal_ancestor_id,
+            nearest_parent_mod,
             lazy_resolutions: Default::default(),
-            populate_on_access: Cell::new(!normal_ancestor_id.is_local()),
+            populate_on_access: Cell::new(!nearest_parent_mod.is_local()),
             unexpanded_invocations: Default::default(),
             no_implicit_prelude: false,
             glob_importers: RefCell::new(Vec::new()),
@@ -1527,11 +1535,11 @@ impl<'a> Resolver<'a> {
         &self,
         parent: Module<'a>,
         kind: ModuleKind,
-        normal_ancestor_id: DefId,
+        nearest_parent_mod: DefId,
         expn_id: ExpnId,
         span: Span,
     ) -> Module<'a> {
-        let module = ModuleData::new(Some(parent), kind, normal_ancestor_id, expn_id, span);
+        let module = ModuleData::new(Some(parent), kind, nearest_parent_mod, expn_id, span);
         self.arenas.alloc_module(module)
     }
 
@@ -2124,7 +2132,7 @@ impl<'a> Resolver<'a> {
                 return self.graph_root;
             }
         };
-        let module = self.get_module(DefId { index: CRATE_DEF_INDEX, ..module.normal_ancestor_id });
+        let module = self.get_module(DefId { index: CRATE_DEF_INDEX, ..module.nearest_parent_mod });
         debug!(
             "resolve_crate_root({:?}): got module {:?} ({:?}) (ident.span = {:?})",
             ident,
@@ -2136,10 +2144,10 @@ impl<'a> Resolver<'a> {
     }
 
     fn resolve_self(&mut self, ctxt: &mut SyntaxContext, module: Module<'a>) -> Module<'a> {
-        let mut module = self.get_module(module.normal_ancestor_id);
+        let mut module = self.get_module(module.nearest_parent_mod);
         while module.span.ctxt().normalize_to_macros_2_0() != *ctxt {
             let parent = module.parent.unwrap_or_else(|| self.macro_def_scope(ctxt.remove_mark()));
-            module = self.get_module(parent.normal_ancestor_id);
+            module = self.get_module(parent.nearest_parent_mod);
         }
         module
     }
@@ -2801,7 +2809,7 @@ impl<'a> Resolver<'a> {
     }
 
     fn is_accessible_from(&self, vis: ty::Visibility, module: Module<'a>) -> bool {
-        vis.is_accessible_from(module.normal_ancestor_id, self)
+        vis.is_accessible_from(module.nearest_parent_mod, self)
     }
 
     fn set_binding_parent_module(&mut self, binding: &'a NameBinding<'a>, module: Module<'a>) {
@@ -2825,7 +2833,7 @@ impl<'a> Resolver<'a> {
             self.binding_parent_modules.get(&PtrKey(modularized)),
         ) {
             (Some(macro_rules), Some(modularized)) => {
-                macro_rules.normal_ancestor_id == modularized.normal_ancestor_id
+                macro_rules.nearest_parent_mod == modularized.nearest_parent_mod
                     && modularized.is_ancestor_of(macro_rules)
             }
             _ => false,
diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs
index 5ad7c83ca36af..e6360cccf3b96 100644
--- a/compiler/rustc_resolve/src/macros.rs
+++ b/compiler/rustc_resolve/src/macros.rs
@@ -328,7 +328,7 @@ impl<'a> ResolverExpand for Resolver<'a> {
             if after_derive {
                 self.session.span_err(span, "macro attributes must be placed before `#[derive]`");
             }
-            let normal_module_def_id = self.macro_def_scope(invoc_id).normal_ancestor_id;
+            let normal_module_def_id = self.macro_def_scope(invoc_id).nearest_parent_mod;
             self.definitions.add_parent_module_of_macro_def(invoc_id, normal_module_def_id);
         }