Add and use an arena for `NameBinding`s

src/librustc_resolve/build_reduced_graph.rs

@@ -101,14 +101,14 @@ fn build_reduced_graph(self, krate: &hir::Crate) {
     fn try_define<T>(&self, parent: Module<'b>, name: Name, ns: Namespace, def: T)
         where T: ToNameBinding<'b>
     {
-        parent.try_define_child(name, ns, def.to_name_binding());
+        parent.try_define_child(name, ns, self.new_name_binding(def.to_name_binding()));
     }
 
     /// Defines `name` in namespace `ns` of module `parent` to be `def` if it is not yet defined;
     /// otherwise, reports an error.
     fn define<T: ToNameBinding<'b>>(&self, parent: Module<'b>, name: Name, ns: Namespace, def: T) {
-        let binding = def.to_name_binding();
-        let old_binding = match parent.try_define_child(name, ns, binding.clone()) {
+        let binding = self.new_name_binding(def.to_name_binding());
+        let old_binding = match parent.try_define_child(name, ns, binding) {
             Some(old_binding) => old_binding,
             None => return,
         };

src/librustc_resolve/lib.rs

@@ -794,7 +794,7 @@ pub struct ModuleS<'a> {
     is_public: bool,
     is_extern_crate: bool,
 
-    children: RefCell<HashMap<(Name, Namespace), NameBinding<'a>>>,
+    children: RefCell<HashMap<(Name, Namespace), &'a NameBinding<'a>>>,
     imports: RefCell<Vec<ImportDirective>>,
 
     // The anonymous children of this node. Anonymous children are pseudo-
@@ -855,21 +855,21 @@ fn new(parent_link: ParentLink<'a>, def: Option<Def>, external: bool, is_public:
         }
     }
 
-    fn get_child(&self, name: Name, ns: Namespace) -> Option<NameBinding<'a>> {
+    fn get_child(&self, name: Name, ns: Namespace) -> Option<&'a NameBinding<'a>> {
         self.children.borrow().get(&(name, ns)).cloned()
     }
 
     // If the name is not yet defined, define the name and return None.
     // Otherwise, return the existing definition.
-    fn try_define_child(&self, name: Name, ns: Namespace, binding: NameBinding<'a>)
-                        -> Option<NameBinding<'a>> {
+    fn try_define_child(&self, name: Name, ns: Namespace, binding: &'a NameBinding<'a>)
+                        -> Option<&'a NameBinding<'a>> {
         match self.children.borrow_mut().entry((name, ns)) {
             hash_map::Entry::Vacant(entry) => { entry.insert(binding); None }
-            hash_map::Entry::Occupied(entry) => { Some(entry.get().clone()) },
+            hash_map::Entry::Occupied(entry) => { Some(entry.get()) },
         }
     }
 
-    fn for_each_local_child<F: FnMut(Name, Namespace, &NameBinding<'a>)>(&self, mut f: F) {
+    fn for_each_local_child<F: FnMut(Name, Namespace, &'a NameBinding<'a>)>(&self, mut f: F) {
         for (&(name, ns), name_binding) in self.children.borrow().iter() {
             if !name_binding.is_extern_crate() {
                 f(name, ns, name_binding)
@@ -1112,6 +1112,7 @@ pub struct Resolver<'a, 'tcx: 'a> {
 
 pub struct ResolverArenas<'a> {
     modules: arena::TypedArena<ModuleS<'a>>,
+    name_bindings: arena::TypedArena<NameBinding<'a>>,
 }
 
 #[derive(PartialEq)]
@@ -1177,6 +1178,7 @@ fn new(session: &'a Session,
     fn arenas() -> ResolverArenas<'a> {
         ResolverArenas {
             modules: arena::TypedArena::new(),
+            name_bindings: arena::TypedArena::new(),
         }
     }
 
@@ -1188,6 +1190,10 @@ fn new_module(&self,
         self.arenas.modules.alloc(ModuleS::new(parent_link, def, external, is_public))
     }
 
+    fn new_name_binding(&self, name_binding: NameBinding<'a>) -> &'a NameBinding<'a> {
+        self.arenas.name_bindings.alloc(name_binding)
+    }
+
     fn new_extern_crate_module(&self, parent_link: ParentLink<'a>, def: Def) -> Module<'a> {
         let mut module = ModuleS::new(parent_link, Some(def), false, true);
         module.is_extern_crate = true;
@@ -1234,7 +1240,7 @@ fn resolve_module_path_from_root(&mut self,
                                      -> ResolveResult<(Module<'a>, LastPrivate)> {
         fn search_parent_externals<'a>(needle: Name, module: Module<'a>) -> Option<Module<'a>> {
             match module.get_child(needle, TypeNS) {
-                Some(ref binding) if binding.is_extern_crate() => Some(module),
+                Some(binding) if binding.is_extern_crate() => Some(module),
                 _ => match module.parent_link {
                     ModuleParentLink(ref parent, _) => {
                         search_parent_externals(needle, parent)
@@ -1424,7 +1430,7 @@ fn resolve_item_in_lexical_scope(&mut self,
                                      name: Name,
                                      namespace: Namespace,
                                      record_used: bool)
-                                     -> ResolveResult<(NameBinding<'a>, bool)> {
+                                     -> ResolveResult<(&'a NameBinding<'a>, bool)> {
         debug!("(resolving item in lexical scope) resolving `{}` in namespace {:?} in `{}`",
                name,
                namespace,
@@ -1554,7 +1560,7 @@ fn resolve_name_in_module(&mut self,
                               namespace: Namespace,
                               allow_private_imports: bool,
                               record_used: bool)
-                              -> ResolveResult<(NameBinding<'a>, bool)> {
+                              -> ResolveResult<(&'a NameBinding<'a>, bool)> {
         debug!("(resolving name in module) resolving `{}` in `{}`",
                name,
                module_to_string(&*module_));
@@ -1580,7 +1586,7 @@ fn resolve_name_in_module(&mut self,
                     debug!("(resolving name in module) import unresolved; bailing out");
                     return Indeterminate;
                 }
-                if let Some(binding) = import_resolution.binding.clone() {
+                if let Some(binding) = import_resolution.binding {
                     debug!("(resolving name in module) resolved to import");
                     if record_used {
                         self.record_import_use(name, namespace, &import_resolution);
@@ -2619,7 +2625,7 @@ fn resolve_bare_identifier_pattern(&mut self,
             Success((binding, _)) => {
                 debug!("(resolve bare identifier pattern) succeeded in finding {} at {:?}",
                        name,
-                       &binding);
+                       binding);
                 match binding.def() {
                     None => {
                         panic!("resolved name in the value namespace to a set of name bindings \
@@ -3058,7 +3064,6 @@ fn get_module<'a, 'tcx>(this: &mut Resolver<'a, 'tcx>,
                 match this.primitive_type_table.primitive_types.get(last_name) {
                     Some(_) => None,
                     None => this.current_module.get_child(*last_name, TypeNS)
-                                               .as_ref()
                                                .and_then(NameBinding::module)
                 }
             } else {
@@ -3456,7 +3461,7 @@ fn add_trait_info(found_traits: &mut Vec<DefId>, trait_def_id: DefId, name: Name
             for (&(_, ns), import) in search_module.import_resolutions.borrow().iter() {
                 if ns != TypeNS { continue }
                 let binding = match import.binding {
-                    Some(ref binding) => binding,
+                    Some(binding) => binding,
                     None => continue,
                 };
                 let did = match binding.def() {

src/librustc_resolve/resolve_imports.rs

@@ -81,7 +81,8 @@ pub fn new(module_path: Vec<Name>,
 
     // Given the binding to which this directive resolves in a particular namespace,
     // this returns the binding for the name this directive defines in that namespace.
-    fn import<'a>(&self, mut binding: NameBinding<'a>) -> NameBinding<'a> {
+    fn import<'a>(&self, binding: &'a NameBinding<'a>) -> NameBinding<'a> {
+        let mut binding = binding.clone();
         if self.shadowable == Shadowable::Always {
             binding.modifiers = binding.modifiers | DefModifiers::PRELUDE;
         }
@@ -107,7 +108,7 @@ pub struct ImportResolution<'a> {
     pub is_public: bool,
 
     /// Resolution of the name in the namespace
-    pub binding: Option<NameBinding<'a>>,
+    pub binding: Option<&'a NameBinding<'a>>,
 
     /// The source node of the `use` directive
     pub id: NodeId,
@@ -125,7 +126,7 @@ pub fn new(id: NodeId, is_public: bool) -> Self {
 
     pub fn shadowable(&self) -> Shadowable {
         match self.binding {
-            Some(ref binding) if binding.defined_with(DefModifiers::PRELUDE) =>
+            Some(binding) if binding.defined_with(DefModifiers::PRELUDE) =>
                 Shadowable::Always,
             Some(_) => Shadowable::Never,
             None => Shadowable::Always,
@@ -195,7 +196,7 @@ fn resolve_imports(&mut self) {
 
     /// Resolves an `ImportResolvingError` into the correct enum discriminant
     /// and passes that on to `resolve_error`.
-    fn import_resolving_error(&self, e: ImportResolvingError) {
+    fn import_resolving_error(&self, e: ImportResolvingError<'b>) {
         // If it's a single failed import then create a "fake" import
         // resolution for it so that later resolve stages won't complain.
         if let SingleImport(target, _) = e.import_directive.subclass {
@@ -213,11 +214,11 @@ fn import_resolving_error(&self, e: ImportResolvingError) {
                 debug!("(resolving import error) adding fake target to import resolution of `{}`",
                        target);
 
-                let dummy_binding = NameBinding {
+                let dummy_binding = self.resolver.new_name_binding(NameBinding {
                     modifiers: DefModifiers::IMPORTABLE,
                     def_or_module: DefOrModule::Def(Def::Err),
                     span: None,
-                };
+                });
 
                 resolution.binding = Some(dummy_binding);
             }
@@ -367,7 +368,7 @@ fn resolve_name_in_module(&mut self,
                               name: Name,
                               ns: Namespace,
                               importing_module: Module<'b>) // Module importing the name
-                              -> (ResolveResult<NameBinding<'b>>, bool) {
+                              -> (ResolveResult<&'b NameBinding<'b>>, bool) {
         build_reduced_graph::populate_module_if_necessary(self.resolver, module);
         if let Some(name_binding) = module.get_child(name, ns) {
             if name_binding.is_extern_crate() {
@@ -397,7 +398,7 @@ fn resolve_name_in_module(&mut self,
                     return (Failed(None), false);
                 }
 
-                if let Some(binding) = resolution.binding.clone() {
+                if let Some(binding) = resolution.binding {
                     self.resolver.record_import_use(name, ns, &resolution);
                     (Success(binding), true)
                 } else {
@@ -455,9 +456,9 @@ fn resolve_single_import(&mut self,
             self.resolve_name_in_module(target_module, source, TypeNS, module_);
 
         match (&value_result, &type_result) {
-            (&Success(ref name_binding), _) if !value_used_reexport &&
-                                               directive.is_public &&
-                                               !name_binding.is_public() => {
+            (&Success(name_binding), _) if !value_used_reexport &&
+                                           directive.is_public &&
+                                           !name_binding.is_public() => {
                 let msg = format!("`{}` is private, and cannot be reexported", source);
                 let note_msg = format!("Consider marking `{}` as `pub` in the imported module",
                                         source);
@@ -466,8 +467,7 @@ fn resolve_single_import(&mut self,
                     .emit();
             }
 
-            (_, &Success(ref name_binding)) if !type_used_reexport &&
-                                               directive.is_public => {
+            (_, &Success(name_binding)) if !type_used_reexport && directive.is_public => {
                 if !name_binding.is_public() {
                     let msg = format!("`{}` is private, and cannot be reexported", source);
                     let note_msg =
@@ -519,7 +519,7 @@ fn resolve_single_import(&mut self,
 
         {
             let mut check_and_write_import = |namespace, result, used_public: &mut bool| {
-                let result: &ResolveResult<NameBinding> = result;
+                let result: &ResolveResult<&NameBinding> = result;
 
                 let import_resolution = import_resolutions.get_mut(&(target, namespace)).unwrap();
                 let namespace_name = match namespace {
@@ -528,7 +528,7 @@ fn resolve_single_import(&mut self,
                 };
 
                 match *result {
-                    Success(ref name_binding) => {
+                    Success(name_binding) => {
                         debug!("(resolving single import) found {:?} target: {:?}",
                                namespace_name,
                                name_binding.def());
@@ -541,7 +541,8 @@ fn resolve_single_import(&mut self,
                                                              directive.span,
                                                              target);
 
-                        import_resolution.binding = Some(directive.import(name_binding.clone()));
+                        import_resolution.binding =
+                            Some(self.resolver.new_name_binding(directive.import(name_binding)));
                         import_resolution.id = directive.id;
                         import_resolution.is_public = directive.is_public;
 
@@ -679,14 +680,15 @@ fn resolve_glob_import(&mut self,
                                   .or_insert_with(|| ImportResolution::new(id, is_public));
 
             match target_import_resolution.binding {
-                Some(ref binding) if target_import_resolution.is_public => {
+                Some(binding) if target_import_resolution.is_public => {
                     self.check_for_conflicting_import(&dest_import_resolution,
                                                       import_directive.span,
                                                       name,
                                                       ns);
                     dest_import_resolution.id = id;
                     dest_import_resolution.is_public = is_public;
-                    dest_import_resolution.binding = Some(import_directive.import(binding.clone()));
+                    dest_import_resolution.binding =
+                        Some(self.resolver.new_name_binding(import_directive.import(binding)));
                     self.add_export(module_, name, &dest_import_resolution);
                 }
                 _ => {}
@@ -701,7 +703,7 @@ fn resolve_glob_import(&mut self,
                                          target_module,
                                          import_directive,
                                          (name, ns),
-                                         name_binding.clone());
+                                         name_binding);
         });
 
         // Record the destination of this import
@@ -723,7 +725,7 @@ fn merge_import_resolution(&mut self,
                                containing_module: Module<'b>,
                                import_directive: &ImportDirective,
                                (name, ns): (Name, Namespace),
-                               name_binding: NameBinding<'b>) {
+                               name_binding: &'b NameBinding<'b>) {
         let id = import_directive.id;
         let is_public = import_directive.is_public;
 
@@ -761,7 +763,8 @@ fn merge_import_resolution(&mut self,
                                  name);
                 span_err!(self.resolver.session, import_directive.span, E0251, "{}", msg);
             } else {
-                dest_import_resolution.binding = Some(import_directive.import(name_binding.clone()));
+                dest_import_resolution.binding =
+                    Some(self.resolver.new_name_binding(import_directive.import(name_binding)));
                 dest_import_resolution.id = id;
                 dest_import_resolution.is_public = is_public;
                 self.add_export(module_, name, &dest_import_resolution);
@@ -799,7 +802,7 @@ fn check_for_conflicting_import(&mut self,
                binding.is_some());
 
         match *binding {
-            Some(ref binding) if !binding.defined_with(DefModifiers::PRELUDE) => {
+            Some(binding) if !binding.defined_with(DefModifiers::PRELUDE) => {
                 let ns_word = match namespace {
                     TypeNS => {
                         match binding.module() {
@@ -857,7 +860,7 @@ fn check_for_conflicts_between_imports_and_items(&mut self,
 
         if ns == ValueNS {
             match import.binding {
-                Some(ref binding) if !binding.defined_with(DefModifiers::PRELUDE) => {
+                Some(binding) if !binding.defined_with(DefModifiers::PRELUDE) => {
                     let mut err = struct_span_err!(self.resolver.session,
                                                    import_span,
                                                    E0255,
@@ -873,7 +876,7 @@ fn check_for_conflicts_between_imports_and_items(&mut self,
             }
         } else {
             match import.binding {
-                Some(ref binding) if !binding.defined_with(DefModifiers::PRELUDE) => {
+                Some(binding) if !binding.defined_with(DefModifiers::PRELUDE) => {
                     if name_binding.is_extern_crate() {
                         let msg = format!("import `{0}` conflicts with imported crate \
                                            in this module (maybe you meant `use {0}::*`?)",