From 6a6e1dba55388bdf252e79eec2f084a45e0e862f Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Wed, 18 Nov 2015 01:22:32 +0000 Subject: [PATCH] Refactor away get_module_if_available and get_module and reformat one-liners --- src/librustc_resolve/build_reduced_graph.rs | 49 +++++------- src/librustc_resolve/lib.rs | 85 ++++++++------------- src/librustc_resolve/record_exports.rs | 4 +- src/librustc_resolve/resolve_imports.rs | 6 +- 4 files changed, 56 insertions(+), 88 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index b519a729918..c669e847bf2 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/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 {}", diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 6a49db8a146..e9d714c5524 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -813,8 +813,8 @@ impl Module { def: Option, external: bool, is_public: bool) - -> Module { - Module { + -> Rc { + 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 { @@ -971,19 +971,27 @@ impl NameBinding { } } - fn and_then Option>(&self, f: F) -> Option { - self.borrow().as_ref().and_then(f) + fn borrow(&self) -> ::std::cell::Ref> { + self.0.borrow() } - fn borrow(&self) -> ::std::cell::Ref> { self.0.borrow() } - // Lifted versions of the NsDef methods and fields - fn def(&self) -> Option { self.and_then(NsDef::def) } - fn module(&self) -> Option> { self.and_then(NsDef::module) } - fn span(&self) -> Option { self.and_then(|def| def.span) } - fn modifiers(&self) -> Option { self.and_then(|def| Some(def.modifiers)) } + fn def(&self) -> Option { + self.borrow().as_ref().and_then(NsDef::def) + } + fn module(&self) -> Option> { + self.borrow().as_ref().and_then(NsDef::module) + } + fn span(&self) -> Option { + self.borrow().as_ref().and_then(|def| def.span) + } + fn modifiers(&self) -> Option { + 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, - 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, 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> { self.type_ns.module() } - - /// Returns the module node. Panics if this node does not have a module - /// definition. - fn get_module(&self) -> Rc { - 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, 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::>(); - 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 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; diff --git a/src/librustc_resolve/record_exports.rs b/src/librustc_resolve/record_exports.rs index 6e8d2ac4ca5..3a6a5a031b6 100644 --- a/src/librustc_resolve/record_exports.rs +++ b/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); } diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 24a21bcdfb1..4f67d6e2f7e 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/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[..],