resolve: use arena allocation instead of reference counting for Modules to fix memory leaks from Rc cycles

Author: jseyfried

mk/crates.mk

@@ -103,7 +103,7 @@ DEPS_rustc_lint := rustc log syntax
 DEPS_rustc_llvm := native:rustllvm libc std rustc_bitflags
 DEPS_rustc_metadata := rustc rustc_front syntax rbml
 DEPS_rustc_mir := rustc rustc_front syntax
-DEPS_rustc_resolve := rustc rustc_front log syntax
+DEPS_rustc_resolve := arena rustc rustc_front log syntax
 DEPS_rustc_platform_intrinsics := rustc rustc_llvm
 DEPS_rustc_plugin := rustc rustc_metadata syntax
 DEPS_rustc_privacy := rustc rustc_front log syntax

src/librustc_resolve/build_reduced_graph.rs

@@ -21,7 +21,7 @@ use Module;
 use Namespace::{TypeNS, ValueNS};
 use NameBindings;
 use {names_to_string, module_to_string};
-use ParentLink::{self, ModuleParentLink, BlockParentLink};
+use ParentLink::{ModuleParentLink, BlockParentLink};
 use Resolver;
 use resolve_imports::Shadowable;
 use {resolve_error, resolve_struct_error, ResolutionError};
@@ -52,7 +52,6 @@ use rustc_front::intravisit::{self, Visitor};
 
 use std::mem::replace;
 use std::ops::{Deref, DerefMut};
-use std::rc::Rc;
 
 // Specifies how duplicates should be handled when adding a child item if
 // another item exists with the same name in some namespace.
@@ -86,7 +85,7 @@ 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 mut visitor = BuildReducedGraphVisitor {
-            parent: self.graph_root.clone(),
+            parent: self.graph_root,
             builder: self,
         };
         intravisit::walk_crate(&mut visitor, krate);
@@ -97,12 +96,12 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
     /// Returns the child's corresponding name bindings.
     fn add_child(&self,
                  name: Name,
-                 parent: &Rc<Module>,
+                 parent: Module<'b>,
                  duplicate_checking_mode: DuplicateCheckingMode,
                  // For printing errors
                  sp: Span)
-                 -> NameBindings {
-        self.check_for_conflicts_between_external_crates_and_items(&**parent, name, sp);
+                 -> NameBindings<'b> {
+        self.check_for_conflicts_between_external_crates_and_items(parent, name, sp);
 
         // Add or reuse the child.
         let child = parent.children.borrow().get(&name).cloned();
@@ -178,12 +177,8 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
         return false;
     }
 
-    fn get_parent_link(&mut self, parent: &Rc<Module>, name: Name) -> ParentLink {
-        ModuleParentLink(Rc::downgrade(parent), name)
-    }
-
     /// Constructs the reduced graph for one item.
-    fn build_reduced_graph_for_item(&mut self, item: &Item, parent: &Rc<Module>) -> Rc<Module> {
+    fn build_reduced_graph_for_item(&mut self, item: &Item, parent: Module<'b>) -> Module<'b> {
         let name = item.name;
         let sp = item.span;
         let is_public = item.vis == hir::Public;
@@ -238,7 +233,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                         }
 
                         let subclass = SingleImport(binding, source_name);
-                        self.build_import_directive(&**parent,
+                        self.build_import_directive(parent,
                                                     module_path,
                                                     subclass,
                                                     view_path.span,
@@ -288,7 +283,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                                     (module_path.to_vec(), name, rename)
                                 }
                             };
-                            self.build_import_directive(&**parent,
+                            self.build_import_directive(parent,
                                                         module_path,
                                                         SingleImport(rename, name),
                                                         source_item.span,
@@ -298,7 +293,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                         }
                     }
                     ViewPathGlob(_) => {
-                        self.build_import_directive(&**parent,
+                        self.build_import_directive(parent,
                                                     module_path,
                                                     GlobImport,
                                                     view_path.span,
@@ -307,7 +302,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                                                     shadowable);
                     }
                 }
-                parent.clone()
+                parent
             }
 
             ItemExternCrate(_) => {
@@ -319,32 +314,32 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                         index: CRATE_DEF_INDEX,
                     };
                     self.external_exports.insert(def_id);
-                    let parent_link = ModuleParentLink(Rc::downgrade(parent), name);
+                    let parent_link = ModuleParentLink(parent, name);
                     let def = DefMod(def_id);
-                    let external_module = Module::new(parent_link, Some(def), false, true);
+                    let external_module = self.new_module(parent_link, Some(def), false, true);
 
                     debug!("(build reduced graph for item) found extern `{}`",
                            module_to_string(&*external_module));
-                    self.check_for_conflicts_for_external_crate(&parent, name, sp);
+                    self.check_for_conflicts_for_external_crate(parent, name, sp);
                     parent.external_module_children
                           .borrow_mut()
-                          .insert(name, external_module.clone());
+                          .insert(name, external_module);
                     self.build_reduced_graph_for_external_crate(&external_module);
                 }
-                parent.clone()
+                parent
             }
 
             ItemMod(..) => {
                 let name_bindings = self.add_child(name, parent, ForbidDuplicateTypes, sp);
 
-                let parent_link = self.get_parent_link(parent, name);
+                let parent_link = ModuleParentLink(parent, name);
                 let def = DefMod(self.ast_map.local_def_id(item.id));
-                let module = Module::new(parent_link, Some(def), false, is_public);
+                let module = self.new_module(parent_link, Some(def), false, is_public);
                 name_bindings.define_module(module.clone(), sp);
                 module
             }
 
-            ItemForeignMod(..) => parent.clone(),
+            ItemForeignMod(..) => parent,
 
             // These items live in the value namespace.
             ItemStatic(_, m, _) => {
@@ -354,19 +349,19 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                 name_bindings.define_value(DefStatic(self.ast_map.local_def_id(item.id), mutbl),
                                            sp,
                                            modifiers);
-                parent.clone()
+                parent
             }
             ItemConst(_, _) => {
                 self.add_child(name, parent, ForbidDuplicateValues, sp)
                     .define_value(DefConst(self.ast_map.local_def_id(item.id)), sp, modifiers);
-                parent.clone()
+                parent
             }
             ItemFn(_, _, _, _, _, _) => {
                 let name_bindings = self.add_child(name, parent, ForbidDuplicateValues, sp);
 
                 let def = DefFn(self.ast_map.local_def_id(item.id), false);
                 name_bindings.define_value(def, sp, modifiers);
-                parent.clone()
+                parent
             }
 
             // These items live in the type namespace.
@@ -376,11 +371,11 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                                                    ForbidDuplicateTypes,
                                                    sp);
 
-                let parent_link = self.get_parent_link(parent, name);
+                let parent_link = ModuleParentLink(parent, name);
                 let def = DefTy(self.ast_map.local_def_id(item.id), false);
-                let module = Module::new(parent_link, Some(def), false, is_public);
+                let module = self.new_module(parent_link, Some(def), false, is_public);
                 name_bindings.define_module(module, sp);
-                parent.clone()
+                parent
             }
 
             ItemEnum(ref enum_definition, _) => {
@@ -389,9 +384,9 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                                                    ForbidDuplicateTypes,
                                                    sp);
 
-                let parent_link = self.get_parent_link(parent, name);
+                let parent_link = ModuleParentLink(parent, name);
                 let def = DefTy(self.ast_map.local_def_id(item.id), true);
-                let module = Module::new(parent_link, Some(def), false, is_public);
+                let module = self.new_module(parent_link, Some(def), false, is_public);
                 name_bindings.define_module(module.clone(), sp);
 
                 let variant_modifiers = if is_public {
@@ -404,7 +399,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                     self.build_reduced_graph_for_variant(variant, item_def_id,
                                                          &module, variant_modifiers);
                 }
-                parent.clone()
+                parent
             }
 
             // These items live in both the type and value namespaces.
@@ -444,11 +439,11 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                 let item_def_id = self.ast_map.local_def_id(item.id);
                 self.structs.insert(item_def_id, named_fields);
 
-                parent.clone()
+                parent
             }
 
             ItemDefaultImpl(_, _) |
-            ItemImpl(..) => parent.clone(),
+            ItemImpl(..) => parent,
 
             ItemTrait(_, _, _, ref items) => {
                 let name_bindings = self.add_child(name,
@@ -459,9 +454,9 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                 let def_id = self.ast_map.local_def_id(item.id);
 
                 // Add all the items within to a new module.
-                let parent_link = self.get_parent_link(parent, name);
+                let parent_link = ModuleParentLink(parent, name);
                 let def = DefTrait(def_id);
-                let module_parent = Module::new(parent_link, Some(def), false, is_public);
+                let module_parent = self.new_module(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.
@@ -494,7 +489,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                     self.trait_item_map.insert((trait_item.name, def_id), trait_item_def_id);
                 }
 
-                parent.clone()
+                parent
             }
         }
     }
@@ -504,7 +499,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
     fn build_reduced_graph_for_variant(&mut self,
                                        variant: &Variant,
                                        item_id: DefId,
-                                       parent: &Rc<Module>,
+                                       parent: Module<'b>,
                                        variant_modifiers: DefModifiers) {
         let name = variant.node.name;
         let is_exported = if variant.node.data.is_struct() {
@@ -534,7 +529,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
     /// Constructs the reduced graph for one foreign item.
     fn build_reduced_graph_for_foreign_item(&mut self,
                                             foreign_item: &ForeignItem,
-                                            parent: &Rc<Module>) {
+                                            parent: Module<'b>) {
         let name = foreign_item.name;
         let is_public = foreign_item.vis == hir::Public;
         let modifiers = if is_public {
@@ -555,30 +550,30 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
         name_bindings.define_value(def, foreign_item.span, modifiers);
     }
 
-    fn build_reduced_graph_for_block(&mut self, block: &Block, parent: &Rc<Module>) -> Rc<Module> {
+    fn build_reduced_graph_for_block(&mut self, block: &Block, parent: Module<'b>) -> Module<'b> {
         if self.block_needs_anonymous_module(block) {
             let block_id = block.id;
 
             debug!("(building reduced graph for block) creating a new anonymous module for block \
                     {}",
                    block_id);
 
-            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());
+            let parent_link = BlockParentLink(parent, block_id);
+            let new_module = self.new_module(parent_link, None, false, false);
+            parent.anonymous_children.borrow_mut().insert(block_id, new_module);
             new_module
         } else {
-            parent.clone()
+            parent
         }
     }
 
     fn handle_external_def(&mut self,
                            def: Def,
                            vis: Visibility,
-                           child_name_bindings: &NameBindings,
+                           child_name_bindings: &NameBindings<'b>,
                            final_ident: &str,
                            name: Name,
-                           new_parent: &Rc<Module>) {
+                           new_parent: Module<'b>) {
         debug!("(building reduced graph for external crate) building external def {}, priv {:?}",
                final_ident,
                vis);
@@ -609,8 +604,8 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                     debug!("(building reduced graph for external crate) building module {} {}",
                            final_ident,
                            is_public);
-                    let parent_link = self.get_parent_link(new_parent, name);
-                    let module = Module::new(parent_link, Some(def), true, is_public);
+                    let parent_link = ModuleParentLink(new_parent, name);
+                    let module = self.new_module(parent_link, Some(def), true, is_public);
                     child_name_bindings.define_module(module, DUMMY_SP);
                 }
             }
@@ -681,8 +676,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);
-                let module = Module::new(parent_link, Some(def), true, is_public);
+                let parent_link = ModuleParentLink(new_parent, name);
+                let module = self.new_module(parent_link, Some(def), true, is_public);
                 child_name_bindings.define_module(module, DUMMY_SP);
             }
             DefTy(..) | DefAssociatedTy(..) => {
@@ -728,7 +723,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
 
     /// Builds the reduced graph for a single item in an external crate.
     fn build_reduced_graph_for_external_crate_def(&mut self,
-                                                  root: &Rc<Module>,
+                                                  root: Module<'b>,
                                                   xcdef: ChildItem) {
         match xcdef.def {
             DlDef(def) => {
@@ -766,9 +761,9 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
     }
 
     /// Builds the reduced graph rooted at the given external module.
-    fn populate_external_module(&mut self, module: &Rc<Module>) {
+    fn populate_external_module(&mut self, module: Module<'b>) {
         debug!("(populating external module) attempting to populate {}",
-               module_to_string(&**module));
+               module_to_string(module));
 
         let def_id = match module.def_id() {
             None => {
@@ -788,7 +783,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
 
     /// Ensures that the reduced graph rooted at the given external module
     /// is built, building it if it is not.
-    fn populate_module_if_necessary(&mut self, module: &Rc<Module>) {
+    fn populate_module_if_necessary(&mut self, module: Module<'b>) {
         if !module.populated.get() {
             self.populate_external_module(module)
         }
@@ -797,7 +792,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
 
     /// Builds the reduced graph rooted at the 'use' directive for an external
     /// crate.
-    fn build_reduced_graph_for_external_crate(&mut self, root: &Rc<Module>) {
+    fn build_reduced_graph_for_external_crate(&mut self, root: Module<'b>) {
         let root_cnum = root.def_id().unwrap().krate;
         for child in self.session.cstore.crate_top_level_items(root_cnum) {
             self.build_reduced_graph_for_external_crate_def(root, child);
@@ -806,7 +801,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
 
     /// Creates and adds an import directive to the given module.
     fn build_import_directive(&mut self,
-                              module_: &Module,
+                              module_: Module<'b>,
                               module_path: Vec<Name>,
                               subclass: ImportDirectiveSubclass,
                               span: Span,
@@ -866,7 +861,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
 
 struct BuildReducedGraphVisitor<'a, 'b: 'a, 'tcx: 'b> {
     builder: GraphBuilder<'a, 'b, 'tcx>,
-    parent: Rc<Module>,
+    parent: Module<'b>,
 }
 
 impl<'a, 'b, 'v, 'tcx> Visitor<'v> for BuildReducedGraphVisitor<'a, 'b, 'tcx> {
@@ -897,6 +892,7 @@ pub fn build_reduced_graph(resolver: &mut Resolver, krate: &hir::Crate) {
     GraphBuilder { resolver: resolver }.build_reduced_graph(krate);
 }
 
-pub fn populate_module_if_necessary(resolver: &mut Resolver, module: &Rc<Module>) {
+pub fn populate_module_if_necessary<'a, 'tcx>(resolver: &mut Resolver<'a, 'tcx>,
+                                              module: Module<'a>) {
     GraphBuilder { resolver: resolver }.populate_module_if_necessary(module);
 }