Refactor away ResolveResult.

jseyfried · jseyfried · commit e9e178a5810a · 2016-11-29T00:18:39.000Z
diff --git 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
@@ -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<T> {
-    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<T> ResolveResult<T> {
-    fn success(self) -> Option<T> {
-        match self {
-            Success(t) => Some(t),
-            _ => None,
-        }
-    }
-}
-
 enum FallbackSuggestion {
     NoSuggestion,
     Field,
@@ -1417,15 +1400,15 @@ 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));
                 }
 
                 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
@@ -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
@@ -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<Span>)
-                                  -> 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,24 +202,24 @@ 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
         for directive in module.globs.borrow().iter() {
             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,21 +487,15 @@ 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!(),
         };
 
         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<String> {
         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>) {
