From d107d2259098a2d683bb7c92876ec0abdf4716ca Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Wed, 17 Aug 2016 00:22:03 +0000 Subject: [PATCH 01/18] Refactor `module.add_import_directive()` -> `resolver.add_import_directive()`. --- src/librustc_resolve/build_reduced_graph.rs | 6 +++--- src/librustc_resolve/resolve_imports.rs | 15 ++++++++------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 116c1b7a6d06f..ea946d0117b66 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -130,7 +130,7 @@ impl<'b> Resolver<'b> { let subclass = ImportDirectiveSubclass::single(binding.name, source_name); let span = view_path.span; - parent.add_import_directive(module_path, subclass, span, item.id, vis); + self.add_import_directive(module_path, subclass, span, item.id, vis); self.unresolved_imports += 1; } ViewPathList(_, ref source_items) => { @@ -176,14 +176,14 @@ impl<'b> Resolver<'b> { }; let subclass = ImportDirectiveSubclass::single(rename, name); let (span, id) = (source_item.span, source_item.node.id()); - parent.add_import_directive(module_path, subclass, span, id, vis); + self.add_import_directive(module_path, subclass, span, id, vis); self.unresolved_imports += 1; } } ViewPathGlob(_) => { let subclass = GlobImport { is_prelude: is_prelude }; let span = view_path.span; - parent.add_import_directive(module_path, subclass, span, item.id, vis); + self.add_import_directive(module_path, subclass, span, item.id, vis); self.unresolved_imports += 1; } } diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index e08a30e40d354..a0539206fa42f 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -212,8 +212,11 @@ impl<'a> ::ModuleS<'a> { Failed(None) } +} - pub fn add_import_directive(&self, +impl<'a> Resolver<'a> { + // Add an import directive to the current module. + pub fn add_import_directive(&mut self, module_path: Vec, subclass: ImportDirectiveSubclass, span: Span, @@ -228,23 +231,21 @@ impl<'a> ::ModuleS<'a> { vis: vis, }); - self.unresolved_imports.borrow_mut().push(directive); + self.current_module.unresolved_imports.borrow_mut().push(directive); match directive.subclass { SingleImport { target, .. } => { for &ns in &[ValueNS, TypeNS] { - self.resolution(target, ns).borrow_mut().single_imports - .add_directive(directive); + let mut resolution = self.current_module.resolution(target, ns).borrow_mut(); + resolution.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. GlobImport { is_prelude: true } => {} - GlobImport { .. } => self.globs.borrow_mut().push(directive), + GlobImport { .. } => self.current_module.globs.borrow_mut().push(directive), } } -} -impl<'a> Resolver<'a> { // 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>) From c64cd86be866242a88bb1c103dfddf407ebdadde Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Wed, 17 Aug 2016 00:42:14 +0000 Subject: [PATCH 02/18] Add field `parent` to `ImportDirective`. --- src/librustc_resolve/lib.rs | 2 +- src/librustc_resolve/resolve_imports.rs | 23 ++++++++++------------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index d90a932a63d86..742d955e38a2b 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -756,7 +756,7 @@ pub struct ModuleS<'a> { no_implicit_prelude: Cell, - glob_importers: RefCell, &'a ImportDirective<'a>)>>, + glob_importers: RefCell>>, globs: RefCell>>, // Used to memoize the traits in this module for faster searches through all traits in scope. diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index a0539206fa42f..83b1f64a33a9a 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -63,6 +63,7 @@ impl ImportDirectiveSubclass { #[derive(Debug,Clone)] pub struct ImportDirective<'a> { pub id: NodeId, + parent: Module<'a>, module_path: Vec, target_module: Cell>>, // the resolution of `module_path` subclass: ImportDirectiveSubclass, @@ -223,6 +224,7 @@ impl<'a> Resolver<'a> { id: NodeId, vis: ty::Visibility) { let directive = self.arenas.alloc_import_directive(ImportDirective { + parent: self.current_module, module_path: module_path, target_module: Cell::new(None), subclass: subclass, @@ -306,9 +308,9 @@ impl<'a> Resolver<'a> { // Define `new_binding` in `module`s glob importers. if new_binding.is_importable() && new_binding.is_pseudo_public() { - for &(importer, directive) in module.glob_importers.borrow_mut().iter() { + for directive in module.glob_importers.borrow_mut().iter() { let imported_binding = self.import(new_binding, directive); - let _ = self.try_define(importer, name, ns, imported_binding); + let _ = self.try_define(directive.parent, name, ns, imported_binding); } } @@ -317,8 +319,6 @@ impl<'a> Resolver<'a> { } struct ImportResolvingError<'a> { - /// Module where the error happened - source_module: Module<'a>, import_directive: &'a ImportDirective<'a>, span: Span, help: String, @@ -402,9 +402,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { // Define a "dummy" resolution containing a Def::Err as a placeholder for a // failed resolution - fn import_dummy_binding(&mut self, - source_module: Module<'b>, - directive: &'b ImportDirective<'b>) { + fn import_dummy_binding(&mut self, directive: &'b ImportDirective<'b>) { if let SingleImport { target, .. } = directive.subclass { let dummy_binding = self.arenas.alloc_name_binding(NameBinding { kind: NameBindingKind::Def(Def::Err), @@ -413,8 +411,8 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { }); let dummy_binding = self.import(dummy_binding, directive); - let _ = self.try_define(source_module, target, ValueNS, dummy_binding.clone()); - let _ = self.try_define(source_module, target, TypeNS, dummy_binding); + let _ = self.try_define(directive.parent, target, ValueNS, dummy_binding.clone()); + let _ = self.try_define(directive.parent, target, TypeNS, dummy_binding); } } @@ -423,7 +421,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { fn import_resolving_error(&mut self, e: ImportResolvingError<'b>) { // If the error is a single failed import then create a "fake" import // resolution for it so that later resolve stages won't complain. - self.import_dummy_binding(e.source_module, e.import_directive); + self.import_dummy_binding(e.import_directive); let path = import_path_to_string(&e.import_directive.module_path, &e.import_directive.subclass); resolve_error(self.resolver, @@ -445,7 +443,6 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { None => (import_directive.span, String::new()), }; errors.push(ImportResolvingError { - source_module: self.current_module, import_directive: import_directive, span: span, help: help, @@ -511,7 +508,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { .emit(); // Do not import this illegal binding. Import a dummy binding and pretend // everything is fine - self.import_dummy_binding(module, directive); + self.import_dummy_binding(directive); return Success(()); } Success(binding) if !self.is_accessible(binding.vis) => {} @@ -635,7 +632,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { } // Add to target_module's glob_importers - target_module.glob_importers.borrow_mut().push((module, directive)); + target_module.glob_importers.borrow_mut().push(directive); // Ensure that `resolutions` isn't borrowed during `try_define`, // since it might get updated via a glob cycle. From 37154ca95d801c83974e23b913154da402ae5d79 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Wed, 17 Aug 2016 00:52:18 +0000 Subject: [PATCH 03/18] Refactor `unresolved_imports` -> `indeterminate_imports`. --- src/librustc_resolve/build_reduced_graph.rs | 3 - src/librustc_resolve/lib.rs | 8 +- src/librustc_resolve/resolve_imports.rs | 113 ++++++++------------ 3 files changed, 47 insertions(+), 77 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index ea946d0117b66..2dac64ad2bba9 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -131,7 +131,6 @@ impl<'b> Resolver<'b> { let subclass = ImportDirectiveSubclass::single(binding.name, source_name); let span = view_path.span; self.add_import_directive(module_path, subclass, span, item.id, vis); - self.unresolved_imports += 1; } ViewPathList(_, ref source_items) => { // Make sure there's at most one `mod` import in the list. @@ -177,14 +176,12 @@ impl<'b> Resolver<'b> { let subclass = ImportDirectiveSubclass::single(rename, name); let (span, id) = (source_item.span, source_item.node.id()); self.add_import_directive(module_path, subclass, span, id, vis); - self.unresolved_imports += 1; } } ViewPathGlob(_) => { let subclass = GlobImport { is_prelude: is_prelude }; let span = view_path.span; self.add_import_directive(module_path, subclass, span, item.id, vis); - self.unresolved_imports += 1; } } } diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 742d955e38a2b..1660a270cbd4f 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -752,7 +752,6 @@ pub struct ModuleS<'a> { extern_crate_id: Option, resolutions: RefCell>>>, - unresolved_imports: RefCell>>, no_implicit_prelude: Cell, @@ -782,7 +781,6 @@ impl<'a> ModuleS<'a> { def: def, extern_crate_id: None, resolutions: RefCell::new(HashMap::new()), - unresolved_imports: RefCell::new(Vec::new()), no_implicit_prelude: Cell::new(false), glob_importers: RefCell::new(Vec::new()), globs: RefCell::new((Vec::new())), @@ -965,8 +963,8 @@ pub struct Resolver<'a> { structs: FnvHashMap>, - // The number of imports that are currently unresolved. - unresolved_imports: usize, + // All indeterminate imports (i.e. imports not known to succeed or fail). + indeterminate_imports: Vec<&'a ImportDirective<'a>>, // The module that represents the current item scope. current_module: Module<'a>, @@ -1153,7 +1151,7 @@ impl<'a> Resolver<'a> { trait_item_map: FnvHashMap(), structs: FnvHashMap(), - unresolved_imports: 0, + indeterminate_imports: Vec::new(), current_module: graph_root, value_ribs: vec![Rib::new(ModuleRibKind(graph_root))], diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 83b1f64a33a9a..23a5b3c499011 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -233,7 +233,7 @@ impl<'a> Resolver<'a> { vis: vis, }); - self.current_module.unresolved_imports.borrow_mut().push(directive); + self.indeterminate_imports.push(directive); match directive.subclass { SingleImport { target, .. } => { for &ns in &[ValueNS, TypeNS] { @@ -360,43 +360,52 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { /// point iteration. fn resolve_imports(&mut self) { let mut i = 0; - let mut prev_unresolved_imports = 0; + let mut prev_num_indeterminates = self.indeterminate_imports.len() + 1; let mut errors = Vec::new(); - loop { - debug!("(resolving imports) iteration {}, {} imports left", i, self.unresolved_imports); - - // Attempt to resolve imports in all local modules. - for module in self.arenas.local_modules().iter() { - self.current_module = module; - self.resolve_imports_in_current_module(&mut errors); - } - - if self.unresolved_imports == 0 { - debug!("(resolving imports) success"); - for module in self.arenas.local_modules().iter() { - self.finalize_resolutions_in(module, false); + while self.indeterminate_imports.len() < prev_num_indeterminates { + prev_num_indeterminates = self.indeterminate_imports.len(); + debug!("(resolving imports) iteration {}, {} imports left", i, prev_num_indeterminates); + + let mut imports = Vec::new(); + ::std::mem::swap(&mut imports, &mut self.indeterminate_imports); + + for import in imports { + match self.resolve_import(&import) { + Failed(err) => { + let (span, help) = match err { + Some((span, msg)) => (span, format!(". {}", msg)), + None => (import.span, String::new()), + }; + errors.push(ImportResolvingError { + import_directive: import, + span: span, + help: help, + }); + } + Indeterminate => self.indeterminate_imports.push(import), + Success(()) => {} } - break; } - if self.unresolved_imports == prev_unresolved_imports { - // resolving failed - // Report unresolved imports only if no hard error was already reported - // to avoid generating multiple errors on the same import. - // Imports that are still indeterminate at this point are actually blocked - // by errored imports, so there is no point reporting them. - for module in self.arenas.local_modules().iter() { - self.finalize_resolutions_in(module, errors.len() == 0); - } - for e in errors { - self.import_resolving_error(e) - } - break; + i += 1; + } + + for module in self.arenas.local_modules().iter() { + self.finalize_resolutions_in(module); + } + + // Report unresolved imports only if no hard error was already reported + // to avoid generating multiple errors on the same import. + if errors.len() == 0 { + if let Some(import) = self.indeterminate_imports.iter().next() { + let error = ResolutionError::UnresolvedImport(None); + resolve_error(self.resolver, import.span, error); } + } - i += 1; - prev_unresolved_imports = self.unresolved_imports; + for e in errors { + self.import_resolving_error(e) } } @@ -429,35 +438,6 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { ResolutionError::UnresolvedImport(Some((&path, &e.help)))); } - /// Attempts to resolve imports for the given module only. - fn resolve_imports_in_current_module(&mut self, errors: &mut Vec>) { - let mut imports = Vec::new(); - let mut unresolved_imports = self.current_module.unresolved_imports.borrow_mut(); - ::std::mem::swap(&mut imports, &mut unresolved_imports); - - for import_directive in imports { - match self.resolve_import(&import_directive) { - Failed(err) => { - let (span, help) = match err { - Some((span, msg)) => (span, format!(". {}", msg)), - None => (import_directive.span, String::new()), - }; - errors.push(ImportResolvingError { - import_directive: import_directive, - span: span, - help: help, - }); - } - Indeterminate => unresolved_imports.push(import_directive), - Success(()) => { - // Decrement the count of unresolved imports. - assert!(self.unresolved_imports >= 1); - self.unresolved_imports -= 1; - } - } - } - } - /// Attempts to resolve the given import. The return value indicates /// failure if we're certain the name does not exist, indeterminate if we /// don't know whether the name exists at the moment due to other @@ -468,6 +448,9 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { names_to_string(&directive.module_path), module_to_string(self.current_module)); + let module = directive.parent; + self.current_module = module; + let target_module = match directive.target_module.get() { Some(module) => module, _ => match self.resolve_module_path(&directive.module_path, @@ -490,7 +473,6 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { let value_result = self.resolve_name_in_module(target_module, source, ValueNS, false, true); let type_result = self.resolve_name_in_module(target_module, source, TypeNS, false, true); - let module = self.current_module; let mut privacy_error = true; for &(ns, result, determined) in &[(ValueNS, &value_result, value_determined), (TypeNS, &type_result, type_determined)] { @@ -658,7 +640,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { // Miscellaneous post-processing, including recording reexports, reporting conflicts, // reporting the PRIVATE_IN_PUBLIC lint, and reporting unresolved imports. - fn finalize_resolutions_in(&mut self, module: Module<'b>, report_unresolved_imports: bool) { + fn finalize_resolutions_in(&mut self, module: Module<'b>) { // Since import resolution is finished, globs will not define any more names. *module.globs.borrow_mut() = Vec::new(); @@ -706,13 +688,6 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { self.export_map.insert(node_id, reexports); } } - - if report_unresolved_imports { - for import in module.unresolved_imports.borrow().iter() { - resolve_error(self.resolver, import.span, ResolutionError::UnresolvedImport(None)); - break; - } - } } } From c1362d8cc53176e0bf0db73fefc98ca9b9035457 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Mon, 15 Aug 2016 01:08:31 +0000 Subject: [PATCH 04/18] Clean up `build_reduced_graph.rs`. --- src/librustc_resolve/build_reduced_graph.rs | 44 +++++++++------------ 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 2dac64ad2bba9..3b3058c6da7be 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -56,12 +56,7 @@ impl<'b> Resolver<'b> { pub fn build_reduced_graph(&mut self, krate: &Crate) { let no_implicit_prelude = attr::contains_name(&krate.attrs, "no_implicit_prelude"); self.graph_root.no_implicit_prelude.set(no_implicit_prelude); - - let mut visitor = BuildReducedGraphVisitor { - parent: self.graph_root, - resolver: self, - }; - visit::walk_crate(&mut visitor, krate); + visit::walk_crate(&mut BuildReducedGraphVisitor { resolver: self }, krate); } /// Defines `name` in namespace `ns` of module `parent` to be `def` if it is not yet defined; @@ -84,11 +79,10 @@ impl<'b> Resolver<'b> { } /// Constructs the reduced graph for one item. - fn build_reduced_graph_for_item(&mut self, item: &Item, parent_ref: &mut Module<'b>) { - let parent = *parent_ref; + fn build_reduced_graph_for_item(&mut self, item: &Item) { + let parent = self.current_module; let name = item.ident.name; let sp = item.span; - self.current_module = parent; let vis = self.resolve_visibility(&item.vis); match item.node { @@ -213,7 +207,7 @@ impl<'b> Resolver<'b> { }); self.define(parent, name, TypeNS, (module, sp, vis)); self.module_map.insert(item.id, module); - *parent_ref = module; + self.current_module = module; // Descend into the module. } ItemKind::ForeignMod(..) => {} @@ -306,6 +300,9 @@ impl<'b> Resolver<'b> { } ItemKind::Mac(_) => panic!("unexpanded macro in resolve!"), } + + visit::walk_item(&mut BuildReducedGraphVisitor { resolver: self }, item); + self.current_module = parent; } // Constructs the reduced graph for one variant. Variants exist in the @@ -330,9 +327,8 @@ impl<'b> Resolver<'b> { } /// Constructs the reduced graph for one foreign item. - fn build_reduced_graph_for_foreign_item(&mut self, - foreign_item: &ForeignItem, - parent: Module<'b>) { + fn build_reduced_graph_for_foreign_item(&mut self, foreign_item: &ForeignItem) { + let parent = self.current_module; let name = foreign_item.ident.name; let def = match foreign_item.node { @@ -343,12 +339,12 @@ impl<'b> Resolver<'b> { Def::Static(self.definitions.local_def_id(foreign_item.id), m) } }; - self.current_module = parent; let vis = self.resolve_visibility(&foreign_item.vis); self.define(parent, name, ValueNS, (def, foreign_item.span, vis)); } - fn build_reduced_graph_for_block(&mut self, block: &Block, parent: &mut Module<'b>) { + fn build_reduced_graph_for_block(&mut self, block: &Block) { + let parent = self.current_module; if self.block_needs_anonymous_module(block) { let block_id = block.id; @@ -359,8 +355,11 @@ impl<'b> Resolver<'b> { let parent_link = BlockParentLink(parent, block_id); let new_module = self.new_module(parent_link, None, false); self.module_map.insert(block_id, new_module); - *parent = new_module; + self.current_module = new_module; // Descend into the block. } + + visit::walk_block(&mut BuildReducedGraphVisitor { resolver: self }, block); + self.current_module = parent; } /// Builds the reduced graph for a single item in an external crate. @@ -484,25 +483,18 @@ impl<'b> Resolver<'b> { struct BuildReducedGraphVisitor<'a, 'b: 'a> { resolver: &'a mut Resolver<'b>, - parent: Module<'b>, } impl<'a, 'b> Visitor for BuildReducedGraphVisitor<'a, 'b> { fn visit_item(&mut self, item: &Item) { - let old_parent = self.parent; - self.resolver.build_reduced_graph_for_item(item, &mut self.parent); - visit::walk_item(self, item); - self.parent = old_parent; + self.resolver.build_reduced_graph_for_item(item); } fn visit_foreign_item(&mut self, foreign_item: &ForeignItem) { - self.resolver.build_reduced_graph_for_foreign_item(foreign_item, &self.parent); + self.resolver.build_reduced_graph_for_foreign_item(foreign_item); } fn visit_block(&mut self, block: &Block) { - let old_parent = self.parent; - self.resolver.build_reduced_graph_for_block(block, &mut self.parent); - visit::walk_block(self, block); - self.parent = old_parent; + self.resolver.build_reduced_graph_for_block(block); } } From 89de52eff08d7416b9fd4ab0adc2e818590e84d0 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Sun, 14 Aug 2016 23:42:05 +0000 Subject: [PATCH 05/18] Add field `current_vis` to `Resolver`. --- src/librustc_resolve/build_reduced_graph.rs | 7 ++++++- src/librustc_resolve/lib.rs | 23 ++++++++++----------- src/librustc_resolve/resolve_imports.rs | 10 ++++++++- 3 files changed, 26 insertions(+), 14 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 3b3058c6da7be..579853446525e 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -81,6 +81,7 @@ impl<'b> Resolver<'b> { /// Constructs the reduced graph for one item. fn build_reduced_graph_for_item(&mut self, item: &Item) { let parent = self.current_module; + let parent_vis = self.current_vis; let name = item.ident.name; let sp = item.span; let vis = self.resolve_visibility(&item.vis); @@ -207,7 +208,10 @@ impl<'b> Resolver<'b> { }); self.define(parent, name, TypeNS, (module, sp, vis)); self.module_map.insert(item.id, module); - self.current_module = module; // Descend into the module. + + // Descend into the module. + self.current_module = module; + self.current_vis = ty::Visibility::Restricted(item.id); } ItemKind::ForeignMod(..) => {} @@ -303,6 +307,7 @@ impl<'b> Resolver<'b> { visit::walk_item(&mut BuildReducedGraphVisitor { resolver: self }, item); self.current_module = parent; + self.current_vis = parent_vis; } // Constructs the reduced graph for one variant. Variants exist in the diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 1660a270cbd4f..08cfc662e9bea 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -969,6 +969,10 @@ pub struct Resolver<'a> { // The module that represents the current item scope. current_module: Module<'a>, + // The visibility of `pub(self)` items in the current scope. + // Equivalently, the visibility required for an item to be accessible from the current scope. + current_vis: ty::Visibility, + // The current set of local scopes, for values. // FIXME #4948: Reuse ribs to avoid allocation. value_ribs: Vec>, @@ -1154,6 +1158,7 @@ impl<'a> Resolver<'a> { indeterminate_imports: Vec::new(), current_module: graph_root, + current_vis: ty::Visibility::Restricted(ast::CRATE_NODE_ID), value_ribs: vec![Rib::new(ModuleRibKind(graph_root))], type_ribs: vec![Rib::new(ModuleRibKind(graph_root))], label_ribs: Vec::new(), @@ -1197,6 +1202,7 @@ impl<'a> Resolver<'a> { /// Entry point to crate resolution. pub fn resolve_crate(&mut self, krate: &Crate) { self.current_module = self.graph_root; + self.current_vis = ty::Visibility::Restricted(ast::CRATE_NODE_ID); visit::walk_crate(self, krate); check_unused::check_crate(self, krate); @@ -1562,13 +1568,15 @@ impl<'a> Resolver<'a> { let module = self.module_map.get(&id).cloned(); // clones a reference if let Some(module) = module { // Move down in the graph. - let orig_module = ::std::mem::replace(&mut self.current_module, module); + let orig_module = replace(&mut self.current_module, module); + let orig_vis = replace(&mut self.current_vis, ty::Visibility::Restricted(id)); self.value_ribs.push(Rib::new(ModuleRibKind(module))); self.type_ribs.push(Rib::new(ModuleRibKind(module))); f(self); self.current_module = orig_module; + self.current_vis = orig_vis; self.value_ribs.pop(); self.type_ribs.pop(); } else { @@ -2706,7 +2714,6 @@ impl<'a> Resolver<'a> { fn with_empty_ribs(&mut self, f: F) -> T where F: FnOnce(&mut Resolver<'a>) -> T, { - use ::std::mem::replace; let value_ribs = replace(&mut self.value_ribs, Vec::new()); let type_ribs = replace(&mut self.type_ribs, Vec::new()); let label_ribs = replace(&mut self.label_ribs, Vec::new()); @@ -3264,13 +3271,7 @@ impl<'a> Resolver<'a> { ast::Visibility::Public => return ty::Visibility::Public, ast::Visibility::Crate(_) => return ty::Visibility::Restricted(ast::CRATE_NODE_ID), ast::Visibility::Restricted { ref path, id } => (path, id), - ast::Visibility::Inherited => { - let current_module = - self.get_nearest_normal_module_parent_or_self(self.current_module); - let id = - self.definitions.as_local_node_id(current_module.def_id().unwrap()).unwrap(); - return ty::Visibility::Restricted(id); - } + ast::Visibility::Inherited => return self.current_vis, }; let segments: Vec<_> = path.segments.iter().map(|seg| seg.identifier.name).collect(); @@ -3299,9 +3300,7 @@ impl<'a> Resolver<'a> { } fn is_accessible(&self, vis: ty::Visibility) -> bool { - let current_module = self.get_nearest_normal_module_parent_or_self(self.current_module); - let node_id = self.definitions.as_local_node_id(current_module.def_id().unwrap()).unwrap(); - vis.is_accessible_from(node_id, self) + vis.is_at_least(self.current_vis, self) } fn check_privacy(&mut self, name: Name, binding: &'a NameBinding<'a>, span: Span) { diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 23a5b3c499011..17933abec27b0 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -356,6 +356,14 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { // remain or unsuccessfully when no forward progress in resolving imports // is made. + fn set_current_module(&mut self, module: Module<'b>) { + self.current_module = module; + self.current_vis = ty::Visibility::Restricted({ + let normal_module = self.get_nearest_normal_module_parent_or_self(module); + self.definitions.as_local_node_id(normal_module.def_id().unwrap()).unwrap() + }); + } + /// Resolves all imports for the crate. This method performs the fixed- /// point iteration. fn resolve_imports(&mut self) { @@ -449,7 +457,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { module_to_string(self.current_module)); let module = directive.parent; - self.current_module = module; + self.set_current_module(module); let target_module = match directive.target_module.get() { Some(module) => module, From bfc98f59a48d7c8a65cd1a15656bb6165fb3589f Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Mon, 1 Aug 2016 20:43:48 +0000 Subject: [PATCH 06/18] Refactor away `module.resolve_name()`. --- src/librustc_resolve/lib.rs | 37 ++------ src/librustc_resolve/resolve_imports.rs | 115 ++++++++++++++---------- 2 files changed, 75 insertions(+), 77 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 08cfc662e9bea..71dee48f3396d 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1250,12 +1250,13 @@ impl<'a> Resolver<'a> { index: usize, span: Span) -> ResolveResult> { - fn search_parent_externals(needle: Name, module: Module) -> Option { - match module.resolve_name(needle, TypeNS, false) { + fn search_parent_externals<'a>(this: &mut Resolver<'a>, needle: Name, module: Module<'a>) + -> Option> { + match this.resolve_name_in_module(module, needle, TypeNS, false, false) { Success(binding) if binding.is_extern_crate() => Some(module), _ => match module.parent_link { ModuleParentLink(ref parent, _) => { - search_parent_externals(needle, parent) + search_parent_externals(this, needle, parent) } _ => None, }, @@ -1275,11 +1276,12 @@ impl<'a> Resolver<'a> { let segment_name = name.as_str(); let module_name = module_to_string(search_module); let msg = if "???" == &module_name { - match search_parent_externals(name, &self.current_module) { + let current_module = self.current_module; + match search_parent_externals(self, name, current_module) { Some(module) => { let path_str = names_to_string(module_path); let target_mod_str = module_to_string(&module); - let current_mod_str = module_to_string(&self.current_module); + let current_mod_str = module_to_string(current_module); let prefix = if target_mod_str == current_mod_str { "self::".to_string() @@ -1436,8 +1438,8 @@ impl<'a> Resolver<'a> { if module.def.is_some() { return match self.prelude { Some(prelude) if !module.no_implicit_prelude.get() => { - prelude.resolve_name(name, ns, false).success() - .map(LexicalScopeBinding::Item) + self.resolve_name_in_module(prelude, name, ns, false, false).success() + .map(LexicalScopeBinding::Item) } _ => None, }; @@ -1523,27 +1525,6 @@ impl<'a> Resolver<'a> { return Success(PrefixFound(containing_module, i)); } - /// Attempts to resolve the supplied name in the given module for the - /// given namespace. If successful, returns the binding corresponding to - /// the name. - fn resolve_name_in_module(&mut self, - module: Module<'a>, - name: Name, - namespace: Namespace, - use_lexical_scope: bool, - record_used: bool) - -> ResolveResult<&'a NameBinding<'a>> { - debug!("(resolving name in module) resolving `{}` in `{}`", name, module_to_string(module)); - - self.populate_module_if_necessary(module); - module.resolve_name(name, namespace, use_lexical_scope).and_then(|binding| { - if record_used { - self.record_use(name, namespace, binding); - } - Success(binding) - }) - } - // AST resolution // // We maintain a list of value ribs and type ribs. diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 17933abec27b0..0905632d4e627 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -133,19 +133,77 @@ impl<'a> NameResolution<'a> { _ => None, // The binding could be shadowed by a single import, so it is not known. }) } +} + +impl<'a> ::ModuleS<'a> { + fn resolution(&self, name: Name, ns: Namespace) -> &'a RefCell> { + *self.resolutions.borrow_mut().entry((name, ns)) + .or_insert_with(|| self.arenas.alloc_name_resolution()) + } +} + +impl<'a> Resolver<'a> { + /// Attempts to resolve the supplied name in the given module for the given namespace. + /// If successful, returns the binding corresponding to the name. + pub fn resolve_name_in_module(&mut self, + module: Module<'a>, + name: Name, + ns: Namespace, + allow_private_imports: bool, + record_used: bool) + -> ResolveResult<&'a NameBinding<'a>> { + self.populate_module_if_necessary(module); + + let resolution = module.resolution(name, ns); + let resolution = match resolution.borrow_state() { + ::std::cell::BorrowState::Unused => resolution.borrow_mut(), + _ => return Failed(None), // This happens when there is a cycle of imports + }; + + if let Some(result) = self.try_result(&resolution, ns, allow_private_imports) { + // If the resolution doesn't depend on glob definability, check privacy and return. + return result.and_then(|binding| { + if !allow_private_imports && binding.is_import() && !binding.is_pseudo_public() { + return Failed(None); + } + if record_used { + self.record_use(name, ns, binding); + } + Success(binding) + }); + } + + // Check if the globs are determined + for directive in module.globs.borrow().iter() { + if !allow_private_imports && directive.vis != ty::Visibility::Public { continue } + if let Some(target_module) = directive.target_module.get() { + let result = self.resolve_name_in_module(target_module, name, ns, false, false); + if let Indeterminate = result { + return Indeterminate; + } + } else { + return Indeterminate; + } + } + + 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, ns: Namespace, allow_private_imports: bool) + fn try_result(&mut self, + resolution: &NameResolution<'a>, + ns: Namespace, + allow_private_imports: bool) -> Option>> { - match self.binding { + match resolution.binding { Some(binding) if !binding.is_glob_import() => - return Some(Success(binding)), - _ => {} // Items and single imports are not shadowable + return Some(Success(binding)), // Items and single imports are not shadowable. + _ => {} }; // Check if a single import can still define the name. - match self.single_imports { + match resolution.single_imports { SingleImports::None => {}, SingleImports::AtLeastOne => return Some(Indeterminate), SingleImports::MaybeOne(directive) => { @@ -153,7 +211,7 @@ impl<'a> NameResolution<'a> { // 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.vis != ty::Visibility::Public && - !self.binding.map(NameBinding::is_pseudo_public).unwrap_or(false) { + !resolution.binding.map(NameBinding::is_pseudo_public).unwrap_or(false) { return None; } @@ -165,57 +223,16 @@ impl<'a> NameResolution<'a> { SingleImport { source, .. } => source, GlobImport { .. } => unreachable!(), }; - match target_module.resolve_name(name, ns, false) { + match self.resolve_name_in_module(target_module, name, ns, false, false) { Failed(_) => {} _ => return Some(Indeterminate), } } } - self.binding.map(Success) - } -} - -impl<'a> ::ModuleS<'a> { - fn resolution(&self, name: Name, ns: Namespace) -> &'a RefCell> { - *self.resolutions.borrow_mut().entry((name, ns)) - .or_insert_with(|| self.arenas.alloc_name_resolution()) - } - - pub fn resolve_name(&self, name: Name, ns: Namespace, allow_private_imports: bool) - -> ResolveResult<&'a NameBinding<'a>> { - let resolution = self.resolution(name, ns); - let resolution = match resolution.borrow_state() { - ::std::cell::BorrowState::Unused => resolution.borrow_mut(), - _ => return Failed(None), // This happens when there is a cycle of imports - }; - - if let Some(result) = resolution.try_result(ns, allow_private_imports) { - // 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_pseudo_public(); - if allowed { Success(binding) } else { Failed(None) } - }); - } - - // Check if the globs are determined - for directive in self.globs.borrow().iter() { - if !allow_private_imports && directive.vis != ty::Visibility::Public { continue } - match directive.target_module.get() { - None => return Indeterminate, - Some(target_module) => match target_module.resolve_name(name, ns, false) { - Indeterminate => return Indeterminate, - _ => {} - } - } - } - - Failed(None) + resolution.binding.map(Success) } -} -impl<'a> Resolver<'a> { // Add an import directive to the current module. pub fn add_import_directive(&mut self, module_path: Vec, From 75c3fd89d47d6da340ef6c1b98e80968d0218337 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Wed, 17 Aug 2016 01:33:19 +0000 Subject: [PATCH 07/18] Refactor away the field `arenas` of `ModuleS`. --- src/librustc_resolve/lib.rs | 14 ++++---------- src/librustc_resolve/resolve_imports.rs | 20 ++++++++++---------- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 71dee48f3396d..efc0247e6ff09 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -765,17 +765,12 @@ pub struct ModuleS<'a> { // access the children must be preceded with a // `populate_module_if_necessary` call. populated: Cell, - - arenas: &'a ResolverArenas<'a>, } pub type Module<'a> = &'a ModuleS<'a>; impl<'a> ModuleS<'a> { - fn new(parent_link: ParentLink<'a>, - def: Option, - external: bool, - arenas: &'a ResolverArenas<'a>) -> Self { + fn new(parent_link: ParentLink<'a>, def: Option, external: bool) -> Self { ModuleS { parent_link: parent_link, def: def, @@ -786,7 +781,6 @@ impl<'a> ModuleS<'a> { globs: RefCell::new((Vec::new())), traits: RefCell::new(None), populated: Cell::new(!external), - arenas: arenas } } @@ -1136,7 +1130,7 @@ impl<'a> Resolver<'a> { -> Resolver<'a> { let root_def_id = DefId::local(CRATE_DEF_INDEX); let graph_root = - ModuleS::new(NoParentLink, Some(Def::Mod(root_def_id)), false, arenas); + ModuleS::new(NoParentLink, Some(Def::Mod(root_def_id)), false); let graph_root = arenas.alloc_module(graph_root); let mut module_map = NodeMap(); module_map.insert(CRATE_NODE_ID, graph_root); @@ -1211,12 +1205,12 @@ impl<'a> Resolver<'a> { fn new_module(&self, parent_link: ParentLink<'a>, def: Option, external: bool) -> Module<'a> { - self.arenas.alloc_module(ModuleS::new(parent_link, def, external, self.arenas)) + self.arenas.alloc_module(ModuleS::new(parent_link, def, external)) } fn new_extern_crate_module(&self, parent_link: ParentLink<'a>, def: Def, local_node_id: NodeId) -> Module<'a> { - let mut module = ModuleS::new(parent_link, Some(def), false, self.arenas); + let mut module = ModuleS::new(parent_link, Some(def), false); module.extern_crate_id = Some(local_node_id); self.arenas.modules.alloc(module) } diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 0905632d4e627..05c4430a68700 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -135,14 +135,13 @@ impl<'a> NameResolution<'a> { } } -impl<'a> ::ModuleS<'a> { - fn resolution(&self, name: Name, ns: Namespace) -> &'a RefCell> { - *self.resolutions.borrow_mut().entry((name, ns)) - .or_insert_with(|| self.arenas.alloc_name_resolution()) +impl<'a> Resolver<'a> { + fn resolution(&self, module: Module<'a>, name: Name, ns: Namespace) + -> &'a RefCell> { + *module.resolutions.borrow_mut().entry((name, ns)) + .or_insert_with(|| self.arenas.alloc_name_resolution()) } -} -impl<'a> Resolver<'a> { /// Attempts to resolve the supplied name in the given module for the given namespace. /// If successful, returns the binding corresponding to the name. pub fn resolve_name_in_module(&mut self, @@ -154,7 +153,7 @@ impl<'a> Resolver<'a> { -> ResolveResult<&'a NameBinding<'a>> { self.populate_module_if_necessary(module); - let resolution = module.resolution(name, ns); + let resolution = self.resolution(module, name, ns); let resolution = match resolution.borrow_state() { ::std::cell::BorrowState::Unused => resolution.borrow_mut(), _ => return Failed(None), // This happens when there is a cycle of imports @@ -240,8 +239,9 @@ impl<'a> Resolver<'a> { span: Span, id: NodeId, vis: ty::Visibility) { + let current_module = self.current_module; let directive = self.arenas.alloc_import_directive(ImportDirective { - parent: self.current_module, + parent: current_module, module_path: module_path, target_module: Cell::new(None), subclass: subclass, @@ -254,7 +254,7 @@ impl<'a> Resolver<'a> { match directive.subclass { SingleImport { target, .. } => { for &ns in &[ValueNS, TypeNS] { - let mut resolution = self.current_module.resolution(target, ns).borrow_mut(); + let mut resolution = self.resolution(current_module, target, ns).borrow_mut(); resolution.single_imports.add_directive(directive); } } @@ -311,7 +311,7 @@ impl<'a> Resolver<'a> { // Ensure that `resolution` isn't borrowed when defining in the module's glob importers, // during which the resolution might end up getting re-defined via a glob cycle. let (new_binding, t) = { - let mut resolution = &mut *module.resolution(name, ns).borrow_mut(); + let mut resolution = &mut *self.resolution(module, name, ns).borrow_mut(); let was_known = resolution.binding().is_some(); let t = f(self, resolution); From 05afe15d1f7b29b5f8f006f6e9ec16c8a7cd4a0c Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Fri, 5 Aug 2016 01:58:59 +0000 Subject: [PATCH 08/18] Refactor `record_used: bool` -> `record_used: Option`. --- src/librustc_resolve/lib.rs | 24 +++++++++++++----------- src/librustc_resolve/resolve_imports.rs | 15 +++++++++------ 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index efc0247e6ff09..f200ae2f75873 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1246,7 +1246,7 @@ impl<'a> Resolver<'a> { -> ResolveResult> { fn search_parent_externals<'a>(this: &mut Resolver<'a>, needle: Name, module: Module<'a>) -> Option> { - match this.resolve_name_in_module(module, needle, TypeNS, false, false) { + match this.resolve_name_in_module(module, needle, TypeNS, false, None) { Success(binding) if binding.is_extern_crate() => Some(module), _ => match module.parent_link { ModuleParentLink(ref parent, _) => { @@ -1265,7 +1265,7 @@ impl<'a> Resolver<'a> { // modules as we go. while index < module_path_len { let name = module_path[index]; - match self.resolve_name_in_module(search_module, name, TypeNS, false, true) { + match self.resolve_name_in_module(search_module, name, TypeNS, false, Some(span)) { Failed(None) => { let segment_name = name.as_str(); let module_name = module_to_string(search_module); @@ -1361,7 +1361,7 @@ impl<'a> Resolver<'a> { // first component of the path in the current lexical // scope and then proceed to resolve below that. let ident = ast::Ident::with_empty_ctxt(module_path[0]); - match self.resolve_ident_in_lexical_scope(ident, TypeNS, true) + match self.resolve_ident_in_lexical_scope(ident, TypeNS, Some(span)) .and_then(LexicalScopeBinding::module) { None => return Failed(None), Some(containing_module) => { @@ -1404,7 +1404,7 @@ impl<'a> Resolver<'a> { fn resolve_ident_in_lexical_scope(&mut self, mut ident: ast::Ident, ns: Namespace, - record_used: bool) + record_used: Option) -> Option> { if ns == TypeNS { ident = ast::Ident::with_empty_ctxt(ident.name); @@ -1432,7 +1432,7 @@ impl<'a> Resolver<'a> { if module.def.is_some() { return match self.prelude { Some(prelude) if !module.no_implicit_prelude.get() => { - self.resolve_name_in_module(prelude, name, ns, false, false).success() + self.resolve_name_in_module(prelude, name, ns, false, None).success() .map(LexicalScopeBinding::Item) } _ => None, @@ -2287,7 +2287,7 @@ impl<'a> Resolver<'a> { PatKind::Ident(bmode, ref ident, ref opt_pat) => { // First try to resolve the identifier as some existing // entity, then fall back to a fresh binding. - let binding = self.resolve_ident_in_lexical_scope(ident.node, ValueNS, false) + let binding = self.resolve_ident_in_lexical_scope(ident.node, ValueNS, None) .and_then(LexicalScopeBinding::item); let resolution = binding.and_then(NameBinding::def).and_then(|def| { let always_binding = !pat_src.is_refutable() || opt_pat.is_some() || @@ -2454,11 +2454,11 @@ impl<'a> Resolver<'a> { // // Such behavior is required for backward compatibility. // The same fallback is used when `a` resolves to nothing. - let def = resolve_identifier_with_fallback(self, true).ok_or(false); + let def = resolve_identifier_with_fallback(self, Some(span)).ok_or(false); return def.and_then(|def| self.adjust_local_def(def, span).ok_or(true)).map(mk_res); } - let unqualified_def = resolve_identifier_with_fallback(self, false); + let unqualified_def = resolve_identifier_with_fallback(self, None); let qualified_binding = self.resolve_module_relative_path(span, segments, namespace); match (qualified_binding, unqualified_def) { (Ok(binding), Some(ref ud)) if binding.def().unwrap() == ud.def => { @@ -2478,7 +2478,7 @@ impl<'a> Resolver<'a> { fn resolve_identifier(&mut self, identifier: ast::Ident, namespace: Namespace, - record_used: bool) + record_used: Option) -> Option { if identifier.name == keywords::Invalid.name() { return None; @@ -2613,7 +2613,8 @@ impl<'a> Resolver<'a> { } let name = segments.last().unwrap().identifier.name; - let result = self.resolve_name_in_module(containing_module, name, namespace, false, true); + let result = + self.resolve_name_in_module(containing_module, name, namespace, false, Some(span)); result.success().map(|binding| { self.check_privacy(name, binding, span); binding @@ -2657,7 +2658,8 @@ impl<'a> Resolver<'a> { } let name = segments.last().unwrap().name(); - let result = self.resolve_name_in_module(containing_module, name, namespace, false, true); + let result = + self.resolve_name_in_module(containing_module, name, namespace, false, Some(span)); result.success().map(|binding| { self.check_privacy(name, binding, span); binding diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 05c4430a68700..28401b2e240f0 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -149,7 +149,7 @@ impl<'a> Resolver<'a> { name: Name, ns: Namespace, allow_private_imports: bool, - record_used: bool) + record_used: Option) -> ResolveResult<&'a NameBinding<'a>> { self.populate_module_if_necessary(module); @@ -165,7 +165,7 @@ impl<'a> Resolver<'a> { if !allow_private_imports && binding.is_import() && !binding.is_pseudo_public() { return Failed(None); } - if record_used { + if record_used.is_some() { self.record_use(name, ns, binding); } Success(binding) @@ -176,7 +176,7 @@ impl<'a> Resolver<'a> { for directive in module.globs.borrow().iter() { if !allow_private_imports && directive.vis != ty::Visibility::Public { continue } if let Some(target_module) = directive.target_module.get() { - let result = self.resolve_name_in_module(target_module, name, ns, false, false); + let result = self.resolve_name_in_module(target_module, name, ns, false, None); if let Indeterminate = result { return Indeterminate; } @@ -222,7 +222,7 @@ impl<'a> Resolver<'a> { SingleImport { source, .. } => source, GlobImport { .. } => unreachable!(), }; - match self.resolve_name_in_module(target_module, name, ns, false, false) { + match self.resolve_name_in_module(target_module, name, ns, false, None) { Failed(_) => {} _ => return Some(Indeterminate), } @@ -495,8 +495,11 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { }; // We need to resolve both namespaces for this to succeed. - let value_result = self.resolve_name_in_module(target_module, source, ValueNS, false, true); - let type_result = self.resolve_name_in_module(target_module, source, TypeNS, false, true); + let span = directive.span; + let value_result = + self.resolve_name_in_module(target_module, source, ValueNS, false, Some(span)); + let type_result = + self.resolve_name_in_module(target_module, source, TypeNS, false, Some(span)); let mut privacy_error = true; for &(ns, result, determined) in &[(ValueNS, &value_result, value_determined), From 7608bbdea869b061a65c996cac6c15d840436a7c Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Mon, 15 Aug 2016 20:51:19 +0000 Subject: [PATCH 09/18] Refactor `resolve_module_path` to take an `Option` instead of a `Span`. --- src/librustc_resolve/lib.rs | 36 +++++++++++-------------- src/librustc_resolve/resolve_imports.rs | 2 +- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index f200ae2f75873..d514d7e906a8d 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1242,7 +1242,7 @@ impl<'a> Resolver<'a> { mut search_module: Module<'a>, module_path: &[Name], index: usize, - span: Span) + span: Option) -> ResolveResult> { fn search_parent_externals<'a>(this: &mut Resolver<'a>, needle: Name, module: Module<'a>) -> Option> { @@ -1265,7 +1265,7 @@ impl<'a> Resolver<'a> { // modules as we go. while index < module_path_len { let name = module_path[index]; - match self.resolve_name_in_module(search_module, name, TypeNS, false, Some(span)) { + match self.resolve_name_in_module(search_module, name, TypeNS, false, span) { Failed(None) => { let segment_name = name.as_str(); let module_name = module_to_string(search_module); @@ -1291,7 +1291,7 @@ impl<'a> Resolver<'a> { format!("Could not find `{}` in `{}`", segment_name, module_name) }; - return Failed(Some((span, msg))); + return Failed(span.map(|span| (span, msg))); } Failed(err) => return Failed(err), Indeterminate => { @@ -1304,11 +1304,13 @@ impl<'a> Resolver<'a> { // Check to see whether there are type bindings, and, if // so, whether there is a module within. if let Some(module_def) = binding.module() { - self.check_privacy(name, binding, span); + if let Some(span) = span { + self.check_privacy(name, binding, span); + } search_module = module_def; } else { let msg = format!("Not a module `{}`", name); - return Failed(Some((span, msg))); + return Failed(span.map(|span| (span, msg))); } } } @@ -1324,7 +1326,7 @@ impl<'a> Resolver<'a> { fn resolve_module_path(&mut self, module_path: &[Name], use_lexical_scope: UseLexicalScopeFlag, - span: Span) + span: Option) -> ResolveResult> { if module_path.len() == 0 { return Success(self.graph_root) // Use the crate root @@ -1361,7 +1363,7 @@ impl<'a> Resolver<'a> { // first component of the path in the current lexical // scope and then proceed to resolve below that. let ident = ast::Ident::with_empty_ctxt(module_path[0]); - match self.resolve_ident_in_lexical_scope(ident, TypeNS, Some(span)) + match self.resolve_ident_in_lexical_scope(ident, TypeNS, span) .and_then(LexicalScopeBinding::module) { None => return Failed(None), Some(containing_module) => { @@ -1378,10 +1380,7 @@ impl<'a> Resolver<'a> { } } - self.resolve_module_path_from_root(search_module, - module_path, - start_index, - span) + self.resolve_module_path_from_root(search_module, module_path, start_index, span) } /// This resolves the identifier `ident` in the namespace `ns` in the current lexical scope. @@ -1485,7 +1484,7 @@ impl<'a> Resolver<'a> { /// Resolves a "module prefix". A module prefix is one or both of (a) `self::`; /// (b) some chain of `super::`. /// grammar: (SELF MOD_SEP ) ? (SUPER MOD_SEP) * - fn resolve_module_prefix(&mut self, module_path: &[Name], span: Span) + fn resolve_module_prefix(&mut self, module_path: &[Name], span: Option) -> ResolveResult> { // Start at the current module if we see `self` or `super`, or at the // top of the crate otherwise. @@ -1504,7 +1503,7 @@ impl<'a> Resolver<'a> { match self.get_nearest_normal_module_parent(containing_module) { None => { let msg = "There are too many initial `super`s.".into(); - return Failed(Some((span, msg))); + return Failed(span.map(|span| (span, msg))); } Some(new_module) => { containing_module = new_module; @@ -2592,7 +2591,7 @@ impl<'a> Resolver<'a> { .collect::>(); let containing_module; - match self.resolve_module_path(&module_path, UseLexicalScope, span) { + match self.resolve_module_path(&module_path, UseLexicalScope, Some(span)) { Failed(err) => { let (span, msg) = match err { Some((span, msg)) => (span, msg), @@ -2632,10 +2631,7 @@ impl<'a> Resolver<'a> { let root_module = self.graph_root; let containing_module; - match self.resolve_module_path_from_root(root_module, - &module_path, - 0, - span) { + match self.resolve_module_path_from_root(root_module, &module_path, 0, Some(span)) { Failed(err) => { let (span, msg) = match err { Some((span, msg)) => (span, msg), @@ -2915,7 +2911,7 @@ impl<'a> Resolver<'a> { match self.resolve_module_path(&name_path[..], UseLexicalScope, - expr.span) { + Some(expr.span)) { Success(e) => { if let Some(def_type) = e.def { def = def_type; @@ -3253,7 +3249,7 @@ impl<'a> Resolver<'a> { let segments: Vec<_> = path.segments.iter().map(|seg| seg.identifier.name).collect(); let mut path_resolution = err_path_resolution(); - let vis = match self.resolve_module_path(&segments, DontUseLexicalScope, path.span) { + let vis = match self.resolve_module_path(&segments, DontUseLexicalScope, Some(path.span)) { Success(module) => { let def = module.def.unwrap(); path_resolution = PathResolution::new(def); diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 28401b2e240f0..3c44051503634 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -480,7 +480,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { Some(module) => module, _ => match self.resolve_module_path(&directive.module_path, DontUseLexicalScope, - directive.span) { + Some(directive.span)) { Success(module) => module, Indeterminate => return Indeterminate, Failed(err) => return Failed(err), From e1c9efcba48f098482ee6033835a4fe321268709 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Wed, 17 Aug 2016 20:48:53 +0000 Subject: [PATCH 10/18] Refactor `value_determined` -> `value_result`, `type_determined` -> `type_result`. --- src/librustc_resolve/resolve_imports.rs | 85 +++++++++++++------------ 1 file changed, 45 insertions(+), 40 deletions(-) diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 3c44051503634..6e1128c67735e 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -38,23 +38,23 @@ impl<'a> Resolver<'a> { /// Contains data for specific types of import directives. #[derive(Clone, Debug)] -pub enum ImportDirectiveSubclass { +pub enum ImportDirectiveSubclass<'a> { SingleImport { target: Name, source: Name, - type_determined: Cell, - value_determined: Cell, + value_result: Cell, bool /* determined? */>>, + type_result: Cell, bool /* determined? */>>, }, GlobImport { is_prelude: bool }, } -impl ImportDirectiveSubclass { +impl<'a> ImportDirectiveSubclass<'a> { pub fn single(target: Name, source: Name) -> Self { SingleImport { target: target, source: source, - type_determined: Cell::new(false), - value_determined: Cell::new(false), + type_result: Cell::new(Err(false)), + value_result: Cell::new(Err(false)), } } } @@ -66,7 +66,7 @@ pub struct ImportDirective<'a> { parent: Module<'a>, module_path: Vec, target_module: Cell>>, // the resolution of `module_path` - subclass: ImportDirectiveSubclass, + subclass: ImportDirectiveSubclass<'a>, span: Span, vis: ty::Visibility, // see note in ImportResolutionPerNamespace about how to use this } @@ -235,7 +235,7 @@ impl<'a> Resolver<'a> { // Add an import directive to the current module. pub fn add_import_directive(&mut self, module_path: Vec, - subclass: ImportDirectiveSubclass, + subclass: ImportDirectiveSubclass<'a>, span: Span, id: NodeId, vis: ty::Visibility) { @@ -488,30 +488,35 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { }; directive.target_module.set(Some(target_module)); - let (source, target, value_determined, type_determined) = match directive.subclass { - SingleImport { source, target, ref value_determined, ref type_determined } => - (source, target, value_determined, type_determined), + let (source, target, value_result, type_result) = match directive.subclass { + SingleImport { source, target, ref value_result, ref type_result } => + (source, target, value_result, type_result), GlobImport { .. } => return self.resolve_glob_import(target_module, directive), }; - // We need to resolve both namespaces for this to succeed. - let span = directive.span; - let value_result = - self.resolve_name_in_module(target_module, source, ValueNS, false, Some(span)); - let type_result = - self.resolve_name_in_module(target_module, source, TypeNS, false, Some(span)); - let mut privacy_error = true; - for &(ns, result, determined) in &[(ValueNS, &value_result, value_determined), - (TypeNS, &type_result, type_determined)] { - match *result { - Failed(..) if !determined.get() => { - determined.set(true); + for &(ns, result) in &[(ValueNS, value_result), (TypeNS, type_result)] { + let was_determined = if let Err(false) = result.get() { + result.set({ + let span = Some(directive.span); + match self.resolve_name_in_module(target_module, source, ns, false, span) { + Success(binding) => Ok(binding), + Indeterminate => Err(false), + Failed(_) => Err(true), + } + }); + false + } else { + true + }; + + match result.get() { + Err(true) if !was_determined => { self.update_resolution(module, target, ns, |_, resolution| { resolution.single_imports.directive_failed() }); } - Success(binding) if !binding.is_importable() => { + Ok(binding) if !binding.is_importable() => { let msg = format!("`{}` is not directly importable", target); struct_span_err!(self.session, directive.span, E0253, "{}", &msg) .span_label(directive.span, &format!("cannot be imported directly")) @@ -521,9 +526,8 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { self.import_dummy_binding(directive); return Success(()); } - Success(binding) if !self.is_accessible(binding.vis) => {} - Success(binding) if !determined.get() => { - determined.set(true); + Ok(binding) if !self.is_accessible(binding.vis) => {} + Ok(binding) if !was_determined => { let imported_binding = self.import(binding, directive); let conflict = self.try_define(module, target, ns, imported_binding); if let Err(old_binding) = conflict { @@ -532,14 +536,15 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { } privacy_error = false; } - Success(_) => privacy_error = false, + Ok(_) => privacy_error = false, _ => {} } } - match (&value_result, &type_result) { - (&Indeterminate, _) | (_, &Indeterminate) => return Indeterminate, - (&Failed(_), &Failed(_)) => { + let (value_result, type_result) = (value_result.get(), type_result.get()); + match (value_result, type_result) { + (Err(false), _) | (_, Err(false)) => return Indeterminate, + (Err(true), Err(true)) => { let resolutions = target_module.resolutions.borrow(); let names = resolutions.iter().filter_map(|(&(ref name, _), resolution)| { if *name == source { return None; } // Never suggest the same name @@ -565,17 +570,17 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { } if privacy_error { - for &(ns, result) in &[(ValueNS, &value_result), (TypeNS, &type_result)] { - let binding = match *result { Success(binding) => binding, _ => continue }; + for &(ns, result) in &[(ValueNS, value_result), (TypeNS, type_result)] { + let binding = match result { Ok(binding) => binding, _ => continue }; self.privacy_errors.push(PrivacyError(directive.span, source, binding)); let imported_binding = self.import(binding, directive); let _ = self.try_define(module, target, ns, imported_binding); } } - match (&value_result, &type_result) { - (&Success(binding), _) if !binding.pseudo_vis().is_at_least(directive.vis, self) && - self.is_accessible(binding.vis) => { + match (value_result, type_result) { + (Ok(binding), _) if !binding.pseudo_vis().is_at_least(directive.vis, self) && + self.is_accessible(binding.vis) => { let msg = format!("`{}` is private, and cannot be reexported", source); let note_msg = format!("consider marking `{}` as `pub` in the imported module", source); @@ -584,8 +589,8 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { .emit(); } - (_, &Success(binding)) if !binding.pseudo_vis().is_at_least(directive.vis, self) && - self.is_accessible(binding.vis) => { + (_, Ok(binding)) if !binding.pseudo_vis().is_at_least(directive.vis, self) && + self.is_accessible(binding.vis) => { if binding.is_extern_crate() { let msg = format!("extern crate `{}` is private, and cannot be reexported \ (error E0364), consider declaring with `pub`", @@ -607,9 +612,9 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { // Record what this import resolves to for later uses in documentation, // this may resolve to either a value or a type, but for documentation // purposes it's good enough to just favor one over the other. - let def = match type_result.success().and_then(NameBinding::def) { + let def = match type_result.ok().and_then(NameBinding::def) { Some(def) => def, - None => value_result.success().and_then(NameBinding::def).unwrap(), + None => value_result.ok().and_then(NameBinding::def).unwrap(), }; let path_resolution = PathResolution::new(def); self.def_map.insert(directive.id, path_resolution); From 165b0b618cbe83bf15d40742bd2cb9db935097d6 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Mon, 15 Aug 2016 22:43:25 +0000 Subject: [PATCH 11/18] Check privacy in `resolve_name_in_module`. --- src/librustc_resolve/lib.rs | 19 ++----------------- src/librustc_resolve/resolve_imports.rs | 5 ++++- 2 files changed, 6 insertions(+), 18 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index d514d7e906a8d..6896439640571 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1304,9 +1304,6 @@ impl<'a> Resolver<'a> { // Check to see whether there are type bindings, and, if // so, whether there is a module within. if let Some(module_def) = binding.module() { - if let Some(span) = span { - self.check_privacy(name, binding, span); - } search_module = module_def; } else { let msg = format!("Not a module `{}`", name); @@ -2614,10 +2611,7 @@ impl<'a> Resolver<'a> { let name = segments.last().unwrap().identifier.name; let result = self.resolve_name_in_module(containing_module, name, namespace, false, Some(span)); - result.success().map(|binding| { - self.check_privacy(name, binding, span); - binding - }).ok_or(false) + result.success().ok_or(false) } /// Invariant: This must be called only during main resolution, not during @@ -2656,10 +2650,7 @@ impl<'a> Resolver<'a> { let name = segments.last().unwrap().name(); let result = self.resolve_name_in_module(containing_module, name, namespace, false, Some(span)); - result.success().map(|binding| { - self.check_privacy(name, binding, span); - binding - }).ok_or(false) + result.success().ok_or(false) } fn with_no_errors(&mut self, f: F) -> T @@ -3276,12 +3267,6 @@ impl<'a> Resolver<'a> { vis.is_at_least(self.current_vis, self) } - fn check_privacy(&mut self, name: Name, binding: &'a NameBinding<'a>, span: Span) { - if !self.is_accessible(binding.vis) { - self.privacy_errors.push(PrivacyError(span, name, binding)); - } - } - fn report_privacy_errors(&self) { if self.privacy_errors.len() == 0 { return } let mut reported_spans = HashSet::new(); diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 6e1128c67735e..0757ff7ddbfd8 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -165,8 +165,11 @@ impl<'a> Resolver<'a> { if !allow_private_imports && binding.is_import() && !binding.is_pseudo_public() { return Failed(None); } - if record_used.is_some() { + if let Some(span) = record_used { self.record_use(name, ns, binding); + if !self.is_accessible(binding.vis) { + self.privacy_errors.push(PrivacyError(span, name, binding)); + } } Success(binding) }); From fbc322975f198a98d917b5dad0eee557f9889508 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Mon, 15 Aug 2016 08:19:09 +0000 Subject: [PATCH 12/18] Refactor out `finalize_import()` from `resolve_import()`. --- src/librustc_resolve/lib.rs | 6 +- src/librustc_resolve/resolve_imports.rs | 220 +++++++++++++----------- 2 files changed, 125 insertions(+), 101 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 6896439640571..a0f0fccac8651 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -957,7 +957,10 @@ pub struct Resolver<'a> { structs: FnvHashMap>, - // All indeterminate imports (i.e. imports not known to succeed or fail). + // All imports known to succeed or fail. + determined_imports: Vec<&'a ImportDirective<'a>>, + + // All non-determined imports. indeterminate_imports: Vec<&'a ImportDirective<'a>>, // The module that represents the current item scope. @@ -1149,6 +1152,7 @@ impl<'a> Resolver<'a> { trait_item_map: FnvHashMap(), structs: FnvHashMap(), + determined_imports: Vec::new(), indeterminate_imports: Vec::new(), current_module: graph_root, diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 0757ff7ddbfd8..c2f8e31277c3f 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -144,6 +144,7 @@ impl<'a> Resolver<'a> { /// Attempts to resolve the supplied name in the given module for the given namespace. /// If successful, returns the binding corresponding to the name. + /// Invariant: if `record_used` is `Some`, import resolution must be complete. pub fn resolve_name_in_module(&mut self, module: Module<'a>, name: Name, @@ -159,32 +160,46 @@ impl<'a> Resolver<'a> { _ => return Failed(None), // This happens when there is a cycle of imports }; - if let Some(result) = self.try_result(&resolution, ns, allow_private_imports) { - // If the resolution doesn't depend on glob definability, check privacy and return. - return result.and_then(|binding| { - if !allow_private_imports && binding.is_import() && !binding.is_pseudo_public() { + let is_disallowed_private_import = |binding: &NameBinding| { + !allow_private_imports && !binding.is_pseudo_public() && binding.is_import() + }; + + if let Some(span) = record_used { + if let Some(binding) = resolution.binding { + if is_disallowed_private_import(binding) { return Failed(None); } - if let Some(span) = record_used { - self.record_use(name, ns, binding); - if !self.is_accessible(binding.vis) { - self.privacy_errors.push(PrivacyError(span, name, binding)); - } + self.record_use(name, ns, binding); + if !self.is_accessible(binding.vis) { + self.privacy_errors.push(PrivacyError(span, name, binding)); + } + } + + return resolution.binding.map(Success).unwrap_or(Failed(None)); + } + + // If the resolution doesn't depend on glob definability, check privacy and return. + if let Some(result) = self.try_result(&resolution, ns) { + return result.and_then(|binding| { + if self.is_accessible(binding.vis) && !is_disallowed_private_import(binding) { + Success(binding) + } else { + Failed(None) } - Success(binding) }); } // Check if the globs are determined for directive in module.globs.borrow().iter() { - if !allow_private_imports && directive.vis != ty::Visibility::Public { continue } - if let Some(target_module) = directive.target_module.get() { - let result = self.resolve_name_in_module(target_module, name, ns, false, None); - if let Indeterminate = result { + if self.is_accessible(directive.vis) { + if let Some(target_module) = directive.target_module.get() { + let result = self.resolve_name_in_module(target_module, name, ns, true, None); + if let Indeterminate = result { + return Indeterminate; + } + } else { return Indeterminate; } - } else { - return Indeterminate; } } @@ -193,10 +208,7 @@ impl<'a> Resolver<'a> { // Returns Some(the resolution of the name), or None if the resolution depends // on whether more globs can define the name. - fn try_result(&mut self, - resolution: &NameResolution<'a>, - ns: Namespace, - allow_private_imports: bool) + fn try_result(&mut self, resolution: &NameResolution<'a>, ns: Namespace) -> Option>> { match resolution.binding { Some(binding) if !binding.is_glob_import() => @@ -206,17 +218,8 @@ impl<'a> Resolver<'a> { // Check if a single import can still define the name. match resolution.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.vis != ty::Visibility::Public && - !resolution.binding.map(NameBinding::is_pseudo_public).unwrap_or(false) { - return None; - } - + SingleImports::MaybeOne(directive) if self.is_accessible(directive.vis) => { let target_module = match directive.target_module.get() { Some(target_module) => target_module, None => return Some(Indeterminate), @@ -225,11 +228,12 @@ impl<'a> Resolver<'a> { SingleImport { source, .. } => source, GlobImport { .. } => unreachable!(), }; - match self.resolve_name_in_module(target_module, name, ns, false, None) { + match self.resolve_name_in_module(target_module, name, ns, true, None) { Failed(_) => {} _ => return Some(Indeterminate), } } + SingleImports::MaybeOne(_) | SingleImports::None => {}, } resolution.binding.map(Success) @@ -389,7 +393,6 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { fn resolve_imports(&mut self) { let mut i = 0; let mut prev_num_indeterminates = self.indeterminate_imports.len() + 1; - let mut errors = Vec::new(); while self.indeterminate_imports.len() < prev_num_indeterminates { prev_num_indeterminates = self.indeterminate_imports.len(); @@ -400,19 +403,9 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { for import in imports { match self.resolve_import(&import) { - Failed(err) => { - let (span, help) = match err { - Some((span, msg)) => (span, format!(". {}", msg)), - None => (import.span, String::new()), - }; - errors.push(ImportResolvingError { - import_directive: import, - span: span, - help: help, - }); - } + Failed(_) => self.determined_imports.push(import), Indeterminate => self.indeterminate_imports.push(import), - Success(()) => {} + Success(()) => self.determined_imports.push(import), } } @@ -423,6 +416,22 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { self.finalize_resolutions_in(module); } + let mut errors = Vec::new(); + for i in 0 .. self.determined_imports.len() { + let import = self.determined_imports[i]; + if let Failed(err) = self.finalize_import(import) { + let (span, help) = match err { + Some((span, msg)) => (span, format!(". {}", msg)), + None => (import.span, String::new()), + }; + errors.push(ImportResolvingError { + import_directive: import, + span: span, + help: help, + }); + } + } + // Report unresolved imports only if no hard error was already reported // to avoid generating multiple errors on the same import. if errors.len() == 0 { @@ -481,9 +490,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { let target_module = match directive.target_module.get() { Some(module) => module, - _ => match self.resolve_module_path(&directive.module_path, - DontUseLexicalScope, - Some(directive.span)) { + _ => match self.resolve_module_path(&directive.module_path, DontUseLexicalScope, None) { Success(module) => module, Indeterminate => return Indeterminate, Failed(err) => return Failed(err), @@ -494,27 +501,29 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { let (source, target, value_result, type_result) = match directive.subclass { SingleImport { source, target, ref value_result, ref type_result } => (source, target, value_result, type_result), - GlobImport { .. } => return self.resolve_glob_import(target_module, directive), + GlobImport { .. } => { + self.resolve_glob_import(directive); + return Success(()); + } }; - let mut privacy_error = true; + let mut indeterminate = false; for &(ns, result) in &[(ValueNS, value_result), (TypeNS, type_result)] { - let was_determined = if let Err(false) = result.get() { + if let Err(false) = result.get() { result.set({ - let span = Some(directive.span); - match self.resolve_name_in_module(target_module, source, ns, false, span) { + match self.resolve_name_in_module(target_module, source, ns, false, None) { Success(binding) => Ok(binding), Indeterminate => Err(false), Failed(_) => Err(true), } }); - false } else { - true + continue }; match result.get() { - Err(true) if !was_determined => { + Err(false) => indeterminate = true, + Err(true) => { self.update_resolution(module, target, ns, |_, resolution| { resolution.single_imports.directive_failed() }); @@ -529,26 +538,55 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { self.import_dummy_binding(directive); return Success(()); } - Ok(binding) if !self.is_accessible(binding.vis) => {} - Ok(binding) if !was_determined => { + Ok(binding) => { let imported_binding = self.import(binding, directive); let conflict = self.try_define(module, target, ns, imported_binding); if let Err(old_binding) = conflict { let binding = &self.import(binding, directive); self.report_conflict(module, target, ns, binding, old_binding); } - privacy_error = false; } - Ok(_) => privacy_error = false, - _ => {} } } - let (value_result, type_result) = (value_result.get(), type_result.get()); - match (value_result, type_result) { - (Err(false), _) | (_, Err(false)) => return Indeterminate, - (Err(true), Err(true)) => { - let resolutions = target_module.resolutions.borrow(); + if indeterminate { Indeterminate } else { Success(()) } + } + + fn finalize_import(&mut self, directive: &'b ImportDirective<'b>) -> ResolveResult<()> { + self.set_current_module(directive.parent); + + let ImportDirective { ref module_path, span, .. } = *directive; + let module_result = self.resolve_module_path(&module_path, DontUseLexicalScope, Some(span)); + let module = match module_result { + Success(module) => module, + Indeterminate => return Indeterminate, + Failed(err) => return Failed(err), + }; + + let (source, value_result, type_result) = match directive.subclass { + SingleImport { source, ref value_result, ref type_result, .. } => + (source, value_result.get(), type_result.get()), + GlobImport { .. } if module.def_id() == directive.parent.def_id() => { + // Importing a module into itself is not allowed. + let msg = "Cannot glob-import a module into itself.".into(); + return Failed(Some((directive.span, msg))); + } + GlobImport { .. } => return Success(()), + }; + + for &(ns, result) in &[(ValueNS, value_result), (TypeNS, type_result)] { + if let Ok(binding) = result { + self.record_use(source, ns, binding); + } + } + + if value_result.is_err() && type_result.is_err() { + let (value_result, type_result); + value_result = self.resolve_name_in_module(module, source, ValueNS, false, Some(span)); + type_result = self.resolve_name_in_module(module, source, TypeNS, false, Some(span)); + + return if let (Failed(_), Failed(_)) = (value_result, type_result) { + let resolutions = module.resolutions.borrow(); let names = resolutions.iter().filter_map(|(&(ref name, _), resolution)| { if *name == source { return None; } // Never suggest the same name match *resolution.borrow() { @@ -561,29 +599,22 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { Some(name) => format!(". Did you mean to use `{}`?", name), None => "".to_owned(), }; - let module_str = module_to_string(target_module); + let module_str = module_to_string(module); let msg = if &module_str == "???" { format!("There is no `{}` in the crate root{}", source, lev_suggestion) } else { format!("There is no `{}` in `{}`{}", source, module_str, lev_suggestion) }; - return Failed(Some((directive.span, msg))); - } - _ => (), - } - - if privacy_error { - for &(ns, result) in &[(ValueNS, value_result), (TypeNS, type_result)] { - let binding = match result { Ok(binding) => binding, _ => continue }; - self.privacy_errors.push(PrivacyError(directive.span, source, binding)); - let imported_binding = self.import(binding, directive); - let _ = self.try_define(module, target, ns, imported_binding); + Failed(Some((directive.span, msg))) + } else { + // `resolve_name_in_module` reported a privacy error. + self.import_dummy_binding(directive); + Success(()) } } match (value_result, type_result) { - (Ok(binding), _) if !binding.pseudo_vis().is_at_least(directive.vis, self) && - self.is_accessible(binding.vis) => { + (Ok(binding), _) if !binding.pseudo_vis().is_at_least(directive.vis, self) => { let msg = format!("`{}` is private, and cannot be reexported", source); let note_msg = format!("consider marking `{}` as `pub` in the imported module", source); @@ -592,8 +623,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { .emit(); } - (_, Ok(binding)) if !binding.pseudo_vis().is_at_least(directive.vis, self) && - self.is_accessible(binding.vis) => { + (_, Ok(binding)) if !binding.pseudo_vis().is_at_least(directive.vis, self) => { if binding.is_extern_crate() { let msg = format!("extern crate `{}` is private, and cannot be reexported \ (error E0364), consider declaring with `pub`", @@ -626,27 +656,20 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { return Success(()); } - // Resolves a glob import. Note that this function cannot fail; it either - // succeeds or bails out (as importing * from an empty module or a module - // that exports nothing is valid). target_module is the module we are - // actually importing, i.e., `foo` in `use foo::*`. - fn resolve_glob_import(&mut self, target_module: Module<'b>, directive: &'b ImportDirective<'b>) - -> ResolveResult<()> { + fn resolve_glob_import(&mut self, directive: &'b ImportDirective<'b>) { + let target_module = directive.target_module.get().unwrap(); + self.populate_module_if_necessary(target_module); + if let Some(Def::Trait(_)) = target_module.def { self.session.span_err(directive.span, "items in traits are not importable."); } - let module = self.current_module; - 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))); - } - self.populate_module_if_necessary(target_module); - - if let GlobImport { is_prelude: true } = directive.subclass { + let module = directive.parent; + if target_module.def_id() == module.def_id() { + return; + } else if let GlobImport { is_prelude: true } = directive.subclass { self.prelude = Some(target_module); - return Success(()); + return; } // Add to target_module's glob_importers @@ -669,9 +692,6 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { let resolution = PathResolution::new(Def::Mod(did)); self.def_map.insert(directive.id, resolution); } - - debug!("(resolving glob import) successfully resolved import"); - return Success(()); } // Miscellaneous post-processing, including recording reexports, reporting conflicts, From 5b969a2a58da5a1c7f6ea0587b9ff97b56e2f21d Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Thu, 18 Aug 2016 00:11:56 +0000 Subject: [PATCH 13/18] Improve import failure detection. --- src/librustc_resolve/resolve_imports.rs | 31 ++++++++++++++++--------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index c2f8e31277c3f..195333f4acda2 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -68,7 +68,7 @@ pub struct ImportDirective<'a> { target_module: Cell>>, // the resolution of `module_path` subclass: ImportDirectiveSubclass<'a>, span: Span, - vis: ty::Visibility, // see note in ImportResolutionPerNamespace about how to use this + vis: Cell, } impl<'a> ImportDirective<'a> { @@ -191,7 +191,7 @@ impl<'a> Resolver<'a> { // Check if the globs are determined for directive in module.globs.borrow().iter() { - if self.is_accessible(directive.vis) { + if self.is_accessible(directive.vis.get()) { if let Some(target_module) = directive.target_module.get() { let result = self.resolve_name_in_module(target_module, name, ns, true, None); if let Indeterminate = result { @@ -219,7 +219,7 @@ impl<'a> Resolver<'a> { // Check if a single import can still define the name. match resolution.single_imports { SingleImports::AtLeastOne => return Some(Indeterminate), - SingleImports::MaybeOne(directive) if self.is_accessible(directive.vis) => { + SingleImports::MaybeOne(directive) if self.is_accessible(directive.vis.get()) => { let target_module = match directive.target_module.get() { Some(target_module) => target_module, None => return Some(Indeterminate), @@ -254,7 +254,7 @@ impl<'a> Resolver<'a> { subclass: subclass, span: span, id: id, - vis: vis, + vis: Cell::new(vis), }); self.indeterminate_imports.push(directive); @@ -282,7 +282,7 @@ impl<'a> Resolver<'a> { directive: directive, }, span: directive.span, - vis: directive.vis, + vis: directive.vis.get(), } } @@ -488,13 +488,22 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { let module = directive.parent; self.set_current_module(module); - let target_module = match directive.target_module.get() { - Some(module) => module, - _ => match self.resolve_module_path(&directive.module_path, DontUseLexicalScope, None) { + let target_module = if let Some(module) = directive.target_module.get() { + module + } else { + let vis = directive.vis.get(); + // For better failure detection, pretend that the import will not define any names + // while resolving its module path. + directive.vis.set(ty::Visibility::PrivateExternal); + let result = + self.resolve_module_path(&directive.module_path, DontUseLexicalScope, None); + directive.vis.set(vis); + + match result { Success(module) => module, Indeterminate => return Indeterminate, Failed(err) => return Failed(err), - }, + } }; directive.target_module.set(Some(target_module)); @@ -614,7 +623,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { } match (value_result, type_result) { - (Ok(binding), _) if !binding.pseudo_vis().is_at_least(directive.vis, self) => { + (Ok(binding), _) if !binding.pseudo_vis().is_at_least(directive.vis.get(), self) => { let msg = format!("`{}` is private, and cannot be reexported", source); let note_msg = format!("consider marking `{}` as `pub` in the imported module", source); @@ -623,7 +632,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { .emit(); } - (_, Ok(binding)) if !binding.pseudo_vis().is_at_least(directive.vis, self) => { + (_, Ok(binding)) if !binding.pseudo_vis().is_at_least(directive.vis.get(), self) => { if binding.is_extern_crate() { let msg = format!("extern crate `{}` is private, and cannot be reexported \ (error E0364), consider declaring with `pub`", From 11c38fdce08d3fa38cb9a6a585215ce8957da3a9 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Thu, 18 Aug 2016 01:43:26 +0000 Subject: [PATCH 14/18] Rename `target_module` to `module` or `imported_module`. --- src/librustc_resolve/resolve_imports.rs | 50 ++++++++++++------------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 195333f4acda2..e76a93a6db03a 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -65,7 +65,7 @@ pub struct ImportDirective<'a> { pub id: NodeId, parent: Module<'a>, module_path: Vec, - target_module: Cell>>, // the resolution of `module_path` + imported_module: Cell>>, // the resolution of `module_path` subclass: ImportDirectiveSubclass<'a>, span: Span, vis: Cell, @@ -192,8 +192,8 @@ impl<'a> Resolver<'a> { // Check if the globs are determined for directive in module.globs.borrow().iter() { if self.is_accessible(directive.vis.get()) { - if let Some(target_module) = directive.target_module.get() { - let result = self.resolve_name_in_module(target_module, name, ns, true, None); + if let Some(module) = directive.imported_module.get() { + let result = self.resolve_name_in_module(module, name, ns, true, None); if let Indeterminate = result { return Indeterminate; } @@ -220,15 +220,15 @@ impl<'a> Resolver<'a> { match resolution.single_imports { SingleImports::AtLeastOne => return Some(Indeterminate), SingleImports::MaybeOne(directive) if self.is_accessible(directive.vis.get()) => { - let target_module = match directive.target_module.get() { - Some(target_module) => target_module, + let module = match directive.imported_module.get() { + Some(module) => module, None => return Some(Indeterminate), }; let name = match directive.subclass { SingleImport { source, .. } => source, GlobImport { .. } => unreachable!(), }; - match self.resolve_name_in_module(target_module, name, ns, true, None) { + match self.resolve_name_in_module(module, name, ns, true, None) { Failed(_) => {} _ => return Some(Indeterminate), } @@ -250,7 +250,7 @@ impl<'a> Resolver<'a> { let directive = self.arenas.alloc_import_directive(ImportDirective { parent: current_module, module_path: module_path, - target_module: Cell::new(None), + imported_module: Cell::new(None), subclass: subclass, span: span, id: id, @@ -485,10 +485,9 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { names_to_string(&directive.module_path), module_to_string(self.current_module)); - let module = directive.parent; - self.set_current_module(module); + self.set_current_module(directive.parent); - let target_module = if let Some(module) = directive.target_module.get() { + let module = if let Some(module) = directive.imported_module.get() { module } else { let vis = directive.vis.get(); @@ -506,7 +505,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { } }; - directive.target_module.set(Some(target_module)); + directive.imported_module.set(Some(module)); let (source, target, value_result, type_result) = match directive.subclass { SingleImport { source, target, ref value_result, ref type_result } => (source, target, value_result, type_result), @@ -520,7 +519,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { for &(ns, result) in &[(ValueNS, value_result), (TypeNS, type_result)] { if let Err(false) = result.get() { result.set({ - match self.resolve_name_in_module(target_module, source, ns, false, None) { + match self.resolve_name_in_module(module, source, ns, false, None) { Success(binding) => Ok(binding), Indeterminate => Err(false), Failed(_) => Err(true), @@ -533,7 +532,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { match result.get() { Err(false) => indeterminate = true, Err(true) => { - self.update_resolution(module, target, ns, |_, resolution| { + self.update_resolution(directive.parent, target, ns, |_, resolution| { resolution.single_imports.directive_failed() }); } @@ -549,10 +548,10 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { } Ok(binding) => { let imported_binding = self.import(binding, directive); - let conflict = self.try_define(module, target, ns, imported_binding); + let conflict = self.try_define(directive.parent, target, ns, imported_binding); if let Err(old_binding) = conflict { let binding = &self.import(binding, directive); - self.report_conflict(module, target, ns, binding, old_binding); + self.report_conflict(directive.parent, target, ns, binding, old_binding); } } } @@ -666,38 +665,37 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { } fn resolve_glob_import(&mut self, directive: &'b ImportDirective<'b>) { - let target_module = directive.target_module.get().unwrap(); - self.populate_module_if_necessary(target_module); + let module = directive.imported_module.get().unwrap(); + self.populate_module_if_necessary(module); - if let Some(Def::Trait(_)) = target_module.def { + if let Some(Def::Trait(_)) = module.def { self.session.span_err(directive.span, "items in traits are not importable."); } - let module = directive.parent; - if target_module.def_id() == module.def_id() { + if module.def_id() == directive.parent.def_id() { return; } else if let GlobImport { is_prelude: true } = directive.subclass { - self.prelude = Some(target_module); + self.prelude = Some(module); return; } - // Add to target_module's glob_importers - target_module.glob_importers.borrow_mut().push(directive); + // Add to module's glob_importers + module.glob_importers.borrow_mut().push(directive); // Ensure that `resolutions` isn't borrowed during `try_define`, // since it might get updated via a glob cycle. - let bindings = target_module.resolutions.borrow().iter().filter_map(|(name, resolution)| { + let bindings = module.resolutions.borrow().iter().filter_map(|(name, resolution)| { resolution.borrow().binding().map(|binding| (*name, binding)) }).collect::>(); for ((name, ns), binding) in bindings { if binding.is_importable() && binding.is_pseudo_public() { let imported_binding = self.import(binding, directive); - let _ = self.try_define(module, name, ns, imported_binding); + let _ = self.try_define(directive.parent, name, ns, imported_binding); } } // Record the destination of this import - if let Some(did) = target_module.def_id() { + if let Some(did) = module.def_id() { let resolution = PathResolution::new(Def::Mod(did)); self.def_map.insert(directive.id, resolution); } From 6cd43f6ee92fa3a235482fa28c5ac2c799018b8a Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Thu, 18 Aug 2016 01:46:18 +0000 Subject: [PATCH 15/18] Rename `source` -> `name` in `finalize_import`. --- src/librustc_resolve/resolve_imports.rs | 41 ++++++++++++------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index e76a93a6db03a..04702d02555e1 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -571,7 +571,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { Failed(err) => return Failed(err), }; - let (source, value_result, type_result) = match directive.subclass { + let (name, value_result, type_result) = match directive.subclass { SingleImport { source, ref value_result, ref type_result, .. } => (source, value_result.get(), type_result.get()), GlobImport { .. } if module.def_id() == directive.parent.def_id() => { @@ -584,34 +584,34 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { for &(ns, result) in &[(ValueNS, value_result), (TypeNS, type_result)] { if let Ok(binding) = result { - self.record_use(source, ns, binding); + self.record_use(name, ns, binding); } } if value_result.is_err() && type_result.is_err() { let (value_result, type_result); - value_result = self.resolve_name_in_module(module, source, ValueNS, false, Some(span)); - type_result = self.resolve_name_in_module(module, source, TypeNS, false, Some(span)); + value_result = self.resolve_name_in_module(module, name, ValueNS, false, Some(span)); + type_result = self.resolve_name_in_module(module, name, TypeNS, false, Some(span)); return if let (Failed(_), Failed(_)) = (value_result, type_result) { let resolutions = module.resolutions.borrow(); - let names = resolutions.iter().filter_map(|(&(ref name, _), resolution)| { - if *name == source { return None; } // Never suggest the same name + let names = resolutions.iter().filter_map(|(&(ref n, _), resolution)| { + if *n == name { return None; } // Never suggest the same name match *resolution.borrow() { - NameResolution { binding: Some(_), .. } => Some(name), + NameResolution { binding: Some(_), .. } => Some(n), NameResolution { single_imports: SingleImports::None, .. } => None, - _ => Some(name), + _ => Some(n), } }); - let lev_suggestion = match find_best_match_for_name(names, &source.as_str(), None) { + let lev_suggestion = match find_best_match_for_name(names, &name.as_str(), None) { Some(name) => format!(". Did you mean to use `{}`?", name), None => "".to_owned(), }; let module_str = module_to_string(module); let msg = if &module_str == "???" { - format!("There is no `{}` in the crate root{}", source, lev_suggestion) + format!("There is no `{}` in the crate root{}", name, lev_suggestion) } else { - format!("There is no `{}` in `{}`{}", source, module_str, lev_suggestion) + format!("There is no `{}` in `{}`{}", name, module_str, lev_suggestion) }; Failed(Some((directive.span, msg))) } else { @@ -623,9 +623,9 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { match (value_result, type_result) { (Ok(binding), _) if !binding.pseudo_vis().is_at_least(directive.vis.get(), self) => { - let msg = format!("`{}` is private, and cannot be reexported", source); - let note_msg = format!("consider marking `{}` as `pub` in the imported module", - source); + let msg = format!("`{}` is private, and cannot be reexported", name); + let note_msg = + format!("consider marking `{}` as `pub` in the imported module", name); struct_span_err!(self.session, directive.span, E0364, "{}", &msg) .span_note(directive.span, ¬e_msg) .emit(); @@ -635,15 +635,14 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { if binding.is_extern_crate() { let msg = format!("extern crate `{}` is private, and cannot be reexported \ (error E0364), consider declaring with `pub`", - source); + name); self.session.add_lint(PRIVATE_IN_PUBLIC, directive.id, directive.span, msg); } else { - let mut err = struct_span_err!(self.session, directive.span, E0365, - "`{}` is private, and cannot be reexported", - source); - err.span_label(directive.span, &format!("reexport of private `{}`", source)); - err.note(&format!("consider declaring type or module `{}` with `pub`", source)); - err.emit(); + struct_span_err!(self.session, directive.span, E0365, + "`{}` is private, and cannot be reexported", name) + .span_label(directive.span, &format!("reexport of private `{}`", name)) + .note(&format!("consider declaring type or module `{}` with `pub`", name)) + .emit(); } } From cc079c3af2624066686c2941f65f07f3aa609d23 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Thu, 18 Aug 2016 03:39:48 +0000 Subject: [PATCH 16/18] Refactor away `ImportResolvingError`. --- src/librustc_resolve/resolve_imports.rs | 40 +++++++------------------ 1 file changed, 10 insertions(+), 30 deletions(-) diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 04702d02555e1..bde175bcd6f19 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -342,12 +342,6 @@ impl<'a> Resolver<'a> { } } -struct ImportResolvingError<'a> { - import_directive: &'a ImportDirective<'a>, - span: Span, - help: String, -} - struct ImportResolver<'a, 'b: 'a> { resolver: &'a mut Resolver<'b>, } @@ -416,34 +410,33 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { self.finalize_resolutions_in(module); } - let mut errors = Vec::new(); + let mut errors = false; for i in 0 .. self.determined_imports.len() { let import = self.determined_imports[i]; if let Failed(err) = self.finalize_import(import) { + errors = true; let (span, help) = match err { Some((span, msg)) => (span, format!(". {}", msg)), None => (import.span, String::new()), }; - errors.push(ImportResolvingError { - import_directive: import, - span: span, - help: help, - }); + + // If the error is a single failed import then create a "fake" import + // resolution for it so that later resolve stages won't complain. + self.import_dummy_binding(import); + let path = import_path_to_string(&import.module_path, &import.subclass); + let error = ResolutionError::UnresolvedImport(Some((&path, &help))); + resolve_error(self.resolver, span, error); } } // Report unresolved imports only if no hard error was already reported // to avoid generating multiple errors on the same import. - if errors.len() == 0 { + if !errors { if let Some(import) = self.indeterminate_imports.iter().next() { let error = ResolutionError::UnresolvedImport(None); resolve_error(self.resolver, import.span, error); } } - - for e in errors { - self.import_resolving_error(e) - } } // Define a "dummy" resolution containing a Def::Err as a placeholder for a @@ -462,19 +455,6 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { } } - /// Resolves an `ImportResolvingError` into the correct enum discriminant - /// and passes that on to `resolve_error`. - fn import_resolving_error(&mut self, e: ImportResolvingError<'b>) { - // If the error is a single failed import then create a "fake" import - // resolution for it so that later resolve stages won't complain. - self.import_dummy_binding(e.import_directive); - let path = import_path_to_string(&e.import_directive.module_path, - &e.import_directive.subclass); - resolve_error(self.resolver, - e.span, - ResolutionError::UnresolvedImport(Some((&path, &e.help)))); - } - /// Attempts to resolve the given import. The return value indicates /// failure if we're certain the name does not exist, indeterminate if we /// don't know whether the name exists at the moment due to other From 951d3d6872daa1db11a7678606a5a9791220fb1d Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Thu, 18 Aug 2016 02:00:47 +0000 Subject: [PATCH 17/18] Fix fallout in tests. --- src/test/compile-fail/task-rng-isnt-sendable.rs | 1 - src/test/compile-fail/use-from-trait-xc.rs | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/test/compile-fail/task-rng-isnt-sendable.rs b/src/test/compile-fail/task-rng-isnt-sendable.rs index a11df776e06d3..9c0a2267d7cf8 100644 --- a/src/test/compile-fail/task-rng-isnt-sendable.rs +++ b/src/test/compile-fail/task-rng-isnt-sendable.rs @@ -16,5 +16,4 @@ fn test_send() {} pub fn main() { test_send::(); - //~^ ERROR : std::marker::Send` is not satisfied } diff --git a/src/test/compile-fail/use-from-trait-xc.rs b/src/test/compile-fail/use-from-trait-xc.rs index 687cdba1542bd..a40fa2337214e 100644 --- a/src/test/compile-fail/use-from-trait-xc.rs +++ b/src/test/compile-fail/use-from-trait-xc.rs @@ -21,10 +21,10 @@ use use_from_trait_xc::Trait::Assoc; use use_from_trait_xc::Trait::CONST; //~^ ERROR `CONST` is not directly importable -use use_from_trait_xc::Foo::new; +use use_from_trait_xc::Foo::new; //~ ERROR struct `Foo` is private //~^ ERROR unresolved import `use_from_trait_xc::Foo::new` -use use_from_trait_xc::Foo::C; +use use_from_trait_xc::Foo::C; //~ ERROR struct `Foo` is private //~^ ERROR unresolved import `use_from_trait_xc::Foo::C` use use_from_trait_xc::Bar::new as bnew; From a6e8f3ba8332cc4a7c32a64cf0df6f67b32a3dd3 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Thu, 18 Aug 2016 05:39:32 +0000 Subject: [PATCH 18/18] Add type `Determinacy`. --- src/librustc_resolve/resolve_imports.rs | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index bde175bcd6f19..c0a9ee1c48380 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -8,6 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use self::Determinacy::*; use self::ImportDirectiveSubclass::*; use Module; @@ -36,14 +37,20 @@ impl<'a> Resolver<'a> { } } +#[derive(Copy, Clone, Debug)] +pub enum Determinacy { + Determined, + Undetermined, +} + /// Contains data for specific types of import directives. #[derive(Clone, Debug)] pub enum ImportDirectiveSubclass<'a> { SingleImport { target: Name, source: Name, - value_result: Cell, bool /* determined? */>>, - type_result: Cell, bool /* determined? */>>, + value_result: Cell, Determinacy>>, + type_result: Cell, Determinacy>>, }, GlobImport { is_prelude: bool }, } @@ -53,8 +60,8 @@ impl<'a> ImportDirectiveSubclass<'a> { SingleImport { target: target, source: source, - type_result: Cell::new(Err(false)), - value_result: Cell::new(Err(false)), + type_result: Cell::new(Err(Undetermined)), + value_result: Cell::new(Err(Undetermined)), } } } @@ -497,12 +504,12 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { let mut indeterminate = false; for &(ns, result) in &[(ValueNS, value_result), (TypeNS, type_result)] { - if let Err(false) = result.get() { + if let Err(Undetermined) = result.get() { result.set({ match self.resolve_name_in_module(module, source, ns, false, None) { Success(binding) => Ok(binding), - Indeterminate => Err(false), - Failed(_) => Err(true), + Indeterminate => Err(Undetermined), + Failed(_) => Err(Determined), } }); } else { @@ -510,8 +517,8 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { }; match result.get() { - Err(false) => indeterminate = true, - Err(true) => { + Err(Undetermined) => indeterminate = true, + Err(Determined) => { self.update_resolution(directive.parent, target, ns, |_, resolution| { resolution.single_imports.directive_failed() });