From: Jeffrey Seyfried Date: Tue, 16 Feb 2016 03:54:14 +0000 (+0000) Subject: Start importing bindings from globs as soon as the glob path is known. X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=20b99d303d0fb70c98d8c460324c8e1824fef482;p=rust.git Start importing bindings from globs as soon as the glob path is known. --- diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index f82cf98c2d6..2ab1d637ef9 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -295,14 +295,6 @@ fn build_reduced_graph_for_item(&mut self, item: &Item, parent: Module<'b>) -> M let module = self.new_extern_crate_module(parent_link, def, is_public, item.id); self.define(parent, name, TypeNS, (module, sp)); - if is_public { - let export = Export { name: name, def_id: def_id }; - if let Some(def_id) = parent.def_id() { - let node_id = self.resolver.ast_map.as_local_node_id(def_id).unwrap(); - self.export_map.entry(node_id).or_insert(Vec::new()).push(export); - } - } - self.build_reduced_graph_for_external_crate(module); } parent diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 0e21b769b74..2a65b26cc94 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -18,6 +18,7 @@ #![cfg_attr(not(stage0), deny(warnings))] #![feature(associated_consts)] +#![feature(borrow_state)] #![feature(rustc_diagnostic_macros)] #![feature(rustc_private)] #![feature(staged_api)] @@ -832,6 +833,9 @@ pub struct ModuleS<'a> { shadowed_traits: RefCell>>, + glob_importers: RefCell, &'a ImportDirective)>>, + resolved_globs: RefCell<(Vec> /* public */, Vec> /* private */)>, + // The number of unresolved globs that this module exports. glob_count: Cell, @@ -866,6 +870,8 @@ fn new(parent_link: ParentLink<'a>, unresolved_imports: RefCell::new(Vec::new()), module_children: RefCell::new(NodeMap()), shadowed_traits: RefCell::new(Vec::new()), + glob_importers: RefCell::new(Vec::new()), + resolved_globs: RefCell::new((Vec::new(), Vec::new())), glob_count: Cell::new(0), pub_count: Cell::new(0), pub_glob_count: Cell::new(0), @@ -874,54 +880,11 @@ fn new(parent_link: ParentLink<'a>, } } - fn resolve_name(&self, name: Name, ns: Namespace, allow_private_imports: bool) - -> ResolveResult<&'a NameBinding<'a>> { - let glob_count = - if allow_private_imports { self.glob_count.get() } else { self.pub_glob_count.get() }; - - self.resolutions.borrow().get(&(name, ns)).cloned().unwrap_or_default().result(glob_count) - .and_then(|binding| { - let allowed = allow_private_imports || !binding.is_import() || binding.is_public(); - if allowed { Success(binding) } else { Failed(None) } - }) - } - - // Define the name or return the existing binding if there is a collision. - fn try_define_child(&self, name: Name, ns: Namespace, binding: NameBinding<'a>) - -> Result<(), &'a NameBinding<'a>> { - let binding = self.arenas.alloc_name_binding(binding); - let mut children = self.resolutions.borrow_mut(); - let resolution = children.entry((name, ns)).or_insert_with(Default::default); - - // FIXME #31379: We can use methods from imported traits shadowed by non-import items - if let Some(old_binding) = resolution.binding { - if !old_binding.is_import() && binding.is_import() { - if let Some(Def::Trait(_)) = binding.def() { - self.shadowed_traits.borrow_mut().push(binding); - } - } - } - - resolution.try_define(binding) - } - fn add_import_directive(&self, import_directive: ImportDirective) { let import_directive = self.arenas.alloc_import_directive(import_directive); self.unresolved_imports.borrow_mut().push(import_directive); } - fn increment_outstanding_references_for(&self, name: Name, ns: Namespace) { - let mut children = self.resolutions.borrow_mut(); - children.entry((name, ns)).or_insert_with(Default::default).outstanding_references += 1; - } - - fn decrement_outstanding_references_for(&self, name: Name, ns: Namespace) { - match self.resolutions.borrow_mut().get_mut(&(name, ns)).unwrap().outstanding_references { - 0 => panic!("No more outstanding references!"), - ref mut outstanding_references => { *outstanding_references -= 1; } - } - } - fn for_each_child)>(&self, mut f: F) { for (&(name, ns), name_resolution) in self.resolutions.borrow().iter() { name_resolution.binding.map(|binding| f(name, ns, binding)); diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 15c9528d7ef..226b6559e68 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -125,47 +125,163 @@ fn import<'a>(&self, /// Records information about the resolution of a name in a module. pub struct NameResolution<'a> { /// The number of unresolved single imports that could define the name. - pub outstanding_references: usize, + outstanding_references: usize, /// The least shadowable known binding for this name, or None if there are no known bindings. pub binding: Option<&'a NameBinding<'a>>, + duplicate_globs: Vec<&'a NameBinding<'a>>, } impl<'a> NameResolution<'a> { - pub fn result(&self, outstanding_globs: usize) -> ResolveResult<&'a NameBinding<'a>> { - // If no unresolved imports (single or glob) can define the name, self.binding is final. - if self.outstanding_references == 0 && outstanding_globs == 0 { - return self.binding.map(Success).unwrap_or(Failed(None)); - } - - if let Some(binding) = self.binding { - // Single imports will never be shadowable by other single or glob imports. - if !binding.defined_with(DefModifiers::GLOB_IMPORTED) { return Success(binding); } - // Non-PRELUDE glob imports will never be shadowable by other glob imports. - if self.outstanding_references == 0 && !binding.defined_with(DefModifiers::PRELUDE) { - return Success(binding); + fn try_define(&mut self, binding: &'a NameBinding<'a>) -> Result<(), &'a NameBinding<'a>> { + match self.binding { + Some(old_binding) if !old_binding.defined_with(DefModifiers::PRELUDE) => { + if binding.defined_with(DefModifiers::GLOB_IMPORTED) { + self.duplicate_globs.push(binding); + } else if old_binding.defined_with(DefModifiers::GLOB_IMPORTED) { + self.duplicate_globs.push(old_binding); + self.binding = Some(binding); + } else { + return Err(old_binding); + } } + _ => self.binding = Some(binding), } - Indeterminate + Ok(()) } - // Define the name or return the existing binding if there is a collision. - pub fn try_define(&mut self, binding: &'a NameBinding<'a>) -> Result<(), &'a NameBinding<'a>> { - let is_prelude = |binding: &NameBinding| binding.defined_with(DefModifiers::PRELUDE); - let old_binding = match self.binding { - Some(_) if is_prelude(binding) => return Ok(()), - Some(old_binding) if !is_prelude(old_binding) => old_binding, - _ => { self.binding = Some(binding); return Ok(()); } + // Returns the resolution of the name assuming no more globs will define it. + fn result(&self) -> ResolveResult<&'a NameBinding<'a>> { + match self.binding { + Some(binding) if !binding.defined_with(DefModifiers::GLOB_IMPORTED) => Success(binding), + _ if self.outstanding_references > 0 => Indeterminate, + Some(binding) => Success(binding), + None => Failed(None), + } + } + + // Returns Some(the resolution of the name), or None if the resolution depends + // on whether more globs can define the name. + fn try_result(&self) -> Option>> { + match self.result() { + Success(binding) if binding.defined_with(DefModifiers::PRELUDE) => None, + Failed(_) => None, + result @ _ => Some(result), + } + } + + fn report_conflicts(&self, mut report: F) { + let binding = match self.binding { + Some(binding) => binding, + None => return, }; - // FIXME #31337: We currently allow items to shadow glob-imported re-exports. - if !old_binding.is_import() && binding.defined_with(DefModifiers::GLOB_IMPORTED) { - if let NameBindingKind::Import { binding, .. } = binding.kind { - if binding.is_import() { return Ok(()); } + for duplicate_glob in self.duplicate_globs.iter() { + if duplicate_glob.defined_with(DefModifiers::PRELUDE) { continue } + + // FIXME #31337: We currently allow items to shadow glob-imported re-exports. + if !binding.is_import() { + if let NameBindingKind::Import { binding, .. } = duplicate_glob.kind { + if binding.is_import() { continue } + } + } + + report(duplicate_glob, binding); + } + } +} + +impl<'a> ::ModuleS<'a> { + pub fn resolve_name(&self, name: Name, ns: Namespace, allow_private_imports: bool) + -> ResolveResult<&'a NameBinding<'a>> { + let resolutions = match self.resolutions.borrow_state() { + ::std::cell::BorrowState::Unused => self.resolutions.borrow(), + _ => return Failed(None), // This happens when there is a cycle of glob imports + }; + + let resolution = resolutions.get(&(name, ns)).cloned().unwrap_or_default(); + if let Some(result) = resolution.try_result() { + // If the resolution doesn't depend on glob definability, check privacy and return. + return result.and_then(|binding| { + let allowed = allow_private_imports || !binding.is_import() || binding.is_public(); + if allowed { Success(binding) } else { Failed(None) } + }); + } + + let (ref mut public_globs, ref mut private_globs) = *self.resolved_globs.borrow_mut(); + + // Check if the public globs are determined + if self.pub_glob_count.get() > 0 { + return Indeterminate; + } + for module in public_globs.iter() { + if let Indeterminate = module.resolve_name(name, ns, false) { + return Indeterminate; + } + } + + if !allow_private_imports { + return Failed(None); + } + + // Check if the private globs are determined + if self.glob_count.get() > 0 { + return Indeterminate; + } + for module in private_globs.iter() { + if let Indeterminate = module.resolve_name(name, ns, false) { + return Indeterminate; + } + } + + resolution.result() + } + + // Define the name or return the existing binding if there is a collision. + pub fn try_define_child(&self, name: Name, ns: Namespace, binding: NameBinding<'a>) + -> Result<(), &'a NameBinding<'a>> { + if self.resolutions.borrow_state() != ::std::cell::BorrowState::Unused { return Ok(()); } + self.update_resolution(name, ns, |resolution| { + resolution.try_define(self.arenas.alloc_name_binding(binding)) + }) + } + + pub fn increment_outstanding_references_for(&self, name: Name, ns: Namespace) { + let mut resolutions = self.resolutions.borrow_mut(); + resolutions.entry((name, ns)).or_insert_with(Default::default).outstanding_references += 1; + } + + fn decrement_outstanding_references_for(&self, name: Name, ns: Namespace) { + self.update_resolution(name, ns, |resolution| match resolution.outstanding_references { + 0 => panic!("No more outstanding references!"), + ref mut outstanding_references => *outstanding_references -= 1, + }) + } + + // Use `update` to mutate the resolution for the name. + // If the resolution becomes a success, define it in the module's glob importers. + fn update_resolution(&self, name: Name, ns: Namespace, update: F) -> T + where F: FnOnce(&mut NameResolution<'a>) -> T + { + let mut resolutions = self.resolutions.borrow_mut(); + let resolution = resolutions.entry((name, ns)).or_insert_with(Default::default); + let was_success = resolution.try_result().and_then(ResolveResult::success).is_some(); + + let t = update(resolution); + if !was_success { + if let Some(Success(binding)) = resolution.try_result() { + self.define_in_glob_importers(name, ns, binding); } } + t + } - Err(old_binding) + fn define_in_glob_importers(&self, name: Name, ns: Namespace, binding: &'a NameBinding<'a>) { + if !binding.defined_with(DefModifiers::PUBLIC | DefModifiers::IMPORTABLE) { return } + if binding.is_extern_crate() { return } + for &(importer, directive) in self.glob_importers.borrow_mut().iter() { + let _ = importer.try_define_child(name, ns, directive.import(binding, None)); + } } } @@ -206,11 +322,13 @@ fn resolve_imports(&mut self) { if self.resolver.unresolved_imports == 0 { debug!("(resolving imports) success"); + self.finalize_resolutions(self.resolver.graph_root); break; } if self.resolver.unresolved_imports == prev_unresolved_imports { // resolving failed + self.finalize_resolutions(self.resolver.graph_root); if errors.len() > 0 { for e in errors { self.import_resolving_error(e) @@ -306,7 +424,7 @@ fn resolve_imports_for_module(&mut self, /// If successful, the resolved bindings are written into the module. fn resolve_import_for_module(&mut self, module_: Module<'b>, - import_directive: &ImportDirective) + import_directive: &'b ImportDirective) -> ResolveResult<()> { debug!("(resolving import for module) resolving import `{}::...` in `{}`", names_to_string(&import_directive.module_path), @@ -343,7 +461,7 @@ fn resolve_import_for_module(&mut self, fn resolve_import(&mut self, module_: Module<'b>, target_module: Module<'b>, - directive: &ImportDirective) + directive: &'b ImportDirective) -> ResolveResult<()> { let (source, target, value_determined, type_determined) = match directive.subclass { SingleImport { source, target, ref value_determined, ref type_determined } => @@ -418,32 +536,24 @@ fn resolve_import(&mut self, .emit(); } - (_, &Success(name_binding)) if !name_binding.is_import() && directive.is_public => { - if !name_binding.is_public() { - if name_binding.is_extern_crate() { - let msg = format!("extern crate `{}` is private, and cannot be reexported \ - (error E0364), consider declaring with `pub`", - source); - self.resolver.session.add_lint(lint::builtin::PRIVATE_IN_PUBLIC, - directive.id, - directive.span, - msg); - } else { - let msg = format!("`{}` is private, and cannot be reexported", source); - let note_msg = - format!("consider declaring type or module `{}` with `pub`", source); - struct_span_err!(self.resolver.session, directive.span, E0365, "{}", &msg) - .span_note(directive.span, ¬e_msg) - .emit(); - } - } else if name_binding.defined_with(DefModifiers::PRIVATE_VARIANT) { - let msg = format!("variant `{}` is private, and cannot be reexported \ - (error E0364), consider declaring its enum as `pub`", + (_, &Success(name_binding)) if !name_binding.is_import() && + directive.is_public && + !name_binding.is_public() => { + if name_binding.is_extern_crate() { + let msg = format!("extern crate `{}` is private, and cannot be reexported \ + (error E0364), consider declaring with `pub`", source); self.resolver.session.add_lint(lint::builtin::PRIVATE_IN_PUBLIC, directive.id, directive.span, msg); + } else { + let msg = format!("`{}` is private, and cannot be reexported", source); + let note_msg = + format!("consider declaring type or module `{}` with `pub`", source); + struct_span_err!(self.resolver.session, directive.span, E0365, "{}", &msg) + .span_note(directive.span, ¬e_msg) + .emit(); } } @@ -484,36 +594,29 @@ fn resolve_import(&mut self, fn resolve_glob_import(&mut self, module_: Module<'b>, target_module: Module<'b>, - directive: &ImportDirective) + directive: &'b ImportDirective) -> ResolveResult<()> { - // We must bail out if the node has unresolved imports of any kind (including globs). - if target_module.pub_count.get() > 0 { - debug!("(resolving glob import) target module has unresolved pub imports; bailing out"); - return Indeterminate; - } - if module_.def_id() == target_module.def_id() { // This means we are trying to glob import a module into itself, and it is a no-go let msg = "Cannot glob-import a module into itself.".into(); return Failed(Some((directive.span, msg))); } - - // Add all children from the containing module. build_reduced_graph::populate_module_if_necessary(self.resolver, target_module); - target_module.for_each_child(|name, ns, binding| { - if !binding.defined_with(DefModifiers::IMPORTABLE | DefModifiers::PUBLIC) { return } - self.define(module_, name, ns, directive.import(binding, None)); - - if ns == TypeNS && directive.is_public && - binding.defined_with(DefModifiers::PRIVATE_VARIANT) { - let msg = format!("variant `{}` is private, and cannot be reexported (error \ - E0364), consider declaring its enum as `pub`", name); - self.resolver.session.add_lint(lint::builtin::PRIVATE_IN_PUBLIC, - directive.id, - directive.span, - msg); + + // Add to target_module's glob_importers and module_'s resolved_globs + target_module.glob_importers.borrow_mut().push((module_, directive)); + match *module_.resolved_globs.borrow_mut() { + (ref mut public_globs, _) if directive.is_public => public_globs.push(target_module), + (_, ref mut private_globs) => private_globs.push(target_module), + } + + for (&(name, ns), resolution) in target_module.resolutions.borrow().iter() { + if let Some(Success(binding)) = resolution.try_result() { + if binding.defined_with(DefModifiers::IMPORTABLE | DefModifiers::PUBLIC) { + self.define(module_, name, ns, directive.import(binding, None)); + } } - }); + } // Record the destination of this import if let Some(did) = target_module.def_id() { @@ -535,12 +638,6 @@ fn define(&mut self, binding: NameBinding<'b>) { if let Err(old_binding) = parent.try_define_child(name, ns, binding.clone()) { self.report_conflict(name, ns, &binding, old_binding); - } else if binding.is_public() { // Add to the export map - if let (Some(parent_def_id), Some(def)) = (parent.def_id(), binding.def()) { - let parent_node_id = self.resolver.ast_map.as_local_node_id(parent_def_id).unwrap(); - let export = Export { name: name, def_id: def.def_id() }; - self.resolver.export_map.entry(parent_node_id).or_insert(Vec::new()).push(export); - } } } @@ -604,6 +701,58 @@ fn report_conflict(&mut self, err.emit(); } } + + // Miscellaneous post-processing, including recording reexports, recording shadowed traits, + // reporting conflicts, and reporting the PRIVATE_IN_PUBLIC lint. + fn finalize_resolutions(&mut self, module: Module<'b>) { + // Since import resolution is finished, globs will not define any more names. + module.pub_glob_count.set(0); module.glob_count.set(0); + *module.resolved_globs.borrow_mut() = (Vec::new(), Vec::new()); + + let mut reexports = Vec::new(); + for (&(name, ns), resolution) in module.resolutions.borrow().iter() { + resolution.report_conflicts(|b1, b2| self.report_conflict(name, ns, b1, b2)); + let binding = match resolution.binding { + Some(binding) => binding, + None => continue, + }; + + if binding.is_public() && (binding.is_import() || binding.is_extern_crate()) { + if let Some(def) = binding.def() { + reexports.push(Export { name: name, def_id: def.def_id() }); + } + } + + if let NameBindingKind::Import { binding: orig_binding, id, .. } = binding.kind { + if ns == TypeNS && binding.is_public() && + orig_binding.defined_with(DefModifiers::PRIVATE_VARIANT) { + let msg = format!("variant `{}` is private, and cannot be reexported \ + (error E0364), consider declaring its enum as `pub`", + name); + let lint = lint::builtin::PRIVATE_IN_PUBLIC; + self.resolver.session.add_lint(lint, id, binding.span.unwrap(), msg); + } + } + + // FIXME #31379: We can use methods from imported traits shadowed by non-import items + if !binding.is_import() { + for glob_binding in resolution.duplicate_globs.iter() { + module.shadowed_traits.borrow_mut().push(glob_binding); + } + } + } + + if reexports.len() > 0 { + if let Some(def_id) = module.def_id() { + let node_id = self.resolver.ast_map.as_local_node_id(def_id).unwrap(); + self.resolver.export_map.insert(node_id, reexports); + } + } + + for (_, child) in module.module_children.borrow().iter() { + self.finalize_resolutions(child); + } + } } fn import_path_to_string(names: &[Name], subclass: &ImportDirectiveSubclass) -> String {