From e9e178a5810a133d759b418f53a43cab1e716cab Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Sat, 26 Nov 2016 12:21:47 +0000 Subject: [PATCH] Refactor away `ResolveResult`. --- src/librustc_resolve/build_reduced_graph.rs | 5 +- src/librustc_resolve/lib.rs | 27 +----- src/librustc_resolve/macros.rs | 7 +- src/librustc_resolve/resolve_imports.rs | 92 +++++++++------------ 4 files changed, 45 insertions(+), 86 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 854c5f910c1ae..d90a49213d141 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -18,7 +18,6 @@ use resolve_imports::ImportDirective; use resolve_imports::ImportDirectiveSubclass::{self, GlobImport, SingleImport}; use {Resolver, Module, ModuleS, ModuleKind, NameBinding, NameBindingKind, ToNameBinding}; use Namespace::{self, TypeNS, ValueNS, MacroNS}; -use ResolveResult::Success; use {resolve_error, resolve_struct_error, ResolutionError}; use rustc::middle::cstore::LoadedMacro; @@ -583,7 +582,7 @@ impl<'b> Resolver<'b> { } else { for (name, span) in legacy_imports.imports { let result = self.resolve_name_in_module(module, name, MacroNS, false, None); - if let Success(binding) = result { + if let Ok(binding) = result { self.legacy_import_macro(name, binding, span, allow_shadowing); } else { span_err!(self.session, span, E0469, "imported macro not found"); @@ -595,7 +594,7 @@ impl<'b> Resolver<'b> { self.used_crates.insert(krate); self.session.cstore.export_macros(krate); let result = self.resolve_name_in_module(module, name, MacroNS, false, None); - if let Success(binding) = result { + if let Ok(binding) = result { self.macro_exports.push(Export { name: name, def: binding.def() }); } else { span_err!(self.session, span, E0470, "reexported macro not found"); diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 19764f2e68200..18e14c2dc3986 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -35,7 +35,6 @@ extern crate arena; extern crate rustc; use self::Namespace::*; -use self::ResolveResult::*; use self::FallbackSuggestion::*; use self::TypeParameters::*; use self::RibKind::*; @@ -668,22 +667,6 @@ impl<'a> Visitor for Resolver<'a> { pub type ErrorMessage = Option<(Span, String)>; -#[derive(Clone, PartialEq, Eq, Debug)] -pub enum ResolveResult { - Failed(ErrorMessage), // Failed to resolve the name, optional helpful error message. - Indeterminate, // Couldn't determine due to unresolved globs. - Success(T), // Successfully resolved the import. -} - -impl ResolveResult { - fn success(self) -> Option { - match self { - Success(t) => Some(t), - _ => None, - } - } -} - enum FallbackSuggestion { NoSuggestion, Field, @@ -1417,7 +1400,7 @@ impl<'a> Resolver<'a> { if let ModuleRibKind(module) = self.ribs[ns][i].kind { let name = ident.name; let item = self.resolve_name_in_module(module, name, ns, false, record_used); - if let Success(binding) = item { + if let Ok(binding) = item { // The ident resolves to an item. return Some(LexicalScopeBinding::Item(binding)); } @@ -1425,7 +1408,7 @@ impl<'a> Resolver<'a> { if let ModuleKind::Block(..) = module.kind { // We can see through blocks } else if !module.no_implicit_prelude { return self.prelude.and_then(|prelude| { - self.resolve_name_in_module(prelude, name, ns, false, None).success() + self.resolve_name_in_module(prelude, name, ns, false, None).ok() }).map(LexicalScopeBinding::Item) } else { return None; @@ -2398,11 +2381,7 @@ impl<'a> Resolver<'a> { allow_super = false; let binding = if let Some(module) = module { - match self.resolve_name_in_module(module, ident.name, ns, false, record_used) { - Success(binding) => Ok(binding), - Indeterminate => Err(Undetermined), - Failed(_) => Err(Determined), - } + self.resolve_name_in_module(module, ident.name, ns, false, record_used) } else { match self.resolve_ident_in_lexical_scope(ident, ns, record_used) { Some(LexicalScopeBinding::Item(binding)) => Ok(binding), diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index cdb51f459e8c2..3b34a60c58525 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -10,7 +10,6 @@ use {Module, ModuleKind, NameBinding, NameBindingKind, Resolver, AmbiguityError}; use Namespace::{self, MacroNS}; -use ResolveResult::{Success, Indeterminate, Failed}; use build_reduced_graph::BuildReducedGraphVisitor; use resolve_imports::ImportResolver; use rustc::hir::def_id::{DefId, BUILTIN_MACROS_CRATE, CRATE_DEF_INDEX, DefIndex}; @@ -254,7 +253,7 @@ impl<'a> Resolver<'a> { // Since expanded macros may not shadow the lexical scope (enforced below), // we can ignore unresolved invocations (indicated by the penultimate argument). match self.resolve_name_in_module(module, name, ns, true, record_used) { - Success(binding) => { + Ok(binding) => { let span = match record_used { Some(span) => span, None => return Some(binding), @@ -270,8 +269,8 @@ impl<'a> Resolver<'a> { potential_expanded_shadower = Some(binding); } }, - Indeterminate => return None, - Failed(..) => {} + Err(Determinacy::Undetermined) => return None, + Err(Determinacy::Determined) => {} } match module.kind { diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index c30154b9bba64..2a803d72fd1bd 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -13,8 +13,6 @@ use self::ImportDirectiveSubclass::*; use {Module, PerNS}; use Namespace::{self, TypeNS, MacroNS}; use {NameBinding, NameBindingKind, PathResult, PathScope, PrivacyError, ToNameBinding}; -use ResolveResult; -use ResolveResult::*; use Resolver; use {names_to_string, module_to_string}; use {resolve_error, ResolutionError}; @@ -142,32 +140,32 @@ impl<'a> Resolver<'a> { ns: Namespace, ignore_unresolved_invocations: bool, record_used: Option) - -> ResolveResult<&'a NameBinding<'a>> { + -> Result<&'a NameBinding<'a>, Determinacy> { self.populate_module_if_necessary(module); 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 + _ => return Err(Determined), // This happens when there is a cycle of imports }; if let Some(span) = record_used { if let Some(binding) = resolution.binding { if self.record_use(name, ns, binding, span) { - return Success(self.dummy_binding); + return Ok(self.dummy_binding); } if !self.is_accessible(binding.vis) { self.privacy_errors.push(PrivacyError(span, name, binding)); } } - return resolution.binding.map(Success).unwrap_or(Failed(None)); + return resolution.binding.ok_or(Determined); } let check_usable = |this: &mut Self, binding: &'a NameBinding<'a>| { // `extern crate` are always usable for backwards compatability, see issue #37020. let usable = this.is_accessible(binding.vis) || binding.is_extern_crate(); - if usable { Success(binding) } else { Failed(None) } + if usable { Ok(binding) } else { Err(Determined) } }; // Items and single imports are not shadowable. @@ -179,19 +177,19 @@ impl<'a> Resolver<'a> { // Check if a single import can still define the name. match resolution.single_imports { - SingleImports::AtLeastOne => return Indeterminate, + SingleImports::AtLeastOne => return Err(Undetermined), SingleImports::MaybeOne(directive) if self.is_accessible(directive.vis.get()) => { let module = match directive.imported_module.get() { Some(module) => module, - None => return Indeterminate, + None => return Err(Undetermined), }; let name = match directive.subclass { SingleImport { source, .. } => source, _ => unreachable!(), }; match self.resolve_name_in_module(module, name, ns, false, None) { - Failed(_) => {} - _ => return Indeterminate, + Err(Determined) => {} + _ => return Err(Undetermined), } } SingleImports::MaybeOne(_) | SingleImports::None => {}, @@ -204,7 +202,7 @@ impl<'a> Resolver<'a> { Some(binding) if no_unresolved_invocations || ns == MacroNS => return check_usable(self, binding), None if no_unresolved_invocations => {} - _ => return Indeterminate, + _ => return Err(Undetermined), } // Check if the globs are determined @@ -212,16 +210,16 @@ impl<'a> Resolver<'a> { if self.is_accessible(directive.vis.get()) { if let Some(module) = directive.imported_module.get() { let result = self.resolve_name_in_module(module, name, ns, false, None); - if let Indeterminate = result { - return Indeterminate; + if let Err(Undetermined) = result { + return Err(Undetermined); } } else { - return Indeterminate; + return Err(Undetermined); } } } - Failed(None) + Err(Determined) } // Add an import directive to the current module. @@ -421,9 +419,8 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { prev_num_indeterminates = self.indeterminate_imports.len(); for import in mem::replace(&mut self.indeterminate_imports, Vec::new()) { match self.resolve_import(&import) { - Failed(_) => self.determined_imports.push(import), - Indeterminate => self.indeterminate_imports.push(import), - Success(()) => self.determined_imports.push(import), + true => self.determined_imports.push(import), + false => self.indeterminate_imports.push(import), } } } @@ -437,19 +434,15 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { 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) { + if let Some(err) = self.finalize_import(import) { errors = true; - let (span, help) = match err { - Some((span, msg)) => (span, msg), - None => continue, - }; // 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); + let error = ResolutionError::UnresolvedImport(Some((&path, &err))); + resolve_error(self.resolver, import.span, error); } } @@ -463,12 +456,9 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { } } - /// 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 - /// currently-unresolved imports, or success if we know the name exists. + /// Attempts to resolve the given import, returning true if its resolution is determined. /// If successful, the resolved bindings are written into the module. - fn resolve_import(&mut self, directive: &'b ImportDirective<'b>) -> ResolveResult<()> { + fn resolve_import(&mut self, directive: &'b ImportDirective<'b>) -> bool { debug!("(resolving import for module) resolving import `{}::...` in `{}`", names_to_string(&directive.module_path), module_to_string(self.current_module)); @@ -487,8 +477,8 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { match result { PathResult::Module(module) => module, - PathResult::Indeterminate => return Indeterminate, - _ => return Failed(None), + PathResult::Indeterminate => return false, + _ => return true, } }; @@ -497,7 +487,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { SingleImport { source, target, ref result } => (source, target, result), GlobImport { .. } => { self.resolve_glob_import(directive); - return Success(()); + return true; } _ => unreachable!(), }; @@ -505,13 +495,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { let mut indeterminate = false; self.per_ns(|this, ns| { if let Err(Undetermined) = result[ns].get() { - result[ns].set({ - match this.resolve_name_in_module(module, source, ns, false, None) { - Success(binding) => Ok(binding), - Indeterminate => Err(Undetermined), - Failed(_) => Err(Determined), - } - }); + result[ns].set(this.resolve_name_in_module(module, source, ns, false, None)); } else { return }; @@ -543,37 +527,35 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { } }); - if indeterminate { Indeterminate } else { Success(()) } + !indeterminate } - fn finalize_import(&mut self, directive: &'b ImportDirective<'b>) -> ResolveResult<()> { + // If appropriate, returns an error to report. + fn finalize_import(&mut self, directive: &'b ImportDirective<'b>) -> Option { self.current_module = directive.parent; let ImportDirective { ref module_path, span, .. } = *directive; let module_result = self.resolve_path(&module_path, PathScope::Import, None, Some(span)); let module = match module_result { PathResult::Module(module) => module, - PathResult::NonModule(..) => return Success(()), - PathResult::Indeterminate => return Indeterminate, PathResult::Failed(msg, _) => { let mut path = vec![keywords::SelfValue.ident()]; path.extend(module_path); let result = self.resolve_path(&path, PathScope::Import, None, None); return if let PathResult::Module(..) = result { - let msg = format!("Did you mean `self::{}`?", &names_to_string(module_path)); - Failed(Some((span, msg))) + Some(format!("Did you mean `self::{}`?", &names_to_string(module_path))) } else { - Failed(Some((span, msg))) + Some(msg) }; }, + _ => return None, }; let (name, result) = match directive.subclass { SingleImport { source, ref result, .. } => (source, result), 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))); + return Some("Cannot glob-import a module into itself.".to_string()); } GlobImport { is_prelude, ref max_vis } => { if !is_prelude && @@ -582,7 +564,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { let msg = "A non-empty glob must import something with the glob's visibility"; self.session.span_err(directive.span, msg); } - return Success(()); + return None; } _ => unreachable!(), }; @@ -602,7 +584,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { let mut all_ns_failed = true; self.per_ns(|this, ns| { match this.resolve_name_in_module(module, name, ns, false, Some(span)) { - Success(_) => all_ns_failed = false, + Ok(_) => all_ns_failed = false, _ => {} } }); @@ -627,11 +609,11 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { } else { format!("no `{}` in `{}`{}", name, module_str, lev_suggestion) }; - Failed(Some((directive.span, msg))) + Some(msg) } else { // `resolve_name_in_module` reported a privacy error. self.import_dummy_binding(directive); - Success(()) + None } } @@ -680,7 +662,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { }); debug!("(resolving single import) successfully resolved import"); - return Success(()); + None } fn resolve_glob_import(&mut self, directive: &'b ImportDirective<'b>) {