From: Jeffrey Seyfried Date: Mon, 15 Aug 2016 08:19:09 +0000 (+0000) Subject: Refactor out `finalize_import()` from `resolve_import()`. X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=fbc322975f198a98d917b5dad0eee557f9889508;p=rust.git Refactor out `finalize_import()` from `resolve_import()`. --- diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 68964396405..a0f0fccac86 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 @@ pub fn new(session: &'a Session, make_glob_map: MakeGlobMap, arenas: &'a Resolve 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 0757ff7ddbf..c2f8e31277c 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -144,6 +144,7 @@ fn resolution(&self, module: Module<'a>, name: Name, ns: Namespace) /// 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 @@ pub fn resolve_name_in_module(&mut self, _ => 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 @@ pub fn resolve_name_in_module(&mut self, // 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 @@ fn try_result(&mut self, // 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 @@ fn try_result(&mut self, 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 @@ fn set_current_module(&mut self, module: Module<'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 @@ fn resolve_imports(&mut self) { 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 @@ fn resolve_imports(&mut self) { 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 @@ fn resolve_import(&mut self, directive: &'b ImportDirective<'b>) -> ResolveResul 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 @@ fn resolve_import(&mut self, directive: &'b ImportDirective<'b>) -> ResolveResul 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 @@ fn resolve_import(&mut self, directive: &'b ImportDirective<'b>) -> ResolveResul 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 @@ fn resolve_import(&mut self, directive: &'b ImportDirective<'b>) -> ResolveResul 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 @@ fn resolve_import(&mut self, directive: &'b ImportDirective<'b>) -> ResolveResul .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 @@ fn resolve_import(&mut self, directive: &'b ImportDirective<'b>) -> ResolveResul 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 @@ fn resolve_glob_import(&mut self, target_module: Module<'b>, directive: &'b Impo 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,