From bfb832e7c886a720e0aa847ffd8500621a3152d5 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Thu, 17 Mar 2016 08:43:17 +0000 Subject: [PATCH] Add `SingleImports` and use it in place of `outstanding_references` and `pub_outstanding_references`. --- src/librustc_resolve/build_reduced_graph.rs | 43 +---- src/librustc_resolve/resolve_imports.rs | 175 ++++++++++++-------- 2 files changed, 114 insertions(+), 104 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 2480fd02bea1..10a8647906e3 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -14,8 +14,7 @@ //! any imports resolved. use DefModifiers; -use resolve_imports::ImportDirective; -use resolve_imports::ImportDirectiveSubclass::{self, SingleImport, GlobImport}; +use resolve_imports::ImportDirectiveSubclass::{self, GlobImport}; use Module; use Namespace::{self, TypeNS, ValueNS}; use {NameBinding, NameBindingKind}; @@ -28,7 +27,7 @@ use rustc::middle::def::*; use rustc::middle::def_id::{CRATE_DEF_INDEX, DefId}; use rustc::ty::VariantKind; -use syntax::ast::{Name, NodeId}; +use syntax::ast::Name; use syntax::attr::AttrMetaMethods; use syntax::parse::token::special_idents; use syntax::codemap::{Span, DUMMY_SP}; @@ -152,8 +151,8 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> { } let subclass = ImportDirectiveSubclass::single(binding, source_name); - self.build_import_directive(parent, - module_path, + self.unresolved_imports += 1; + parent.add_import_directive(module_path, subclass, view_path.span, item.id, @@ -203,8 +202,8 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> { } }; let subclass = ImportDirectiveSubclass::single(rename, name); - self.build_import_directive(parent, - module_path, + self.unresolved_imports += 1; + parent.add_import_directive(module_path, subclass, source_item.span, source_item.node.id(), @@ -213,8 +212,8 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> { } } ViewPathGlob(_) => { - self.build_import_directive(parent, - module_path, + self.unresolved_imports += 1; + parent.add_import_directive(module_path, GlobImport, view_path.span, item.id, @@ -521,32 +520,6 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> { } } - /// Creates and adds an import directive to the given module. - fn build_import_directive(&mut self, - module_: Module<'b>, - module_path: Vec, - subclass: ImportDirectiveSubclass, - span: Span, - id: NodeId, - is_public: bool, - is_prelude: bool) { - // Bump the reference count on the name. Or, if this is a glob, set - // the appropriate flag. - - match subclass { - SingleImport { target, .. } => { - module_.increment_outstanding_references_for(target, ValueNS, is_public); - module_.increment_outstanding_references_for(target, TypeNS, is_public); - } - GlobImport => {} - } - - let directive = - ImportDirective::new(module_path, subclass, span, id, is_public, is_prelude); - module_.add_import_directive(directive); - self.unresolved_imports += 1; - } - /// Ensures that the reduced graph rooted at the given external module /// is built, building it if it is not. pub fn populate_module_if_necessary(&mut self, module: Module<'b>) { diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 86cfa928a9d2..a13ecc26ad80 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -68,24 +68,6 @@ pub struct ImportDirective<'a> { } impl<'a> ImportDirective<'a> { - pub fn new(module_path: Vec, - subclass: ImportDirectiveSubclass, - span: Span, - id: NodeId, - is_public: bool, - is_prelude: bool) - -> Self { - ImportDirective { - module_path: module_path, - target_module: Cell::new(None), - subclass: subclass, - span: span, - id: id, - is_public: is_public, - is_prelude: is_prelude, - } - } - // 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(&self, binding: &'a NameBinding<'a>, privacy_error: Option>>) @@ -111,17 +93,52 @@ impl<'a> ImportDirective<'a> { } #[derive(Clone, Default)] -/// Records information about the resolution of a name in a module. +/// Records information about the resolution of a name in a namespace of a module. pub struct NameResolution<'a> { - /// The number of unresolved single imports of any visibility that could define the name. - outstanding_references: u32, - /// The number of unresolved `pub` single imports that could define the name. - pub_outstanding_references: u32, + /// The single imports that define the name in the namespace. + single_imports: SingleImports<'a>, /// 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>>, } +#[derive(Clone, Debug)] +enum SingleImports<'a> { + /// No single imports can define the name in the namespace. + None, + /// Only the given single import can define the name in the namespace. + MaybeOne(&'a ImportDirective<'a>), + /// At least one single import will define the name in the namespace. + AtLeastOne, +} + +impl<'a> Default for SingleImports<'a> { + fn default() -> Self { + SingleImports::None + } +} + +impl<'a> SingleImports<'a> { + fn add_directive(&mut self, directive: &'a ImportDirective<'a>) { + match *self { + SingleImports::None => *self = SingleImports::MaybeOne(directive), + // If two single imports can define the name in the namespace, we can assume that at + // least one of them will define it since otherwise both would have to define only one + // namespace, leading to a duplicate error. + SingleImports::MaybeOne(_) => *self = SingleImports::AtLeastOne, + SingleImports::AtLeastOne => {} + }; + } + + fn directive_failed(&mut self) { + match *self { + SingleImports::None => unreachable!(), + SingleImports::MaybeOne(_) => *self = SingleImports::None, + SingleImports::AtLeastOne => {} + } + } +} + impl<'a> NameResolution<'a> { fn try_define(&mut self, binding: &'a NameBinding<'a>) -> Result<(), &'a NameBinding<'a>> { if let Some(old_binding) = self.binding { @@ -140,40 +157,43 @@ impl<'a> NameResolution<'a> { Ok(()) } + // Returns the binding for the name if it is known or None if it not known. + fn binding(&self) -> Option<&'a NameBinding<'a>> { + self.binding.and_then(|binding| match self.single_imports { + SingleImports::None => Some(binding), + _ if !binding.defined_with(DefModifiers::GLOB_IMPORTED) => Some(binding), + _ => None, // The binding could be shadowed by a single import, so it is not known. + }) + } + // 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, allow_private_imports: bool) -> Option>> { match self.binding { Some(binding) if !binding.defined_with(DefModifiers::GLOB_IMPORTED) => - Some(Success(binding)), - // If (1) we don't allow private imports, (2) no public single import can define the - // name, and (3) no public glob has defined the name, the resolution depends on globs. - _ if !allow_private_imports && self.pub_outstanding_references == 0 && - !self.binding.map(NameBinding::is_public).unwrap_or(false) => None, - _ if self.outstanding_references > 0 => Some(Indeterminate), - Some(binding) => Some(Success(binding)), - None => None, - } - } - - fn increment_outstanding_references(&mut self, is_public: bool) { - self.outstanding_references += 1; - if is_public { - self.pub_outstanding_references += 1; - } - } - - fn decrement_outstanding_references(&mut self, is_public: bool) { - let decrement_references = |count: &mut _| { - assert!(*count > 0); - *count -= 1; + return Some(Success(binding)), + _ => {} // Items and single imports are not shadowable }; - decrement_references(&mut self.outstanding_references); - if is_public { - decrement_references(&mut self.pub_outstanding_references); + // Check if a single import can still define the name. + match self.single_imports { + SingleImports::None => {}, + SingleImports::AtLeastOne => return Some(Indeterminate), + SingleImports::MaybeOne(directive) => { + // If (1) we don't allow private imports, (2) no public single import can define + // the name, and (3) no public glob has defined the name, the resolution depends + // on whether more globs can define the name. + if !allow_private_imports && !directive.is_public && + !self.binding.map(NameBinding::is_public).unwrap_or(false) { + return None; + } + + return Indeterminate; + } } + + self.binding.map(Success) } fn report_conflicts(&self, mut report: F) { @@ -245,23 +265,39 @@ impl<'a> ::ModuleS<'a> { }) } - pub fn add_import_directive(&self, directive: ImportDirective<'a>) { - let directive = self.arenas.alloc_import_directive(directive); + pub fn add_import_directive(&self, + module_path: Vec, + subclass: ImportDirectiveSubclass, + span: Span, + id: NodeId, + is_public: bool, + is_prelude: bool) { + let directive = self.arenas.alloc_import_directive(ImportDirective { + module_path: module_path, + target_module: Cell::new(None), + subclass: subclass, + span: span, + id: id, + is_public: is_public, + is_prelude: is_prelude, + }); + self.unresolved_imports.borrow_mut().push(directive); - if let GlobImport = directive.subclass { + match directive.subclass { + SingleImport { target, .. } => { + let mut resolutions = self.resolutions.borrow_mut(); + for &ns in &[ValueNS, TypeNS] { + resolutions.entry((target, ns)).or_insert_with(Default::default) + .single_imports.add_directive(directive); + } + } // We don't add prelude imports to the globs since they only affect lexical scopes, // which are not relevant to import resolution. - if !directive.is_prelude { - self.globs.borrow_mut().push(directive); - } + GlobImport if directive.is_prelude => {} + GlobImport => self.globs.borrow_mut().push(directive), } } - pub fn increment_outstanding_references_for(&self, name: Name, ns: Namespace, is_public: bool) { - self.resolutions.borrow_mut().entry((name, ns)).or_insert_with(Default::default) - .increment_outstanding_references(is_public); - } - // 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 @@ -269,11 +305,11 @@ impl<'a> ::ModuleS<'a> { { let mut resolutions = self.resolutions.borrow_mut(); let resolution = resolutions.entry((name, ns)).or_insert_with(Default::default); - let was_success = resolution.try_result(false).and_then(ResolveResult::success).is_some(); + let was_known = resolution.binding().is_some(); let t = update(resolution); - if !was_success { - if let Some(Success(binding)) = resolution.try_result(false) { + if !was_known { + if let Some(binding) = resolution.binding() { self.define_in_glob_importers(name, ns, binding); } } @@ -454,12 +490,13 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { // (as opposed to being indeterminate) when it can only be defined by the directive. if !determined { module_.resolutions.borrow_mut().get_mut(&(target, ns)).unwrap() - .decrement_outstanding_references(directive.is_public); + .single_imports.directive_failed(); } let result = self.resolver.resolve_name_in_module(target_module, source, ns, false, true); if !determined { - module_.increment_outstanding_references_for(target, ns, directive.is_public) + module_.resolutions.borrow_mut().get_mut(&(target, ns)).unwrap() + .single_imports.add_directive(directive); } result }; @@ -491,11 +528,11 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { let binding = &directive.import(binding, None); self.resolver.report_conflict(module_, target, ns, binding, old_binding); } + } else { + module_.update_resolution(target, ns, |resolution| { + resolution.single_imports.directive_failed(); + }); } - - module_.update_resolution(target, ns, |resolution| { - resolution.decrement_outstanding_references(directive.is_public); - }) } match (&value_result, &type_result) { @@ -605,7 +642,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { target_module.glob_importers.borrow_mut().push((module_, directive)); for (&(name, ns), resolution) in target_module.resolutions.borrow().iter() { - if let Some(Success(binding)) = resolution.try_result(false) { + if let Some(binding) = resolution.binding() { if binding.defined_with(DefModifiers::IMPORTABLE | DefModifiers::PUBLIC) { let _ = module_.try_define_child(name, ns, directive.import(binding, None)); }