Refactor away get_module_if_available and get_module and reformat one-liners

Author: jseyfried

src/librustc_resolve/build_reduced_graph.rs

@@ -86,10 +86,9 @@ impl<'a, 'b:'a, 'tcx:'b> DerefMut for GraphBuilder<'a, 'b, 'tcx> {
 impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
     /// Constructs the reduced graph for the entire crate.
     fn build_reduced_graph(self, krate: &hir::Crate) {
-        let parent = self.graph_root.get_module();
         let mut visitor = BuildReducedGraphVisitor {
+            parent: self.graph_root.clone(),
             builder: self,
-            parent: parent,
         };
         visit::walk_crate(&mut visitor, krate);
     }
@@ -318,10 +317,9 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                     };
                     self.external_exports.insert(def_id);
                     let parent_link = ModuleParentLink(Rc::downgrade(parent), name);
-                    let external_module = Rc::new(Module::new(parent_link,
-                                                              Some(DefMod(def_id)),
-                                                              false,
-                                                              true));
+                    let def = DefMod(def_id);
+                    let external_module = Module::new(parent_link, Some(def), false, true);
+
                     debug!("(build reduced graph for item) found extern `{}`",
                            module_to_string(&*external_module));
                     self.check_for_conflicts_between_external_crates(&**parent, name, sp);
@@ -338,9 +336,9 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
 
                 let parent_link = self.get_parent_link(parent, name);
                 let def = DefMod(self.ast_map.local_def_id(item.id));
-                name_bindings.define_module(parent_link, Some(def), false, is_public, sp);
-
-                name_bindings.get_module()
+                let module = Module::new(parent_link, Some(def), false, is_public);
+                name_bindings.define_module(module.clone(), sp);
+                module
             }
 
             ItemForeignMod(..) => parent.clone(),
@@ -377,7 +375,8 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
 
                 let parent_link = self.get_parent_link(parent, name);
                 let def = DefTy(self.ast_map.local_def_id(item.id), false);
-                name_bindings.define_module(parent_link, Some(def), false, is_public, sp);
+                let module = Module::new(parent_link, Some(def), false, is_public);
+                name_bindings.define_module(module, sp);
                 parent.clone()
             }
 
@@ -389,9 +388,8 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
 
                 let parent_link = self.get_parent_link(parent, name);
                 let def = DefTy(self.ast_map.local_def_id(item.id), true);
-                name_bindings.define_module(parent_link, Some(def), false, is_public, sp);
-
-                let module = name_bindings.get_module();
+                let module = Module::new(parent_link, Some(def), false, is_public);
+                name_bindings.define_module(module.clone(), sp);
 
                 for variant in &(*enum_definition).variants {
                     let item_def_id = self.ast_map.local_def_id(item.id);
@@ -454,8 +452,8 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                 // Add all the items within to a new module.
                 let parent_link = self.get_parent_link(parent, name);
                 let def = DefTrait(def_id);
-                name_bindings.define_module(parent_link, Some(def), false, is_public, sp);
-                let module_parent = name_bindings.get_module();
+                let module_parent = Module::new(parent_link, Some(def), false, is_public);
+                name_bindings.define_module(module_parent.clone(), sp);
 
                 // Add the names of all the items to the trait info.
                 for trait_item in items {
@@ -555,10 +553,8 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                     {}",
                    block_id);
 
-            let new_module = Rc::new(Module::new(BlockParentLink(Rc::downgrade(parent), block_id),
-                                                 None,
-                                                 false,
-                                                 false));
+            let parent_link = BlockParentLink(Rc::downgrade(parent), block_id);
+            let new_module = Module::new(parent_link, None, false, false);
             parent.anonymous_children.borrow_mut().insert(block_id, new_module.clone());
             new_module
         } else {
@@ -604,12 +600,8 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                            final_ident,
                            is_public);
                     let parent_link = self.get_parent_link(new_parent, name);
-
-                    child_name_bindings.define_module(parent_link,
-                                                      Some(def),
-                                                      true,
-                                                      is_public,
-                                                      DUMMY_SP);
+                    let module = Module::new(parent_link, Some(def), true, is_public);
+                    child_name_bindings.define_module(module, DUMMY_SP);
                 }
             }
             _ => {}
@@ -681,11 +673,8 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
 
                 // Define a module if necessary.
                 let parent_link = self.get_parent_link(new_parent, name);
-                child_name_bindings.define_module(parent_link,
-                                                  Some(def),
-                                                  true,
-                                                  is_public,
-                                                  DUMMY_SP)
+                let module = Module::new(parent_link, Some(def), true, is_public);
+                child_name_bindings.define_module(module, DUMMY_SP);
             }
             DefTy(..) | DefAssociatedTy(..) => {
                 debug!("(building reduced graph for external crate) building type {}",

src/librustc_resolve/lib.rs

@@ -813,8 +813,8 @@ impl Module {
            def: Option<Def>,
            external: bool,
            is_public: bool)
-           -> Module {
-        Module {
+           -> Rc<Module> {
+        Rc::new(Module {
             parent_link: parent_link,
             def: Cell::new(def),
             is_public: is_public,
@@ -828,7 +828,7 @@ impl Module {
             pub_glob_count: Cell::new(0),
             resolved_import_count: Cell::new(0),
             populated: Cell::new(!external),
-        }
+        })
     }
 
     fn def_id(&self) -> Option<DefId> {
@@ -971,19 +971,27 @@ impl NameBinding {
         }
     }
 
-    fn and_then<T, F: Fn(&NsDef) -> Option<T>>(&self, f: F) -> Option<T> {
-        self.borrow().as_ref().and_then(f)
+    fn borrow(&self) -> ::std::cell::Ref<Option<NsDef>> {
+        self.0.borrow()
     }
 
-    fn borrow(&self) -> ::std::cell::Ref<Option<NsDef>> { self.0.borrow() }
-
     // Lifted versions of the NsDef methods and fields
-    fn def(&self) -> Option<Def>           { self.and_then(NsDef::def) }
-    fn module(&self) -> Option<Rc<Module>> { self.and_then(NsDef::module) }
-    fn span(&self) -> Option<Span>         { self.and_then(|def| def.span) }
-    fn modifiers(&self) -> Option<DefModifiers> { self.and_then(|def| Some(def.modifiers)) }
+    fn def(&self) -> Option<Def> {
+        self.borrow().as_ref().and_then(NsDef::def)
+    }
+    fn module(&self) -> Option<Rc<Module>> {
+        self.borrow().as_ref().and_then(NsDef::module)
+    }
+    fn span(&self) -> Option<Span> {
+        self.borrow().as_ref().and_then(|def| def.span)
+    }
+    fn modifiers(&self) -> Option<DefModifiers> {
+        self.borrow().as_ref().and_then(|def| Some(def.modifiers))
+    }
 
-    fn defined(&self) -> bool { self.borrow().is_some() }
+    fn defined(&self) -> bool {
+        self.borrow().is_some()
+    }
 
     fn defined_with(&self, modifiers: DefModifiers) -> bool {
         self.modifiers().map(|m| m.contains(modifiers)).unwrap_or(false)
@@ -1030,14 +1038,8 @@ impl NameBindings {
     }
 
     /// Creates a new module in this set of name bindings.
-    fn define_module(&self,
-                     parent_link: ParentLink,
-                     def: Option<Def>,
-                     external: bool,
-                     is_public: bool,
-                     sp: Span) {
-        let module = Module::new(parent_link, def, external, is_public);
-        self.type_ns.set(NsDef::create_from_module(Rc::new(module), Some(sp)));
+    fn define_module(&self, module: Rc<Module>, sp: Span) {
+        self.type_ns.set(NsDef::create_from_module(module, Some(sp)));
     }
 
     /// Records a type definition.
@@ -1051,20 +1053,6 @@ impl NameBindings {
         debug!("defining value for def {:?} with modifiers {:?}", def, modifiers);
         self.value_ns.set(NsDef::create_from_def(def, modifiers, Some(sp)));
     }
-
-    /// Returns the module node if applicable.
-    fn get_module_if_available(&self) -> Option<Rc<Module>> { self.type_ns.module() }
-
-    /// Returns the module node. Panics if this node does not have a module
-    /// definition.
-    fn get_module(&self) -> Rc<Module> {
-        match self.get_module_if_available() {
-            None => {
-                panic!("get_module called on a node with no module definition!")
-            }
-            Some(module_def) => module_def,
-        }
-    }
 }
 
 /// Interns the names of the primitive types.
@@ -1106,7 +1094,7 @@ pub struct Resolver<'a, 'tcx: 'a> {
 
     ast_map: &'a hir_map::Map<'tcx>,
 
-    graph_root: NameBindings,
+    graph_root: Rc<Module>,
 
     trait_item_map: FnvHashMap<(Name, DefId), DefId>,
 
@@ -1173,19 +1161,10 @@ enum FallbackChecks {
 impl<'a, 'tcx> Resolver<'a, 'tcx> {
     fn new(session: &'a Session,
            ast_map: &'a hir_map::Map<'tcx>,
-           crate_span: Span,
            make_glob_map: MakeGlobMap)
            -> Resolver<'a, 'tcx> {
-        let graph_root = NameBindings::new();
-
         let root_def_id = ast_map.local_def_id(CRATE_NODE_ID);
-        graph_root.define_module(NoParentLink,
-                                 Some(DefMod(root_def_id)),
-                                 false,
-                                 true,
-                                 crate_span);
-
-        let current_module = graph_root.get_module();
+        let graph_root = Module::new(NoParentLink, Some(DefMod(root_def_id)), false, true);
 
         Resolver {
             session: session,
@@ -1194,14 +1173,14 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
 
             // The outermost module has def ID 0; this is not reflected in the
             // AST.
-            graph_root: graph_root,
+            graph_root: graph_root.clone(),
 
             trait_item_map: FnvHashMap(),
             structs: FnvHashMap(),
 
             unresolved_imports: 0,
 
-            current_module: current_module,
+            current_module: graph_root,
             value_ribs: Vec::new(),
             type_ribs: Vec::new(),
             label_ribs: Vec::new(),
@@ -1441,7 +1420,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                     DontUseLexicalScope => {
                         // This is a crate-relative path. We will start the
                         // resolution process at index zero.
-                        search_module = self.graph_root.get_module();
+                        search_module = self.graph_root.clone();
                         start_index = 0;
                         last_private = LastMod(AllPublic);
                     }
@@ -1792,7 +1771,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         build_reduced_graph::populate_module_if_necessary(self, &module_);
 
         for (_, child_node) in module_.children.borrow().iter() {
-            match child_node.get_module_if_available() {
+            match child_node.type_ns.module() {
                 None => {
                     // Continue.
                 }
@@ -1845,7 +1824,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                                module_to_string(&*orig_module));
                     }
                     Some(name_bindings) => {
-                        match (*name_bindings).get_module_if_available() {
+                        match name_bindings.type_ns.module() {
                             None => {
                                 debug!("!!! (with scope) didn't find module for `{}` in `{}`",
                                        name,
@@ -3115,7 +3094,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                                   .map(|ps| ps.identifier.name)
                                   .collect::<Vec<_>>();
 
-        let root_module = self.graph_root.get_module();
+        let root_module = self.graph_root.clone();
 
         let containing_module;
         let last_private;
@@ -3278,7 +3257,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                     Some(_) => None,
                     None => {
                         match this.current_module.children.borrow().get(last_name) {
-                            Some(child) => child.get_module_if_available(),
+                            Some(child) => child.type_ns.module(),
                             None => None,
                         }
                     }
@@ -3883,7 +3862,7 @@ pub fn create_resolver<'a, 'tcx>(session: &'a Session,
                                  make_glob_map: MakeGlobMap,
                                  callback: Option<Box<Fn(hir_map::Node, &mut bool) -> bool>>)
                                  -> Resolver<'a, 'tcx> {
-    let mut resolver = Resolver::new(session, ast_map, krate.span, make_glob_map);
+    let mut resolver = Resolver::new(session, ast_map, make_glob_map);
 
     resolver.callback = callback;
 

src/librustc_resolve/record_exports.rs

@@ -79,7 +79,7 @@ impl<'a, 'b, 'tcx> ExportRecorder<'a, 'b, 'tcx> {
         build_reduced_graph::populate_module_if_necessary(self.resolver, &module_);
 
         for (_, child_name_bindings) in module_.children.borrow().iter() {
-            match child_name_bindings.get_module_if_available() {
+            match child_name_bindings.type_ns.module() {
                 None => {
                     // Nothing to do.
                 }
@@ -149,6 +149,6 @@ impl<'a, 'b, 'tcx> ExportRecorder<'a, 'b, 'tcx> {
 
 pub fn record(resolver: &mut Resolver) {
     let mut recorder = ExportRecorder { resolver: resolver };
-    let root_module = recorder.graph_root.get_module();
+    let root_module = recorder.graph_root.clone();
     recorder.record_exports_for_module_subtree(root_module);
 }

src/librustc_resolve/resolve_imports.rs

@@ -209,7 +209,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                    i,
                    self.resolver.unresolved_imports);
 
-            let module_root = self.resolver.graph_root.get_module();
+            let module_root = self.resolver.graph_root.clone();
             let errors = self.resolve_imports_for_module_subtree(module_root.clone());
 
             if self.resolver.unresolved_imports == 0 {
@@ -254,7 +254,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
 
         build_reduced_graph::populate_module_if_necessary(self.resolver, &module_);
         for (_, child_node) in module_.children.borrow().iter() {
-            match child_node.get_module_if_available() {
+            match child_node.type_ns.module() {
                 None => {
                     // Nothing to do.
                 }
@@ -337,7 +337,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
         // First, resolve the module path for the directive, if necessary.
         let container = if module_path.is_empty() {
             // Use the crate root.
-            Some((self.resolver.graph_root.get_module(), LastMod(AllPublic)))
+            Some((self.resolver.graph_root.clone(), LastMod(AllPublic)))
         } else {
             match self.resolver.resolve_module_path(module_.clone(),
                                                     &module_path[..],