Treat extern crates more like imports (pure refactoring).

Author: jseyfried

src/librustc_resolve/build_reduced_graph.rs

@@ -14,6 +14,7 @@
 //! any imports resolved.
 
 use macros::{InvocationData, LegacyScope};
+use resolve_imports::ImportDirective;
 use resolve_imports::ImportDirectiveSubclass::{self, GlobImport};
 use {Module, ModuleS, ModuleKind};
 use Namespace::{self, TypeNS, ValueNS};
@@ -237,11 +238,22 @@ impl<'b> Resolver<'b> {
                         index: CRATE_DEF_INDEX,
                     };
                     let module = self.arenas.alloc_module(ModuleS {
-                        extern_crate_id: Some(item.id),
                         populated: Cell::new(false),
                         ..ModuleS::new(Some(parent), ModuleKind::Def(Def::Mod(def_id), name))
                     });
-                    self.define(parent, name, TypeNS, (module, sp, vis));
+                    let binding = (module, sp, ty::Visibility::Public).to_name_binding();
+                    let binding = self.arenas.alloc_name_binding(binding);
+                    let directive = self.arenas.alloc_import_directive(ImportDirective {
+                        id: item.id,
+                        parent: parent,
+                        imported_module: Cell::new(Some(module)),
+                        subclass: ImportDirectiveSubclass::ExternCrate,
+                        span: item.span,
+                        module_path: Vec::new(),
+                        vis: Cell::new(vis),
+                    });
+                    let imported_binding = self.import(binding, directive);
+                    self.define(parent, name, TypeNS, imported_binding);
                     self.populate_module_if_necessary(module);
                     module
                 } else {

src/librustc_resolve/lib.rs

@@ -76,7 +76,7 @@ use std::fmt;
 use std::mem::replace;
 use std::rc::Rc;
 
-use resolve_imports::{ImportDirective, NameResolution};
+use resolve_imports::{ImportDirective, ImportDirectiveSubclass, NameResolution};
 use macros::{InvocationData, LegacyBinding, LegacyScope};
 
 // NB: This module needs to be declared first so diagnostics are
@@ -796,10 +796,6 @@ pub struct ModuleS<'a> {
     // The node id of the closest normal module (`mod`) ancestor (including this module).
     normal_ancestor_id: Option<NodeId>,
 
-    // If the module is an extern crate, `def` is root of the external crate and `extern_crate_id`
-    // is the NodeId of the local `extern crate` item (otherwise, `extern_crate_id` is None).
-    extern_crate_id: Option<NodeId>,
-
     resolutions: RefCell<FxHashMap<(Name, Namespace), &'a RefCell<NameResolution<'a>>>>,
 
     no_implicit_prelude: bool,
@@ -824,7 +820,6 @@ impl<'a> ModuleS<'a> {
             parent: parent,
             kind: kind,
             normal_ancestor_id: None,
-            extern_crate_id: None,
             resolutions: RefCell::new(FxHashMap()),
             no_implicit_prelude: false,
             glob_importers: RefCell::new(Vec::new()),
@@ -953,7 +948,14 @@ impl<'a> NameBinding<'a> {
     }
 
     fn is_extern_crate(&self) -> bool {
-        self.module().ok().and_then(|module| module.extern_crate_id).is_some()
+        match self.kind {
+            NameBindingKind::Import {
+                directive: &ImportDirective {
+                    subclass: ImportDirectiveSubclass::ExternCrate, ..
+                }, ..
+            } => true,
+            _ => false,
+        }
     }
 
     fn is_import(&self) -> bool {
@@ -3233,7 +3235,7 @@ impl<'a> Resolver<'a> {
             in_module.for_each_child(|name, ns, name_binding| {
 
                 // avoid imports entirely
-                if name_binding.is_import() { return; }
+                if name_binding.is_import() && !name_binding.is_extern_crate() { return; }
 
                 // collect results based on the filter function
                 if name == lookup_name && ns == namespace {
@@ -3269,21 +3271,11 @@ impl<'a> Resolver<'a> {
                 // collect submodules to explore
                 if let Ok(module) = name_binding.module() {
                     // form the path
-                    let path_segments = match module.kind {
-                        _ if module.parent.is_none() => path_segments.clone(),
-                        ModuleKind::Def(_, name) => {
-                            let mut paths = path_segments.clone();
-                            let ident = Ident::with_empty_ctxt(name);
-                            let params = PathParameters::none();
-                            let segm = PathSegment {
-                                identifier: ident,
-                                parameters: params,
-                            };
-                            paths.push(segm);
-                            paths
-                        }
-                        _ => bug!(),
-                    };
+                    let mut path_segments = path_segments.clone();
+                    path_segments.push(PathSegment {
+                        identifier: Ident::with_empty_ctxt(name),
+                        parameters: PathParameters::none(),
+                    });
 
                     if !in_module_is_extern || name_binding.vis == ty::Visibility::Public {
                         // add the module to the lookup
@@ -3369,7 +3361,10 @@ impl<'a> Resolver<'a> {
             if !reported_spans.insert(span) { continue }
             if binding.is_extern_crate() {
                 // Warn when using an inaccessible extern crate.
-                let node_id = binding.module().unwrap().extern_crate_id.unwrap();
+                let node_id = match binding.kind {
+                    NameBindingKind::Import { directive, .. } => directive.id,
+                    _ => unreachable!(),
+                };
                 let msg = format!("extern crate `{}` is private", name);
                 self.session.add_lint(lint::builtin::INACCESSIBLE_EXTERN_CRATE, node_id, span, msg);
             } else {
@@ -3415,7 +3410,7 @@ impl<'a> Resolver<'a> {
             _ => "enum",
         };
 
-        let (participle, noun) = match old_binding.is_import() || old_binding.is_extern_crate() {
+        let (participle, noun) = match old_binding.is_import() {
             true => ("imported", "import"),
             false => ("defined", "definition"),
         };
@@ -3424,7 +3419,7 @@ impl<'a> Resolver<'a> {
         let msg = {
             let kind = match (ns, old_binding.module()) {
                 (ValueNS, _) => "a value",
-                (TypeNS, Ok(module)) if module.extern_crate_id.is_some() => "an extern crate",
+                (TypeNS, _) if old_binding.is_extern_crate() => "an extern crate",
                 (TypeNS, Ok(module)) if module.is_normal() => "a module",
                 (TypeNS, Ok(module)) if module.is_trait() => "a trait",
                 (TypeNS, _) => "a type",
@@ -3439,7 +3434,7 @@ impl<'a> Resolver<'a> {
                 e.span_label(span, &format!("`{}` was already imported", name));
                 e
             },
-            (true, _) | (_, true) if binding.is_import() || old_binding.is_import() => {
+            (true, _) | (_, true) if binding.is_import() && old_binding.is_import() => {
                 let mut e = struct_span_err!(self.session, span, E0254, "{}", msg);
                 e.span_label(span, &"already imported");
                 e

src/librustc_resolve/resolve_imports.rs

@@ -51,6 +51,7 @@ pub enum ImportDirectiveSubclass<'a> {
         max_vis: Cell<ty::Visibility>, // The visibility of the greatest reexport.
         // n.b. `max_vis` is only used in `finalize_import` to check for reexport errors.
     },
+    ExternCrate,
 }
 
 impl<'a> ImportDirectiveSubclass<'a> {
@@ -68,12 +69,12 @@ impl<'a> ImportDirectiveSubclass<'a> {
 #[derive(Debug,Clone)]
 pub struct ImportDirective<'a> {
     pub id: NodeId,
-    parent: Module<'a>,
-    module_path: Vec<Ident>,
-    imported_module: Cell<Option<Module<'a>>>, // the resolution of `module_path`
-    subclass: ImportDirectiveSubclass<'a>,
-    span: Span,
-    vis: Cell<ty::Visibility>,
+    pub parent: Module<'a>,
+    pub module_path: Vec<Ident>,
+    pub imported_module: Cell<Option<Module<'a>>>, // the resolution of `module_path`
+    pub subclass: ImportDirectiveSubclass<'a>,
+    pub span: Span,
+    pub vis: Cell<ty::Visibility>,
 }
 
 impl<'a> ImportDirective<'a> {
@@ -169,7 +170,8 @@ impl<'a> Resolver<'a> {
         let new_import_semantics = self.new_import_semantics;
         let is_disallowed_private_import = |binding: &NameBinding| {
             !new_import_semantics && !allow_private_imports && // disallowed
-            binding.vis != ty::Visibility::Public && binding.is_import() // non-`pub` import
+            binding.vis != ty::Visibility::Public && binding.is_import() && // non-`pub` import
+            !binding.is_extern_crate() // not an `extern crate`
         };
 
         if let Some(span) = record_used {
@@ -237,7 +239,7 @@ impl<'a> Resolver<'a> {
                 };
                 let name = match directive.subclass {
                     SingleImport { source, .. } => source,
-                    GlobImport { .. } => unreachable!(),
+                    _ => unreachable!(),
                 };
                 match self.resolve_name_in_module(module, name, ns, true, None) {
                     Failed(_) => {}
@@ -280,13 +282,14 @@ impl<'a> Resolver<'a> {
             // which are not relevant to import resolution.
             GlobImport { is_prelude: true, .. } => {}
             GlobImport { .. } => self.current_module.globs.borrow_mut().push(directive),
+            _ => unreachable!(),
         }
     }
 
     // Given a binding and an import directive that resolves to it,
     // return the corresponding binding defined by the import directive.
-    fn import(&mut self, binding: &'a NameBinding<'a>, directive: &'a ImportDirective<'a>)
-              -> NameBinding<'a> {
+    pub fn import(&mut self, binding: &'a NameBinding<'a>, directive: &'a ImportDirective<'a>)
+                  -> NameBinding<'a> {
         let vis = if binding.pseudo_vis().is_at_least(directive.vis.get(), self) ||
                      !directive.is_glob() && binding.is_extern_crate() { // c.f. `PRIVATE_IN_PUBLIC`
             directive.vis.get()
@@ -529,6 +532,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
                 self.resolve_glob_import(directive);
                 return Success(());
             }
+            _ => unreachable!(),
         };
 
         let mut indeterminate = false;
@@ -616,6 +620,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
                 }
                 return Success(());
             }
+            _ => unreachable!(),
         };
 
         for &(ns, result) in &[(ValueNS, value_result), (TypeNS, type_result)] {
@@ -831,5 +836,6 @@ fn import_directive_subclass_to_string(subclass: &ImportDirectiveSubclass) -> St
     match *subclass {
         SingleImport { source, .. } => source.to_string(),
         GlobImport { .. } => "*".to_string(),
+        ExternCrate => "<extern crate>".to_string(),
     }
 }