From b0e13dc5ba7a8f91ba564c84c62032735bbdc918 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Fri, 28 Oct 2016 03:40:58 +0000 Subject: [PATCH] Treat `extern crate`s more like imports (pure refactoring). --- src/librustc_resolve/build_reduced_graph.rs | 16 ++++++- src/librustc_resolve/lib.rs | 49 +++++++++------------ src/librustc_resolve/resolve_imports.rs | 26 ++++++----- 3 files changed, 52 insertions(+), 39 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 475d454e220e..ca6e72dd4f44 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/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 { diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 664efc27fbb5..4de272bc3a04 100644 --- a/src/librustc_resolve/lib.rs +++ b/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, - // 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, - resolutions: RefCell>>>, 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 diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 2b3945bd0d92..91a7dc4249ac 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -51,6 +51,7 @@ pub enum ImportDirectiveSubclass<'a> { max_vis: Cell, // 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, - imported_module: Cell>>, // the resolution of `module_path` - subclass: ImportDirectiveSubclass<'a>, - span: Span, - vis: Cell, + pub parent: Module<'a>, + pub module_path: Vec, + pub imported_module: Cell>>, // the resolution of `module_path` + pub subclass: ImportDirectiveSubclass<'a>, + pub span: Span, + pub vis: Cell, } 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 => "".to_string(), } }