Refactor away the fields id and is_public of ImportResolution and rename ImportResolution to NameResolution

Author: jseyfried

src/librustc_resolve/build_reduced_graph.rs

@@ -16,7 +16,7 @@
 use DefModifiers;
 use resolve_imports::ImportDirective;
 use resolve_imports::ImportDirectiveSubclass::{self, SingleImport, GlobImport};
-use resolve_imports::ImportResolution;
+use resolve_imports::NameResolution;
 use Module;
 use Namespace::{self, TypeNS, ValueNS};
 use {NameBinding, NameBindingKind};
@@ -703,13 +703,10 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                 let mut import_resolutions = module_.import_resolutions.borrow_mut();
                 for &ns in [TypeNS, ValueNS].iter() {
                     let mut resolution = import_resolutions.entry((target, ns)).or_insert(
-                        ImportResolution::new(id, is_public)
+                        NameResolution::default()
                     );
 
                     resolution.outstanding_references += 1;
-                    // the source of this name is different now
-                    resolution.id = id;
-                    resolution.is_public = is_public;
                 }
             }
             GlobImport => {

src/librustc_resolve/lib.rs

@@ -92,7 +92,7 @@ use std::cell::{Cell, RefCell};
 use std::fmt;
 use std::mem::replace;
 
-use resolve_imports::{ImportDirective, ImportResolution};
+use resolve_imports::{ImportDirective, NameResolution};
 
 // NB: This module needs to be declared first so diagnostics are
 // registered before they are used.
@@ -346,8 +346,9 @@ fn resolve_struct_error<'b, 'a: 'b, 'tcx: 'a>(resolver: &'b Resolver<'a, 'tcx>,
                                              .import_resolutions
                                              .borrow()
                                              .get(&(name, ValueNS)) {
-                let item = resolver.ast_map.expect_item(directive.id);
-                err.span_note(item.span, "constant imported here");
+                if let Some(binding) = directive.binding {
+                    err.span_note(binding.span.unwrap(), "constant imported here");
+                }
             }
             err
         }
@@ -814,7 +815,7 @@ pub struct ModuleS<'a> {
     anonymous_children: RefCell<NodeMap<Module<'a>>>,
 
     // The status of resolving each import in this module.
-    import_resolutions: RefCell<HashMap<(Name, Namespace), ImportResolution<'a>>>,
+    import_resolutions: RefCell<HashMap<(Name, Namespace), NameResolution<'a>>>,
 
     // The number of unresolved globs that this module exports.
     glob_count: Cell<usize>,
@@ -1219,8 +1220,12 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
     }
 
     #[inline]
-    fn record_import_use(&mut self, name: Name, ns: Namespace, resolution: &ImportResolution<'a>) {
-        let import_id = resolution.id;
+    fn record_import_use(&mut self, name: Name, ns: Namespace, binding: &NameBinding<'a>) {
+        let import_id = match binding.kind {
+            NameBindingKind::Import { id, .. } => id,
+            _ => return,
+        };
+
         self.used_imports.insert((import_id, ns));
 
         if !self.make_glob_map {
@@ -1592,20 +1597,22 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
 
         // Check the list of resolved imports.
         match module_.import_resolutions.borrow().get(&(name, namespace)) {
-            Some(import_resolution) if allow_private_imports || import_resolution.is_public => {
-                if import_resolution.is_public && import_resolution.outstanding_references != 0 {
-                    debug!("(resolving name in module) import unresolved; bailing out");
-                    return Indeterminate;
-                }
+            Some(import_resolution) => {
                 if let Some(binding) = import_resolution.binding {
+                    if !allow_private_imports && binding.is_public() { return Failed(None) }
+                    if binding.is_public() && import_resolution.outstanding_references != 0 {
+                        debug!("(resolving name in module) import unresolved; bailing out");
+                        return Indeterminate;
+                    }
+
                     debug!("(resolving name in module) resolved to import");
                     if record_used {
-                        self.record_import_use(name, namespace, &import_resolution);
+                        self.record_import_use(name, namespace, binding);
                     }
                     return Success(binding);
                 }
             }
-            Some(..) | None => {} // Continue.
+            None => {}
         }
 
         // We're out of luck.
@@ -3482,7 +3489,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                 if self.trait_item_map.contains_key(&(name, did)) {
                     add_trait_info(&mut found_traits, did, name);
                     let trait_name = self.get_trait_name(did);
-                    self.record_import_use(trait_name, TypeNS, &import);
+                    self.record_import_use(trait_name, TypeNS, binding);
                 }
             }
 

src/librustc_resolve/resolve_imports.rs

@@ -101,39 +101,30 @@ impl ImportDirective {
 }
 
 #[derive(Debug)]
-/// An ImportResolution records what we know about an imported name in a given namespace.
+/// An NameResolution records what we know about an imported name in a given namespace.
 /// More specifically, it records the number of unresolved `use` directives that import the name,
 /// the `use` directive importing the name in the namespace, and the `NameBinding` to which the
 /// name in the namespace resolves (if applicable).
 /// Different `use` directives may import the same name in different namespaces.
-pub struct ImportResolution<'a> {
+pub struct NameResolution<'a> {
     // When outstanding_references reaches zero, outside modules can count on the targets being
     // correct. Before then, all bets are off; future `use` directives could override the name.
     // Since shadowing is forbidden, the only way outstanding_references > 1 in a legal program
     // is if the name is imported by exactly two `use` directives, one of which resolves to a
     // value and the other of which resolves to a type.
     pub outstanding_references: usize,
 
-    /// Whether this resolution came from a `use` or a `pub use`.
-    pub is_public: bool,
-
     /// Resolution of the name in the namespace
     pub binding: Option<&'a NameBinding<'a>>,
-
-    /// The source node of the `use` directive
-    pub id: NodeId,
 }
 
-impl<'a> ImportResolution<'a> {
-    pub fn new(id: NodeId, is_public: bool) -> Self {
-        ImportResolution {
-            outstanding_references: 0,
-            id: id,
-            binding: None,
-            is_public: is_public,
-        }
+impl<'a> Default for NameResolution<'a> {
+    fn default() -> Self {
+        NameResolution { outstanding_references: 0, binding: None }
     }
+}
 
+impl<'a> NameResolution<'a> {
     pub fn shadowable(&self) -> Shadowable {
         match self.binding {
             Some(binding) if binding.defined_with(DefModifiers::PRELUDE) =>
@@ -216,8 +207,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                 debug!("(resolving import error) adding import resolution for `{}`",
                        target);
 
-                ImportResolution::new(e.import_directive.id,
-                                      e.import_directive.is_public)
+                NameResolution::default()
             });
 
             if resolution.binding.is_none() {
@@ -402,13 +392,13 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
 
             // The name is an import which has been fully resolved, so we just follow it.
             Some(resolution) if resolution.outstanding_references == 0 => {
-                // Import resolutions must be declared with "pub" in order to be exported.
-                if !resolution.is_public {
-                    return Failed(None);
-                }
-
                 if let Some(binding) = resolution.binding {
-                    self.resolver.record_import_use(name, ns, &resolution);
+                    // Import resolutions must be declared with "pub" in order to be exported.
+                    if !binding.is_public() {
+                        return Failed(None);
+                    }
+
+                    self.resolver.record_import_use(name, ns, binding);
                     Success(binding)
                 } else {
                     Failed(None)
@@ -549,10 +539,8 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
 
                         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;
 
-                        self.add_export(module_, target, &import_resolution);
+                        self.add_export(module_, target, import_resolution.binding.unwrap());
                     }
                     Failed(_) => {
                         // Continue.
@@ -644,7 +632,6 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                            lp: LastPrivate)
                            -> ResolveResult<()> {
         let id = import_directive.id;
-        let is_public = import_directive.is_public;
 
         // This function works in a highly imperative manner; it eagerly adds
         // everything it can to the list of import resolutions of the module
@@ -679,19 +666,17 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
             let mut import_resolutions = module_.import_resolutions.borrow_mut();
             let mut dest_import_resolution =
                 import_resolutions.entry((name, ns))
-                                  .or_insert_with(|| ImportResolution::new(id, is_public));
+                                  .or_insert_with(|| NameResolution::default());
 
             match target_import_resolution.binding {
-                Some(binding) if target_import_resolution.is_public => {
+                Some(binding) if binding.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(self.resolver.new_name_binding(import_directive.import(binding)));
-                    self.add_export(module_, name, &dest_import_resolution);
+                    self.add_export(module_, name, dest_import_resolution.binding.unwrap());
                 }
                 _ => {}
             }
@@ -728,12 +713,11 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                                import_directive: &ImportDirective,
                                (name, ns): (Name, Namespace),
                                name_binding: &'b NameBinding<'b>) {
-        let id = import_directive.id;
         let is_public = import_directive.is_public;
 
         let mut import_resolutions = module_.import_resolutions.borrow_mut();
         let dest_import_resolution = import_resolutions.entry((name, ns)).or_insert_with(|| {
-            ImportResolution::new(id, is_public)
+            NameResolution::default()
         });
 
         debug!("(resolving glob import) writing resolution `{}` in `{}` to `{}`",
@@ -767,9 +751,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
             } else {
                 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);
+                self.add_export(module_, name, dest_import_resolution.binding.unwrap());
             }
         }
 
@@ -779,13 +761,13 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                                                            (name, ns));
     }
 
-    fn add_export(&mut self, module: Module<'b>, name: Name, resolution: &ImportResolution<'b>) {
-        if !resolution.is_public { return }
+    fn add_export(&mut self, module: Module<'b>, name: Name, binding: &NameBinding<'b>) {
+        if !binding.is_public() { return }
         let node_id = match module.def_id() {
             Some(def_id) => self.resolver.ast_map.as_local_node_id(def_id).unwrap(),
             None => return,
         };
-        let export = match resolution.binding.as_ref().unwrap().def() {
+        let export = match binding.def() {
             Some(def) => Export { name: name, def_id: def.def_id() },
             None => return,
         };
@@ -794,7 +776,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
 
     /// Checks that imported names and items don't have the same name.
     fn check_for_conflicting_import(&mut self,
-                                    import_resolution: &ImportResolution,
+                                    import_resolution: &NameResolution,
                                     import_span: Span,
                                     name: Name,
                                     namespace: Namespace) {
@@ -815,8 +797,6 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                     }
                     ValueNS => "value",
                 };
-                let use_id = import_resolution.id;
-                let item = self.resolver.ast_map.expect_item(use_id);
                 let mut err = struct_span_err!(self.resolver.session,
                                                import_span,
                                                E0252,
@@ -825,7 +805,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                                                ns_word,
                                                name);
                 span_note!(&mut err,
-                           item.span,
+                           binding.span.unwrap(),
                            "previous import of `{}` here",
                            name);
                 err.emit();
@@ -848,7 +828,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
     /// Checks that imported names and items don't have the same name.
     fn check_for_conflicts_between_imports_and_items(&mut self,
                                                      module: Module<'b>,
-                                                     import: &ImportResolution<'b>,
+                                                     import: &NameResolution<'b>,
                                                      import_span: Span,
                                                      (name, ns): (Name, Namespace)) {
         // Check for item conflicts.