From cb9f14e987732fa1406911633d9bf4e7eee45978 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Tue, 22 Nov 2016 11:32:52 +0000 Subject: [PATCH 1/6] Use `Def::Err` to signal that an error has already been reported where possible. --- src/librustc_resolve/lib.rs | 111 ++++++++++++++++-------------------- 1 file changed, 48 insertions(+), 63 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 058c8266a35e9..7b73582449bfc 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -584,13 +584,9 @@ impl<'a> Visitor for Resolver<'a> { self.resolve_type(ty); } fn visit_poly_trait_ref(&mut self, tref: &ast::PolyTraitRef, m: &ast::TraitBoundModifier) { - match self.resolve_trait_reference(tref.trait_ref.ref_id, &tref.trait_ref.path, 0, None) { - Ok(def) => self.record_def(tref.trait_ref.ref_id, def), - Err(_) => { - // error already reported - self.record_def(tref.trait_ref.ref_id, err_path_resolution()) - } - } + let def = + self.resolve_trait_reference(tref.trait_ref.ref_id, &tref.trait_ref.path, 0, None); + self.record_def(tref.trait_ref.ref_id, def); visit::walk_poly_trait_ref(self, tref, m); } fn visit_variant(&mut self, @@ -1205,8 +1201,7 @@ impl<'a> hir::lowering::Resolver for Resolver<'a> { let namespace = if is_value { ValueNS } else { TypeNS }; match self.resolve_crate_relative_path(path.span, &path.segments, namespace) { Ok(binding) => path.def = binding.def(), - Err(true) => {} - Err(false) => { + None => { let path_name = &format!("{}", path); let error = ResolutionError::UnresolvedName { @@ -1844,12 +1839,11 @@ impl<'a> Resolver<'a> { match self.resolve_crate_relative_path(prefix.span, &prefix.segments, TypeNS) { - Ok(binding) => { + Some(binding) => { let def = binding.def(); self.record_def(item.id, PathResolution::new(def)); } - Err(true) => self.record_def(item.id, err_path_resolution()), - Err(false) => { + None => { resolve_error(self, prefix.span, ResolutionError::FailedToResolve( @@ -1935,14 +1929,14 @@ impl<'a> Resolver<'a> { trait_path: &Path, path_depth: usize, generics: Option<&Generics>) - -> Result { - self.resolve_path(id, trait_path, path_depth, TypeNS).and_then(|path_res| { + -> PathResolution { + if let Some(path_res) = self.resolve_path(id, trait_path, path_depth, TypeNS) { match path_res.base_def { Def::Trait(_) => { debug!("(resolving trait) found trait def: {:?}", path_res); - return Ok(path_res); + return path_res; } - Def::Err => return Err(true), + Def::Err => return err_path_resolution(), _ => {} } @@ -1963,10 +1957,7 @@ impl<'a> Resolver<'a> { err.note(&format!("type aliases cannot be used for traits")); } err.emit(); - Err(true) - }).map_err(|error_reported| { - if error_reported { return } - + } else { // find possible candidates let trait_name = trait_path.segments.last().unwrap().identifier.name; let candidates = @@ -1988,7 +1979,8 @@ impl<'a> Resolver<'a> { ); resolve_error(self, trait_path.span, error); - }) + } + err_path_resolution() } fn with_current_self_type(&mut self, self_type: &Ty, f: F) -> T @@ -2011,16 +2003,13 @@ impl<'a> Resolver<'a> { let mut new_val = None; let mut new_id = None; if let Some(trait_ref) = opt_trait_ref { - if let Ok(path_res) = self.resolve_trait_reference(trait_ref.ref_id, - &trait_ref.path, - 0, - generics) { - assert!(path_res.depth == 0); - self.record_def(trait_ref.ref_id, path_res); + let path_res = + self.resolve_trait_reference(trait_ref.ref_id, &trait_ref.path, 0, generics); + assert!(path_res.depth == 0); + self.record_def(trait_ref.ref_id, path_res); + if path_res.base_def != Def::Err { new_val = Some((path_res.base_def.def_id(), trait_ref.clone())); new_id = Some(path_res.base_def.def_id()); - } else { - self.record_def(trait_ref.ref_id, err_path_resolution()); } visit::walk_trait_ref(self, trait_ref); } @@ -2276,9 +2265,8 @@ impl<'a> Resolver<'a> { self.record_def(ty.id, err_path_resolution()); // Keep reporting some errors even if they're ignored above. - if let Err(true) = self.resolve_path(ty.id, path, 0, TypeNS) { - // `resolve_path` already reported the error - } else { + let result = self.resolve_path(ty.id, path, 0, TypeNS); + if result.map(|resolution| resolution.base_def) != Some(Def::Err) { let kind = if maybe_qself.is_some() { "associated type" } else { @@ -2420,7 +2408,7 @@ impl<'a> Resolver<'a> { resolution } } else { - if let Err(false) = self.resolve_path(pat_id, path, 0, namespace) { + if self.resolve_path(pat_id, path, 0, namespace).is_none() { resolve_error( self, path.span, @@ -2553,7 +2541,7 @@ impl<'a> Resolver<'a> { } max_assoc_types = path.segments.len() - qself.position; // Make sure the trait is valid. - let _ = self.resolve_trait_reference(id, path, max_assoc_types, None); + self.resolve_trait_reference(id, path, max_assoc_types, None); } None => { max_assoc_types = path.segments.len(); @@ -2561,18 +2549,20 @@ impl<'a> Resolver<'a> { } let mut resolution = self.with_no_errors(|this| { - this.resolve_path(id, path, 0, namespace).ok() + this.resolve_path(id, path, 0, namespace) }); + if resolution.map(|res| res.base_def) == Some(Def::Err) { resolution = None; } for depth in 1..max_assoc_types { if resolution.is_some() { break; } self.with_no_errors(|this| { - let partial_resolution = this.resolve_path(id, path, depth, TypeNS).ok(); + let partial_resolution = this.resolve_path(id, path, depth, TypeNS); if let Some(Def::Mod(..)) = partial_resolution.map(|r| r.base_def) { // Modules cannot have associated items } else { resolution = partial_resolution; + if resolution.map(|res| res.base_def) == Some(Def::Err) { resolution = None; } } }); } @@ -2582,7 +2572,7 @@ impl<'a> Resolver<'a> { /// Skips `path_depth` trailing segments, which is also reflected in the /// returned value. See `hir::def::PathResolution` for more info. fn resolve_path(&mut self, id: NodeId, path: &Path, path_depth: usize, namespace: Namespace) - -> Result { + -> Option { debug!("resolve_path(id={:?} path={:?}, path_depth={:?})", id, path, path_depth); let span = path.span; @@ -2623,15 +2613,15 @@ 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, Some(span)).ok_or(false); - return def.and_then(|def| self.adjust_local_def(def, span).ok_or(true)).map(mk_res); + let def = resolve_identifier_with_fallback(self, Some(span)); + return def.map(|def| mk_res(self.adjust_local_def(def, span))); } 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() == ud.def && - segments[0].identifier.name != "$crate" => { + (Some(binding), Some(ref ud)) if binding.def() == ud.def && + segments[0].identifier.name != "$crate" => { self.session .add_lint(lint::builtin::UNUSED_QUALIFICATIONS, id, @@ -2659,7 +2649,7 @@ impl<'a> Resolver<'a> { } // Resolve a local definition, potentially adjusting for closures. - fn adjust_local_def(&mut self, local_def: LocalDef, span: Span) -> Option { + fn adjust_local_def(&mut self, local_def: LocalDef, span: Span) -> Def { let ribs = match local_def.ribs { Some((ns, i)) => &self.ribs[ns][i + 1..], None => &[] as &[_], @@ -2705,14 +2695,14 @@ impl<'a> Resolver<'a> { resolve_error(self, span, ResolutionError::CannotCaptureDynamicEnvironmentInFnItem); - return None; + return Def::Err; } ConstantItemRibKind => { // Still doesn't deal with upvars resolve_error(self, span, ResolutionError::AttemptToUseNonConstantValueInConstant); - return None; + return Def::Err; } } } @@ -2731,19 +2721,19 @@ impl<'a> Resolver<'a> { resolve_error(self, span, ResolutionError::TypeParametersFromOuterFunction); - return None; + return Def::Err; } ConstantItemRibKind => { // see #9186 resolve_error(self, span, ResolutionError::OuterTypeParameterContext); - return None; + return Def::Err; } } } } _ => {} } - return Some(def); + return def; } // resolve a "module-relative" path, e.g. a::b::c @@ -2751,8 +2741,7 @@ impl<'a> Resolver<'a> { span: Span, segments: &[ast::PathSegment], namespace: Namespace) - -> Result<&'a NameBinding<'a>, - bool /* true if an error was reported */> { + -> Option<&'a NameBinding<'a>> { let module_path = segments.split_last().unwrap().1.iter().map(|ps| ps.identifier).collect::>(); @@ -2761,22 +2750,20 @@ impl<'a> Resolver<'a> { if let Some((span, msg)) = err { resolve_error(self, span, ResolutionError::FailedToResolve(&msg)); } - return Err(true); + return Some(self.dummy_binding); } - Indeterminate => return Err(false), + Indeterminate => return None, Success(module) => module, }; let name = segments.last().unwrap().identifier.name; - let result = self.resolve_name_in_module(module, name, namespace, false, Some(span)); - result.success().ok_or(false) + self.resolve_name_in_module(module, name, namespace, false, Some(span)).success() } /// Invariant: This must be called only during main resolution, not during /// import resolution. fn resolve_crate_relative_path(&mut self, span: Span, segments: &[T], namespace: Namespace) - -> Result<&'a NameBinding<'a>, - bool /* true if an error was reported */> + -> Option<&'a NameBinding<'a>> where T: Named, { let module_path = segments.split_last().unwrap().1.iter().map(T::ident).collect::>(); @@ -2787,17 +2774,16 @@ impl<'a> Resolver<'a> { if let Some((span, msg)) = err { resolve_error(self, span, ResolutionError::FailedToResolve(&msg)); } - return Err(true); + return Some(self.dummy_binding); } - Indeterminate => return Err(false), + Indeterminate => return None, Success(module) => module, }; let name = segments.last().unwrap().ident().name; - let result = self.resolve_name_in_module(module, name, namespace, false, Some(span)); - result.success().ok_or(false) + self.resolve_name_in_module(module, name, namespace, false, Some(span)).success() } fn with_no_errors(&mut self, f: F) -> T @@ -2963,7 +2949,7 @@ impl<'a> Resolver<'a> { self.record_def(expr.id, err_path_resolution()); - if let Ok(Def::Struct(..)) = type_res.map(|r| r.base_def) { + if let Some(Def::Struct(..)) = type_res.map(|r| r.base_def) { let error_variant = ResolutionError::StructVariantUsedAsFunction(&path_name); let mut err = resolve_struct_error(self, expr.span, error_variant); @@ -2979,9 +2965,8 @@ impl<'a> Resolver<'a> { err.emit(); } else { // Keep reporting some errors even if they're ignored above. - if let Err(true) = self.resolve_path(expr.id, path, 0, ValueNS) { - // `resolve_path` already reported the error - } else { + let result = self.resolve_path(expr.id, path, 0, ValueNS); + if result.map(|resolution| resolution.base_def) != Some(Def::Err) { let mut method_scope = false; let mut is_static = false; self.ribs[ValueNS].iter().rev().all(|rib| { From af2d89c7f64a590b5c9e8445fc03716cf0dcbca1 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Fri, 25 Nov 2016 06:07:21 +0000 Subject: [PATCH 2/6] Refactor path resoloution. --- src/librustc/hir/lowering.rs | 4 +- src/librustc_resolve/lib.rs | 793 ++++++++---------------- src/librustc_resolve/resolve_imports.rs | 34 +- src/test/compile-fail/issue-28388-1.rs | 2 +- 4 files changed, 287 insertions(+), 546 deletions(-) diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 39d0ed9a67b09..d113f6b5e4e76 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -77,7 +77,7 @@ pub struct LoweringContext<'a> { pub trait Resolver { // Resolve a global hir path generated by the lowerer when expanding `for`, `if let`, etc. - fn resolve_generated_global_path(&mut self, path: &mut hir::Path, is_value: bool); + fn resolve_hir_path(&mut self, path: &mut hir::Path, is_value: bool); // Obtain the resolution for a node id fn get_resolution(&mut self, id: NodeId) -> Option; @@ -2091,7 +2091,7 @@ impl<'a> LoweringContext<'a> { segments: segments.into(), }; - self.resolver.resolve_generated_global_path(&mut path, is_value); + self.resolver.resolve_hir_path(&mut path, is_value); path } diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 7b73582449bfc..19764f2e68200 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -39,8 +39,6 @@ use self::ResolveResult::*; use self::FallbackSuggestion::*; use self::TypeParameters::*; use self::RibKind::*; -use self::UseLexicalScopeFlag::*; -use self::ModulePrefixResult::*; use rustc::hir::map::{Definitions, DefCollector}; use rustc::hir::{self, PrimTy, TyBool, TyChar, TyFloat, TyInt, TyUint, TyStr}; @@ -57,6 +55,7 @@ use syntax::ext::hygiene::{Mark, SyntaxContext}; use syntax::ast::{self, FloatTy}; use syntax::ast::{CRATE_NODE_ID, Name, NodeId, Ident, SpannedIdent, IntTy, UintTy}; use syntax::ext::base::SyntaxExtension; +use syntax::ext::base::Determinacy::{Determined, Undetermined}; use syntax::symbol::{Symbol, keywords}; use syntax::util::lev_distance::find_best_match_for_name; @@ -191,10 +190,6 @@ fn resolve_struct_error<'b, 'a: 'b, 'c>(resolver: &'b Resolver<'a>, span: syntax_pos::Span, resolution_error: ResolutionError<'c>) -> DiagnosticBuilder<'a> { - if !resolver.emit_errors { - return resolver.session.diagnostic().struct_dummy(); - } - match resolution_error { ResolutionError::TypeParametersFromOuterFunction => { let mut err = struct_span_err!(resolver.session, @@ -584,8 +579,9 @@ impl<'a> Visitor for Resolver<'a> { self.resolve_type(ty); } fn visit_poly_trait_ref(&mut self, tref: &ast::PolyTraitRef, m: &ast::TraitBoundModifier) { - let def = - self.resolve_trait_reference(tref.trait_ref.ref_id, &tref.trait_ref.path, 0, None); + let ast::Path { ref segments, span, global } = tref.trait_ref.path; + let path: Vec<_> = segments.iter().map(|seg| seg.identifier).collect(); + let def = self.resolve_trait_reference(&path, global, None, span); self.record_def(tref.trait_ref.ref_id, def); visit::walk_poly_trait_ref(self, tref, m); } @@ -737,17 +733,6 @@ enum RibKind<'a> { MacroDefinition(Mark), } -#[derive(Copy, Clone)] -enum UseLexicalScopeFlag { - DontUseLexicalScope, - UseLexicalScope, -} - -enum ModulePrefixResult<'a> { - NoPrefixFound, - PrefixFound(Module<'a>, usize), -} - /// One local scope. #[derive(Debug)] struct Rib<'a> { @@ -765,33 +750,18 @@ impl<'a> Rib<'a> { } /// A definition along with the index of the rib it was found on +#[derive(Copy, Clone)] struct LocalDef { ribs: Option<(Namespace, usize)>, def: Def, } -impl LocalDef { - fn from_def(def: Def) -> Self { - LocalDef { - ribs: None, - def: def, - } - } -} - enum LexicalScopeBinding<'a> { Item(&'a NameBinding<'a>), - LocalDef(LocalDef), + Def(Def), } impl<'a> LexicalScopeBinding<'a> { - fn local_def(self) -> LocalDef { - match self { - LexicalScopeBinding::LocalDef(local_def) => local_def, - LexicalScopeBinding::Item(binding) => LocalDef::from_def(binding.def()), - } - } - fn item(self) -> Option<&'a NameBinding<'a>> { match self { LexicalScopeBinding::Item(binding) => Some(binding), @@ -800,6 +770,21 @@ impl<'a> LexicalScopeBinding<'a> { } } +#[derive(Copy, Clone)] +enum PathScope { + Global, + Lexical, + Import, +} + +#[derive(Clone)] +enum PathResult<'a> { + Module(Module<'a>), + NonModule(PathResolution), + Indeterminate, + Failed(String, bool /* is the error from the last segment? */), +} + enum ModuleKind { Block(NodeId), Def(Def, Name), @@ -1107,11 +1092,6 @@ pub struct Resolver<'a> { module_map: NodeMap>, extern_crate_roots: FxHashMap<(CrateNum, bool /* MacrosOnly? */), Module<'a>>, - // Whether or not to print error messages. Can be set to true - // when getting additional info for error message suggestions, - // so as to avoid printing duplicate errors - emit_errors: bool, - pub make_glob_map: bool, // Maps imports to the names of items actually imported (this actually maps // all imports, but only glob imports are actually interesting). @@ -1197,22 +1177,23 @@ impl<'a> ty::NodeIdTree for Resolver<'a> { } impl<'a> hir::lowering::Resolver for Resolver<'a> { - fn resolve_generated_global_path(&mut self, path: &mut hir::Path, is_value: bool) { + fn resolve_hir_path(&mut self, path: &mut hir::Path, is_value: bool) { let namespace = if is_value { ValueNS } else { TypeNS }; - match self.resolve_crate_relative_path(path.span, &path.segments, namespace) { - Ok(binding) => path.def = binding.def(), - None => { - let path_name = &format!("{}", path); - let error = - ResolutionError::UnresolvedName { - path: path_name, - message: "", - context: UnresolvedNameContext::Other, - is_static_method: false, - is_field: false, - def: Def::Err, - }; - resolve_error(self, path.span, error); + let hir::Path { ref segments, span, global, ref mut def } = *path; + let path: Vec<_> = segments.iter().map(|seg| Ident::with_empty_ctxt(seg.name)).collect(); + let scope = if global { PathScope::Global } else { PathScope::Lexical }; + match self.resolve_path(&path, scope, Some(namespace), Some(span)) { + PathResult::Module(module) => *def = module.def().unwrap(), + PathResult::NonModule(path_res) if path_res.depth == 0 => *def = path_res.base_def, + PathResult::NonModule(..) => match self.resolve_path(&path, scope, None, Some(span)) { + PathResult::Failed(msg, _) => { + resolve_error(self, span, ResolutionError::FailedToResolve(&msg)); + } + _ => {} + }, + PathResult::Indeterminate => unreachable!(), + PathResult::Failed(msg, _) => { + resolve_error(self, span, ResolutionError::FailedToResolve(&msg)); } } } @@ -1230,22 +1211,6 @@ impl<'a> hir::lowering::Resolver for Resolver<'a> { } } -trait Named { - fn ident(&self) -> Ident; -} - -impl Named for ast::PathSegment { - fn ident(&self) -> Ident { - self.identifier - } -} - -impl Named for hir::PathSegment { - fn ident(&self) -> Ident { - Ident::with_empty_ctxt(self.name) - } -} - impl<'a> Resolver<'a> { pub fn new(session: &'a Session, krate: &Crate, @@ -1307,7 +1272,6 @@ impl<'a> Resolver<'a> { module_map: module_map, extern_crate_roots: FxHashMap(), - emit_errors: true, make_glob_map: make_glob_map == MakeGlobMap::Yes, glob_map: NodeMap(), @@ -1413,163 +1377,6 @@ impl<'a> Resolver<'a> { } } - fn expect_module(&mut self, name: Name, binding: &'a NameBinding<'a>, span: Option) - -> ResolveResult> { - match binding.module() { - Ok(module) => Success(module), - Err(true) => Failed(None), - Err(false) => { - let msg = format!("Not a module `{}`", name); - Failed(span.map(|span| (span, msg))) - } - } - } - - /// Resolves the given module path from the given root `search_module`. - fn resolve_module_path_from_root(&mut self, - mut search_module: Module<'a>, - module_path: &[Ident], - index: usize, - span: Option) - -> 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, None) { - Success(binding) if binding.is_extern_crate() => Some(module), - _ => if let (&ModuleKind::Def(..), Some(parent)) = (&module.kind, module.parent) { - search_parent_externals(this, needle, parent) - } else { - None - }, - } - } - - let mut index = index; - let module_path_len = module_path.len(); - - // Resolve the module part of the path. This does not involve looking - // upward though scope chains; we simply resolve names directly in - // modules as we go. - while index < module_path_len { - let name = module_path[index].name; - match self.resolve_name_in_module(search_module, name, TypeNS, false, span) { - Failed(_) => { - let module_name = module_to_string(search_module); - let msg = if "???" == &module_name { - 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(current_module); - - let prefix = if target_mod_str == current_mod_str { - "self::".to_string() - } else { - format!("{}::", target_mod_str) - }; - - format!("Did you mean `{}{}`?", prefix, path_str) - } - None => format!("Maybe a missing `extern crate {};`?", name), - } - } else { - format!("Could not find `{}` in `{}`", name, module_name) - }; - - return Failed(span.map(|span| (span, msg))); - } - Indeterminate => { - debug!("(resolving module path for import) module resolution is \ - indeterminate: {}", - name); - return Indeterminate; - } - Success(binding) => { - // Check to see whether there are type bindings, and, if - // so, whether there is a module within. - match self.expect_module(name, binding, span) { - Success(module) => search_module = module, - result @ _ => return result, - } - } - } - - index += 1; - } - - return Success(search_module); - } - - /// Attempts to resolve the module part of an import directive or path - /// rooted at the given module. - fn resolve_module_path(&mut self, - module_path: &[Ident], - use_lexical_scope: UseLexicalScopeFlag, - span: Option) - -> ResolveResult> { - if module_path.len() == 0 { - return Success(self.graph_root) // Use the crate root - } - - debug!("(resolving module path for import) processing `{}` rooted at `{}`", - names_to_string(module_path), - module_to_string(self.current_module)); - - // Resolve the module prefix, if any. - let module_prefix_result = self.resolve_module_prefix(module_path, span); - - let search_module; - let start_index; - match module_prefix_result { - Failed(err) => return Failed(err), - Indeterminate => { - debug!("(resolving module path for import) indeterminate; bailing"); - return Indeterminate; - } - Success(NoPrefixFound) => { - // There was no prefix, so we're considering the first element - // of the path. How we handle this depends on whether we were - // instructed to use lexical scope or not. - match use_lexical_scope { - DontUseLexicalScope => { - // This is a crate-relative path. We will start the - // resolution process at index zero. - search_module = self.graph_root; - start_index = 0; - } - UseLexicalScope => { - // This is not a crate-relative path. We resolve the - // first component of the path in the current lexical - // scope and then proceed to resolve below that. - let ident = module_path[0]; - let lexical_binding = - self.resolve_ident_in_lexical_scope(ident, TypeNS, span); - if let Some(binding) = lexical_binding.and_then(LexicalScopeBinding::item) { - match self.expect_module(ident.name, binding, span) { - Success(containing_module) => { - search_module = containing_module; - start_index = 1; - } - result @ _ => return result, - } - } else { - let msg = - format!("Use of undeclared type or module `{}`", ident.name); - return Failed(span.map(|span| (span, msg))); - } - } - } - } - Success(PrefixFound(ref containing_module, index)) => { - search_module = containing_module; - start_index = index; - } - } - - 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. /// More specifically, we proceed up the hierarchy of scopes and return the binding for /// `ident` in the first scope that defines it (or None if no scopes define it). @@ -1600,9 +1407,10 @@ impl<'a> Resolver<'a> { for i in (0 .. self.ribs[ns].len()).rev() { if let Some(def) = self.ribs[ns][i].bindings.get(&ident).cloned() { // The ident resolves to a type parameter or local variable. - return Some(LexicalScopeBinding::LocalDef(LocalDef { - ribs: Some((ns, i)), - def: def, + return Some(LexicalScopeBinding::Def(if let Some(span) = record_used { + self.adjust_local_def(LocalDef { ribs: Some((ns, i)), def: def }, span) + } else { + def })); } @@ -1637,45 +1445,6 @@ impl<'a> Resolver<'a> { None } - /// 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: &[Ident], span: Option) - -> ResolveResult> { - if module_path[0].name == "$crate" { - return Success(PrefixFound(self.resolve_crate_var(module_path[0].ctxt), 1)); - } - - // Start at the current module if we see `self` or `super`, or at the - // top of the crate otherwise. - let mut i = match &*module_path[0].name.as_str() { - "self" => 1, - "super" => 0, - _ => return Success(NoPrefixFound), - }; - - let mut containing_module = - self.module_map[&self.current_module.normal_ancestor_id.unwrap()]; - - // Now loop through all the `super`s we find. - while i < module_path.len() && module_path[i].name == "super" { - debug!("(resolving module prefix) resolving `super` at {}", - module_to_string(&containing_module)); - if let Some(parent) = containing_module.parent { - containing_module = self.module_map[&parent.normal_ancestor_id.unwrap()]; - i += 1; - } else { - let msg = "There are too many initial `super`s.".into(); - return Failed(span.map(|span| (span, msg))); - } - } - - debug!("(resolving module prefix) finished resolving prefix at {}", - module_to_string(&containing_module)); - - return Success(PrefixFound(containing_module, i)); - } - fn resolve_crate_var(&mut self, mut crate_var_ctxt: SyntaxContext) -> Module<'a> { while crate_var_ctxt.source().0 != SyntaxContext::empty() { crate_var_ctxt = crate_var_ctxt.source().0; @@ -1834,23 +1603,32 @@ impl<'a> Resolver<'a> { ItemKind::Use(ref view_path) => { match view_path.node { ast::ViewPathList(ref prefix, ref items) => { + let path: Vec<_> = + prefix.segments.iter().map(|seg| seg.identifier).collect(); // Resolve prefix of an import with empty braces (issue #28388) if items.is_empty() && !prefix.segments.is_empty() { - match self.resolve_crate_relative_path(prefix.span, - &prefix.segments, - TypeNS) { - Some(binding) => { - let def = binding.def(); - self.record_def(item.id, PathResolution::new(def)); - } - None => { - resolve_error(self, - prefix.span, - ResolutionError::FailedToResolve( - &path_names_to_string(prefix, 0))); - self.record_def(item.id, err_path_resolution()); + let (scope, span) = (PathScope::Import, prefix.span); + // FIXME(#38012) This should be a module path, not anything in TypeNS. + let result = + self.resolve_path(&path, scope, Some(TypeNS), Some(span)); + let (def, msg) = match result { + PathResult::Module(module) => (module.def().unwrap(), None), + PathResult::NonModule(res) if res.depth == 0 => + (res.base_def, None), + PathResult::NonModule(_) => { + // Resolve a module path for better errors + match self.resolve_path(&path, scope, None, Some(span)) { + PathResult::Failed(msg, _) => (Def::Err, Some(msg)), + _ => unreachable!(), + } } + PathResult::Indeterminate => unreachable!(), + PathResult::Failed(msg, _) => (Def::Err, Some(msg)), + }; + if let Some(msg) = msg { + resolve_error(self, span, ResolutionError::FailedToResolve(&msg)); } + self.record_def(item.id, PathResolution::new(def)); } } _ => {} @@ -1925,60 +1703,51 @@ impl<'a> Resolver<'a> { } fn resolve_trait_reference(&mut self, - id: NodeId, - trait_path: &Path, - path_depth: usize, - generics: Option<&Generics>) + path: &[Ident], + global: bool, + generics: Option<&Generics>, + span: Span) -> PathResolution { - if let Some(path_res) = self.resolve_path(id, trait_path, path_depth, TypeNS) { - match path_res.base_def { - Def::Trait(_) => { - debug!("(resolving trait) found trait def: {:?}", path_res); - return path_res; - } - Def::Err => return err_path_resolution(), - _ => {} + let scope = if global { PathScope::Global } else { PathScope::Lexical }; + let def = match self.resolve_path(path, scope, None, Some(span)) { + PathResult::Module(module) => Some(module.def().unwrap()), + PathResult::NonModule(..) => return err_path_resolution(), + PathResult::Failed(msg, false) => { + resolve_error(self, span, ResolutionError::FailedToResolve(&msg)); + return err_path_resolution(); + } + _ => match self.resolve_path(path, scope, Some(TypeNS), None) { + PathResult::NonModule(path_resolution) => Some(path_resolution.base_def), + _ => None, + }, + }; + + if let Some(def) = def { + if let Def::Trait(_) = def { + return PathResolution::new(def); } - let mut err = resolve_struct_error(self, trait_path.span, { - ResolutionError::IsNotATrait(&path_names_to_string(trait_path, path_depth), - path_res.base_def.kind_name()) + let mut err = resolve_struct_error(self, span, { + ResolutionError::IsNotATrait(&names_to_string(path), def.kind_name()) }); if let Some(generics) = generics { - if let Some(span) = generics.span_for_name( - &path_names_to_string(trait_path, path_depth)) { - + if let Some(span) = generics.span_for_name(&names_to_string(path)) { err.span_label(span, &"type parameter defined here"); } } // If it's a typedef, give a note - if let Def::TyAlias(..) = path_res.base_def { + if let Def::TyAlias(..) = def { err.note(&format!("type aliases cannot be used for traits")); } err.emit(); } else { // find possible candidates - let trait_name = trait_path.segments.last().unwrap().identifier.name; - let candidates = - self.lookup_candidates( - trait_name, - TypeNS, - |def| match def { - Def::Trait(_) => true, - _ => false, - }, - ); - - // create error object - let name = &path_names_to_string(trait_path, path_depth); - let error = - ResolutionError::UndeclaredTraitName( - name, - candidates, - ); + let is_trait = |def| match def { Def::Trait(_) => true, _ => false }; + let candidates = self.lookup_candidates(path.last().unwrap().name, TypeNS, is_trait); - resolve_error(self, trait_path.span, error); + let path = names_to_string(path); + resolve_error(self, span, ResolutionError::UndeclaredTraitName(&path, candidates)); } err_path_resolution() } @@ -2003,8 +1772,9 @@ impl<'a> Resolver<'a> { let mut new_val = None; let mut new_id = None; if let Some(trait_ref) = opt_trait_ref { - let path_res = - self.resolve_trait_reference(trait_ref.ref_id, &trait_ref.path, 0, generics); + let ast::Path { ref segments, span, global } = trait_ref.path; + let path: Vec<_> = segments.iter().map(|seg| seg.identifier).collect(); + let path_res = self.resolve_trait_reference(&path, global, generics, span); assert!(path_res.depth == 0); self.record_def(trait_ref.ref_id, path_res); if path_res.base_def != Def::Err { @@ -2265,8 +2035,7 @@ impl<'a> Resolver<'a> { self.record_def(ty.id, err_path_resolution()); // Keep reporting some errors even if they're ignored above. - let result = self.resolve_path(ty.id, path, 0, TypeNS); - if result.map(|resolution| resolution.base_def) != Some(Def::Err) { + { let kind = if maybe_qself.is_some() { "associated type" } else { @@ -2408,13 +2177,8 @@ impl<'a> Resolver<'a> { resolution } } else { - if self.resolve_path(pat_id, path, 0, namespace).is_none() { - resolve_error( - self, - path.span, - ResolutionError::PatPathUnresolved(expected_what, path) - ); - } + let error = ResolutionError::PatPathUnresolved(expected_what, path); + resolve_error(self, path.span, error); err_path_resolution() }; @@ -2526,81 +2290,32 @@ impl<'a> Resolver<'a> { id: NodeId, maybe_qself: Option<&QSelf>, path: &Path, - namespace: Namespace) + ns: Namespace) -> Option { - let max_assoc_types; - - match maybe_qself { - Some(qself) => { - if qself.position == 0 { - // FIXME: Create some fake resolution that can't possibly be a type. - return Some(PathResolution { - base_def: Def::Mod(self.definitions.local_def_id(ast::CRATE_NODE_ID)), - depth: path.segments.len(), - }); - } - max_assoc_types = path.segments.len() - qself.position; - // Make sure the trait is valid. - self.resolve_trait_reference(id, path, max_assoc_types, None); - } - None => { - max_assoc_types = path.segments.len(); - } - } - - let mut resolution = self.with_no_errors(|this| { - this.resolve_path(id, path, 0, namespace) - }); - if resolution.map(|res| res.base_def) == Some(Def::Err) { resolution = None; } - for depth in 1..max_assoc_types { - if resolution.is_some() { - break; + let ast::Path { ref segments, global, span } = *path; + let path: Vec<_> = segments.iter().map(|seg| seg.identifier).collect(); + let scope = if global { PathScope::Global } else { PathScope::Lexical }; + + if let Some(qself) = maybe_qself { + if qself.position == 0 { + // FIXME: Create some fake resolution that can't possibly be a type. + return Some(PathResolution { + base_def: Def::Mod(self.definitions.local_def_id(ast::CRATE_NODE_ID)), + depth: path.len(), + }); } - self.with_no_errors(|this| { - let partial_resolution = this.resolve_path(id, path, depth, TypeNS); - if let Some(Def::Mod(..)) = partial_resolution.map(|r| r.base_def) { - // Modules cannot have associated items - } else { - resolution = partial_resolution; - if resolution.map(|res| res.base_def) == Some(Def::Err) { resolution = None; } - } - }); + // Make sure the trait is valid. + self.resolve_trait_reference(&path[..qself.position], global, None, span); } - resolution - } - - /// Skips `path_depth` trailing segments, which is also reflected in the - /// returned value. See `hir::def::PathResolution` for more info. - fn resolve_path(&mut self, id: NodeId, path: &Path, path_depth: usize, namespace: Namespace) - -> Option { - debug!("resolve_path(id={:?} path={:?}, path_depth={:?})", id, path, path_depth); - let span = path.span; - let segments = &path.segments[..path.segments.len() - path_depth]; - - let mk_res = |def| PathResolution { base_def: def, depth: path_depth }; - - if path.global { - let binding = self.resolve_crate_relative_path(span, segments, namespace); - return binding.map(|binding| mk_res(binding.def())); - } - - // Try to find a path to an item in a module. - let last_ident = segments.last().unwrap().identifier; - // Resolve a single identifier with fallback to primitive types - let resolve_identifier_with_fallback = |this: &mut Self, record_used| { - let def = this.resolve_identifier(last_ident, namespace, record_used); - match def { - None | Some(LocalDef{def: Def::Mod(..), ..}) if namespace == TypeNS => - this.primitive_type_table - .primitive_types - .get(&last_ident.name) - .map_or(def, |prim_ty| Some(LocalDef::from_def(Def::PrimTy(*prim_ty)))), - _ => def + let result = match self.resolve_path(&path, scope, Some(ns), Some(span)) { + PathResult::NonModule(path_res) => match path_res.base_def { + Def::Trait(..) if maybe_qself.is_some() => return None, + _ => path_res, + }, + PathResult::Module(module) if !module.is_normal() => { + PathResolution::new(module.def().unwrap()) } - }; - - if segments.len() == 1 { // In `a(::assoc_item)*` `a` cannot be a module. If `a` does resolve to a module we // don't report an error right away, but try to fallback to a primitive type. // So, we are still able to successfully resolve something like @@ -2613,39 +2328,140 @@ 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, Some(span)); - return def.map(|def| mk_res(self.adjust_local_def(def, span))); + _ if self.primitive_type_table.primitive_types.contains_key(&path[0].name) => { + PathResolution { + base_def: Def::PrimTy(self.primitive_type_table.primitive_types[&path[0].name]), + depth: segments.len() - 1, + } + } + PathResult::Module(module) => PathResolution::new(module.def().unwrap()), + PathResult::Failed(msg, false) => { + resolve_error(self, span, ResolutionError::FailedToResolve(&msg)); + err_path_resolution() + } + _ => return None, + }; + + if path.len() == 1 || result.base_def == Def::Err { + return Some(result); } - 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) { - (Some(binding), Some(ref ud)) if binding.def() == ud.def && - segments[0].identifier.name != "$crate" => { - self.session - .add_lint(lint::builtin::UNUSED_QUALIFICATIONS, - id, - span, - "unnecessary qualification".to_string()); + let unqualified_result = { + match self.resolve_path(&[*path.last().unwrap()], PathScope::Lexical, Some(ns), None) { + PathResult::NonModule(path_res) => path_res.base_def, + PathResult::Module(module) => module.def().unwrap(), + _ => return Some(result), } - _ => {} + }; + if result.base_def == unqualified_result && path[0].name != "$crate" { + let lint = lint::builtin::UNUSED_QUALIFICATIONS; + self.session.add_lint(lint, id, span, "unnecessary qualification".to_string()); } - qualified_binding.map(|binding| mk_res(binding.def())) + Some(result) } - // Resolve a single identifier - fn resolve_identifier(&mut self, - identifier: Ident, - namespace: Namespace, - record_used: Option) - -> Option { - if identifier.name == keywords::Invalid.name() { - return None; + fn resolve_path(&mut self, + path: &[Ident], + scope: PathScope, + opt_ns: Option, // `None` indicates a module path + record_used: Option) + -> PathResult<'a> { + let (mut module, allow_self) = match scope { + PathScope::Lexical => (None, true), + PathScope::Import => (Some(self.graph_root), true), + PathScope::Global => (Some(self.graph_root), false), + }; + let mut allow_super = allow_self; + + for (i, &ident) in path.iter().enumerate() { + let is_last = i == path.len() - 1; + let ns = if is_last { opt_ns.unwrap_or(TypeNS) } else { TypeNS }; + + if i == 0 && allow_self && ns == TypeNS && ident.name == keywords::SelfValue.name() { + module = Some(self.module_map[&self.current_module.normal_ancestor_id.unwrap()]); + continue + } else if i == 0 && allow_self && ns == TypeNS && ident.name == "$crate" { + module = Some(self.resolve_crate_var(ident.ctxt)); + continue + } else if allow_super && ns == TypeNS && ident.name == keywords::Super.name() { + let current_module = if i == 0 { self.current_module } else { module.unwrap() }; + let self_module = self.module_map[¤t_module.normal_ancestor_id.unwrap()]; + if let Some(parent) = self_module.parent { + module = Some(self.module_map[&parent.normal_ancestor_id.unwrap()]); + continue + } else { + let msg = "There are too many initial `super`s.".to_string(); + return PathResult::Failed(msg, false); + } + } + 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), + } + } else { + match self.resolve_ident_in_lexical_scope(ident, ns, record_used) { + Some(LexicalScopeBinding::Item(binding)) => Ok(binding), + Some(LexicalScopeBinding::Def(def)) if opt_ns.is_some() => { + return PathResult::NonModule(PathResolution { + base_def: def, + depth: path.len() - 1, + }); + } + _ => Err(if record_used.is_some() { Determined } else { Undetermined }), + } + }; + + match binding { + Ok(binding) => { + if let Ok(next_module) = binding.module() { + module = Some(next_module); + } else if binding.def() == Def::Err { + return PathResult::NonModule(err_path_resolution()); + } else if opt_ns.is_some() { + return PathResult::NonModule(PathResolution { + base_def: binding.def(), + depth: path.len() - i - 1, + }); + } else { + return PathResult::Failed(format!("Not a module `{}`", ident), is_last); + } + } + Err(Undetermined) => return PathResult::Indeterminate, + Err(Determined) => { + if let Some(module) = module { + if opt_ns.is_some() && !module.is_normal() { + return PathResult::NonModule(PathResolution { + base_def: module.def().unwrap(), + depth: path.len() - i, + }); + } + } + let msg = if module.and_then(ModuleS::def) == self.graph_root.def() { + let is_mod = |def| match def { Def::Mod(..) => true, _ => false }; + let mut candidates = + self.lookup_candidates(ident.name, TypeNS, is_mod).candidates; + candidates.sort_by_key(|path| (path.segments.len(), path.to_string())); + if let Some(candidate) = candidates.get(0) { + format!("Did you mean `{}`?", candidate) + } else { + format!("Maybe a missing `extern crate {};`?", ident) + } + } else if i == 0 { + format!("Use of undeclared type or module `{}`", ident) + } else { + format!("Could not find `{}` in `{}`", ident, path[i - 1]) + }; + return PathResult::Failed(msg, is_last); + } + } } - self.resolve_ident_in_lexical_scope(identifier, namespace, record_used) - .map(LexicalScopeBinding::local_def) + PathResult::Module(module.unwrap()) } // Resolve a local definition, potentially adjusting for closures. @@ -2736,65 +2552,6 @@ impl<'a> Resolver<'a> { return def; } - // resolve a "module-relative" path, e.g. a::b::c - fn resolve_module_relative_path(&mut self, - span: Span, - segments: &[ast::PathSegment], - namespace: Namespace) - -> Option<&'a NameBinding<'a>> { - let module_path = - segments.split_last().unwrap().1.iter().map(|ps| ps.identifier).collect::>(); - - let module = match self.resolve_module_path(&module_path, UseLexicalScope, Some(span)) { - Failed(err) => { - if let Some((span, msg)) = err { - resolve_error(self, span, ResolutionError::FailedToResolve(&msg)); - } - return Some(self.dummy_binding); - } - Indeterminate => return None, - Success(module) => module, - }; - - let name = segments.last().unwrap().identifier.name; - self.resolve_name_in_module(module, name, namespace, false, Some(span)).success() - } - - /// Invariant: This must be called only during main resolution, not during - /// import resolution. - fn resolve_crate_relative_path(&mut self, span: Span, segments: &[T], namespace: Namespace) - -> Option<&'a NameBinding<'a>> - where T: Named, - { - let module_path = segments.split_last().unwrap().1.iter().map(T::ident).collect::>(); - let root = self.graph_root; - - let module = match self.resolve_module_path_from_root(root, &module_path, 0, Some(span)) { - Failed(err) => { - if let Some((span, msg)) = err { - resolve_error(self, span, ResolutionError::FailedToResolve(&msg)); - } - return Some(self.dummy_binding); - } - - Indeterminate => return None, - - Success(module) => module, - }; - - let name = segments.last().unwrap().ident().name; - self.resolve_name_in_module(module, name, namespace, false, Some(span)).success() - } - - fn with_no_errors(&mut self, f: F) -> T - where F: FnOnce(&mut Resolver) -> T - { - self.emit_errors = false; - let rs = f(self); - self.emit_errors = true; - rs - } - // Calls `f` with a `Resolver` whose current lexical scope is `module`'s lexical scope, // i.e. the module's items and the prelude (unless the module is `#[no_implicit_prelude]`). // FIXME #34673: This needs testing. @@ -2918,11 +2675,7 @@ impl<'a> Resolver<'a> { let msg = format!("did you mean to write: `{} {{ /* fields */ }}`?", path_name); - if self.emit_errors { - err.help(&msg); - } else { - err.span_help(expr.span, &msg); - } + err.help(&msg); err.emit(); self.record_def(expr.id, err_path_resolution()); } else { @@ -2943,9 +2696,13 @@ impl<'a> Resolver<'a> { } else { // Be helpful if the name refers to a struct let path_name = path_names_to_string(path, 0); - let type_res = self.with_no_errors(|this| { - this.resolve_path(expr.id, path, 0, TypeNS) - }); + let ast::Path { ref segments, global, .. } = *path; + let path: Vec<_> = segments.iter().map(|seg| seg.identifier).collect(); + let scope = if global { PathScope::Global } else { PathScope::Lexical }; + let type_res = match self.resolve_path(&path, scope, Some(TypeNS), None) { + PathResult::NonModule(type_res) => Some(type_res), + _ => None, + }; self.record_def(expr.id, err_path_resolution()); @@ -2957,16 +2714,11 @@ impl<'a> Resolver<'a> { let msg = format!("did you mean to write: `{} {{ /* fields */ }}`?", path_name); - if self.emit_errors { - err.help(&msg); - } else { - err.span_help(expr.span, &msg); - } + err.help(&msg); err.emit(); } else { // Keep reporting some errors even if they're ignored above. - let result = self.resolve_path(expr.id, path, 0, ValueNS); - if result.map(|resolution| resolution.base_def) != Some(Def::Err) { + { let mut method_scope = false; let mut is_static = false; self.ribs[ValueNS].iter().rev().all(|rib| { @@ -2986,7 +2738,7 @@ impl<'a> Resolver<'a> { expr.span, ResolutionError::SelfNotAvailableInStaticMethod); } else { - let last_name = path.segments.last().unwrap().identifier.name; + let last_name = path.last().unwrap().name; let (mut msg, is_field) = match self.find_fallback_in_self_type(last_name) { NoSuggestion => { @@ -3018,16 +2770,9 @@ impl<'a> Resolver<'a> { msg = format!("did you mean {}?", msg); } else { // we display a help message if this is a module - let name_path: Vec<_> = - path.segments.iter().map(|seg| seg.identifier).collect(); - - match self.resolve_module_path(&name_path[..], - UseLexicalScope, - Some(expr.span)) { - Success(e) => { - if let Some(def_type) = e.def() { - def = def_type; - } + match self.resolve_path(&path, scope, None, None) { + PathResult::Module(module) => { + def = module.def().unwrap(); context = UnresolvedNameContext::PathIsMod(parent); }, _ => {}, @@ -3270,7 +3015,7 @@ impl<'a> Resolver<'a> { segms.push(segment); let path = Path { span: span, - global: true, + global: false, segments: segms, }; // the entity is accessible in the following cases: @@ -3320,34 +3065,32 @@ impl<'a> Resolver<'a> { } fn resolve_visibility(&mut self, vis: &ast::Visibility) -> ty::Visibility { - let (path, id) = match *vis { + let (segments, span, id) = match *vis { 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::Restricted { ref path, id } => (&path.segments, path.span, id), ast::Visibility::Inherited => { return ty::Visibility::Restricted(self.current_module.normal_ancestor_id.unwrap()); } }; - let segments: Vec<_> = path.segments.iter().map(|seg| seg.identifier).collect(); + let path: Vec<_> = segments.iter().map(|seg| seg.identifier).collect(); let mut path_resolution = err_path_resolution(); - let vis = match self.resolve_module_path(&segments, DontUseLexicalScope, Some(path.span)) { - Success(module) => { + let vis = match self.resolve_path(&path, PathScope::Import, None, Some(span)) { + PathResult::Module(module) => { path_resolution = PathResolution::new(module.def().unwrap()); ty::Visibility::Restricted(module.normal_ancestor_id.unwrap()) } - Indeterminate => unreachable!(), - Failed(err) => { - if let Some((span, msg)) = err { - self.session.span_err(span, &format!("failed to resolve module path. {}", msg)); - } + PathResult::Failed(msg, _) => { + self.session.span_err(span, &format!("failed to resolve module path. {}", msg)); ty::Visibility::Public } + _ => ty::Visibility::Public, }; self.def_map.insert(id, path_resolution); if !self.is_accessible(vis) { let msg = format!("visibilities can only be restricted to ancestor modules"); - self.session.span_err(path.span, &msg); + self.session.span_err(span, &msg); } vis } diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 5c9e8bb93718a..c30154b9bba64 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -12,11 +12,10 @@ use self::ImportDirectiveSubclass::*; use {Module, PerNS}; use Namespace::{self, TypeNS, MacroNS}; -use {NameBinding, NameBindingKind, PrivacyError, ToNameBinding}; +use {NameBinding, NameBindingKind, PathResult, PathScope, PrivacyError, ToNameBinding}; use ResolveResult; use ResolveResult::*; use Resolver; -use UseLexicalScopeFlag::DontUseLexicalScope; use {names_to_string, module_to_string}; use {resolve_error, ResolutionError}; @@ -27,6 +26,7 @@ use rustc::hir::def::*; use syntax::ast::{Ident, NodeId, Name}; use syntax::ext::base::Determinacy::{self, Determined, Undetermined}; use syntax::ext::hygiene::Mark; +use syntax::symbol::keywords; use syntax::util::lev_distance::find_best_match_for_name; use syntax_pos::Span; @@ -482,14 +482,13 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { // 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); + let result = self.resolve_path(&directive.module_path, PathScope::Import, None, None); directive.vis.set(vis); match result { - Success(module) => module, - Indeterminate => return Indeterminate, - Failed(err) => return Failed(err), + PathResult::Module(module) => module, + PathResult::Indeterminate => return Indeterminate, + _ => return Failed(None), } }; @@ -551,21 +550,20 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { self.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_result = self.resolve_path(&module_path, PathScope::Import, None, Some(span)); let module = match module_result { - Success(module) => module, - Indeterminate => return Indeterminate, - Failed(err) => { - let self_module = self.module_map[&self.current_module.normal_ancestor_id.unwrap()]; - - let resolve_from_self_result = self.resolve_module_path_from_root( - &self_module, &module_path, 0, Some(span)); - - return if let Success(_) = resolve_from_self_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))) } else { - Failed(err) + Failed(Some((span, msg))) }; }, }; diff --git a/src/test/compile-fail/issue-28388-1.rs b/src/test/compile-fail/issue-28388-1.rs index ef97b400b00a5..bee05cd53133a 100644 --- a/src/test/compile-fail/issue-28388-1.rs +++ b/src/test/compile-fail/issue-28388-1.rs @@ -10,6 +10,6 @@ // Prefix in imports with empty braces should be resolved and checked privacy, stability, etc. -use foo::{}; //~ ERROR failed to resolve. foo +use foo::{}; //~ ERROR failed to resolve. Maybe a missing `extern crate foo;`? fn main() {} From e9e178a5810a133d759b418f53a43cab1e716cab Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Sat, 26 Nov 2016 12:21:47 +0000 Subject: [PATCH 3/6] 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>) { From 7bccc9d7694240159587857e2b681c46a423a625 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Sat, 26 Nov 2016 12:29:21 +0000 Subject: [PATCH 4/6] Clean up formatting. --- src/librustc_resolve/lib.rs | 237 ++++++++++++++++-------------------- 1 file changed, 103 insertions(+), 134 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 18e14c2dc3986..222d0b03e67e5 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1996,77 +1996,52 @@ impl<'a> Resolver<'a> { } fn resolve_type(&mut self, ty: &Ty) { - match ty.node { - TyKind::Path(ref maybe_qself, ref path) => { - // This is a path in the type namespace. Walk through scopes - // looking for it. - if let Some(def) = self.resolve_possibly_assoc_item(ty.id, maybe_qself.as_ref(), - path, TypeNS) { - match def.base_def { - Def::Mod(..) if def.depth == 0 => { - self.session.span_err(path.span, "expected type, found module"); - self.record_def(ty.id, err_path_resolution()); - } - _ => { - // Write the result into the def map. - debug!("(resolving type) writing resolution for `{}` (id {}) = {:?}", - path_names_to_string(path, 0), ty.id, def); - self.record_def(ty.id, def); - } + if let TyKind::Path(ref maybe_qself, ref path) = ty.node { + // This is a path in the type namespace. Walk through scopes looking for it. + if let Some(def) = + self.resolve_possibly_assoc_item(ty.id, maybe_qself.as_ref(), path, TypeNS) { + match def.base_def { + Def::Mod(..) if def.depth == 0 => { + self.session.span_err(path.span, "expected type, found module"); + self.record_def(ty.id, err_path_resolution()); } - } else { - self.record_def(ty.id, err_path_resolution()); - - // Keep reporting some errors even if they're ignored above. - { - let kind = if maybe_qself.is_some() { - "associated type" - } else { - "type name" - }; - - let is_invalid_self_type_name = path.segments.len() > 0 && - maybe_qself.is_none() && - path.segments[0].identifier.name == - keywords::SelfType.name(); - if is_invalid_self_type_name { - resolve_error(self, - ty.span, - ResolutionError::SelfUsedOutsideImplOrTrait); - } else { - let segment = path.segments.last(); - let segment = segment.expect("missing name in path"); - let type_name = segment.identifier.name; - - let candidates = - self.lookup_candidates( - type_name, - TypeNS, - |def| match def { - Def::Trait(_) | - Def::Enum(_) | - Def::Struct(_) | - Def::Union(_) | - Def::TyAlias(_) => true, - _ => false, - }, - ); - - // create error object - let name = &path_names_to_string(path, 0); - let error = - ResolutionError::UseOfUndeclared( - kind, - name, - candidates, - ); + _ => { + // Write the result into the def map. + debug!("(resolving type) writing resolution for `{}` (id {}) = {:?}", + path_names_to_string(path, 0), ty.id, def); + self.record_def(ty.id, def); + } + } + } else { + self.record_def(ty.id, err_path_resolution()); + // Keep reporting some errors even if they're ignored above. + let kind = if maybe_qself.is_some() { "associated type" } else { "type name" }; + let is_invalid_self_type_name = { + path.segments.len() > 0 && + maybe_qself.is_none() && + path.segments[0].identifier.name == keywords::SelfType.name() + }; - resolve_error(self, ty.span, error); + if is_invalid_self_type_name { + resolve_error(self, ty.span, ResolutionError::SelfUsedOutsideImplOrTrait); + } else { + let type_name = path.segments.last().unwrap().identifier.name; + let candidates = self.lookup_candidates(type_name, TypeNS, |def| { + match def { + Def::Trait(_) | + Def::Enum(_) | + Def::Struct(_) | + Def::Union(_) | + Def::TyAlias(_) => true, + _ => false, } - } + }); + + let name = &path_names_to_string(path, 0); + let error = ResolutionError::UseOfUndeclared(kind, name, candidates); + resolve_error(self, ty.span, error); } } - _ => {} } // Resolve embedded types. visit::walk_ty(self, ty); @@ -2697,78 +2672,72 @@ impl<'a> Resolver<'a> { err.emit(); } else { // Keep reporting some errors even if they're ignored above. - { - let mut method_scope = false; - let mut is_static = false; - self.ribs[ValueNS].iter().rev().all(|rib| { - method_scope = match rib.kind { - MethodRibKind(is_static_) => { - is_static = is_static_; - true - } - ItemRibKind | ConstantItemRibKind => false, - _ => return true, // Keep advancing - }; - false // Stop advancing - }); + let mut method_scope = false; + let mut is_static = false; + self.ribs[ValueNS].iter().rev().all(|rib| { + method_scope = match rib.kind { + MethodRibKind(is_static_) => { + is_static = is_static_; + true + } + ItemRibKind | ConstantItemRibKind => false, + _ => return true, // Keep advancing + }; + false // Stop advancing + }); - if method_scope && keywords::SelfValue.name() == &*path_name { - resolve_error(self, - expr.span, - ResolutionError::SelfNotAvailableInStaticMethod); - } else { - let last_name = path.last().unwrap().name; - let (mut msg, is_field) = - match self.find_fallback_in_self_type(last_name) { - NoSuggestion => { - // limit search to 5 to reduce the number - // of stupid suggestions - (match self.find_best_match(&path_name) { - SuggestionType::Macro(s) => { - format!("the macro `{}`", s) - } - SuggestionType::Function(s) => format!("`{}`", s), - SuggestionType::NotFound => "".to_string(), - }, false) - } - Field => { - (if is_static && method_scope { - "".to_string() - } else { - format!("`self.{}`", path_name) - }, true) - } - TraitItem => (format!("to call `self.{}`", path_name), false), - TraitMethod(path_str) => - (format!("to call `{}::{}`", path_str, path_name), false), - }; - - let mut context = UnresolvedNameContext::Other; - let mut def = Def::Err; - if !msg.is_empty() { - msg = format!("did you mean {}?", msg); - } else { - // we display a help message if this is a module - match self.resolve_path(&path, scope, None, None) { - PathResult::Module(module) => { - def = module.def().unwrap(); - context = UnresolvedNameContext::PathIsMod(parent); - }, - _ => {}, - }; + if method_scope && keywords::SelfValue.name() == &*path_name { + let error = ResolutionError::SelfNotAvailableInStaticMethod; + resolve_error(self, expr.span, error); + } else { + let fallback = + self.find_fallback_in_self_type(path.last().unwrap().name); + let (mut msg, is_field) = match fallback { + NoSuggestion => { + // limit search to 5 to reduce the number + // of stupid suggestions + (match self.find_best_match(&path_name) { + SuggestionType::Macro(s) => { + format!("the macro `{}`", s) + } + SuggestionType::Function(s) => format!("`{}`", s), + SuggestionType::NotFound => "".to_string(), + }, false) + } + Field => { + (if is_static && method_scope { + "".to_string() + } else { + format!("`self.{}`", path_name) + }, true) } + TraitItem => (format!("to call `self.{}`", path_name), false), + TraitMethod(path_str) => + (format!("to call `{}::{}`", path_str, path_name), false), + }; - resolve_error(self, - expr.span, - ResolutionError::UnresolvedName { - path: &path_name, - message: &msg, - context: context, - is_static_method: method_scope && is_static, - is_field: is_field, - def: def, - }); + let mut context = UnresolvedNameContext::Other; + let mut def = Def::Err; + if !msg.is_empty() { + msg = format!("did you mean {}?", msg); + } else { + // we display a help message if this is a module + if let PathResult::Module(module) = + self.resolve_path(&path, scope, None, None) { + def = module.def().unwrap(); + context = UnresolvedNameContext::PathIsMod(parent); + } } + + let error = ResolutionError::UnresolvedName { + path: &path_name, + message: &msg, + context: context, + is_static_method: method_scope && is_static, + is_field: is_field, + def: def, + }; + resolve_error(self, expr.span, error); } } } From 8fe525dd1c49510246e8b68f1a33156f817f1292 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Sun, 27 Nov 2016 00:23:54 +0000 Subject: [PATCH 5/6] Simplify `binding.module()`. --- src/librustc_resolve/lib.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 222d0b03e67e5..89da3c221c674 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -908,13 +908,11 @@ struct AmbiguityError<'a> { } impl<'a> NameBinding<'a> { - fn module(&self) -> Result, bool /* true if an error has already been reported */> { + fn module(&self) -> Option> { match self.kind { - NameBindingKind::Module(module) => Ok(module), + NameBindingKind::Module(module) => Some(module), NameBindingKind::Import { binding, .. } => binding.module(), - NameBindingKind::Def(Def::Err) => Err(true), - NameBindingKind::Def(_) => Err(false), - NameBindingKind::Ambiguity { .. } => Err(false), + _ => None, } } @@ -1332,7 +1330,7 @@ impl<'a> Resolver<'a> { fn record_use(&mut self, name: Name, ns: Namespace, binding: &'a NameBinding<'a>, span: Span) -> bool /* true if an error was reported */ { // track extern crates for unused_extern_crate lint - if let Some(DefId { krate, .. }) = binding.module().ok().and_then(ModuleS::def_id) { + if let Some(DefId { krate, .. }) = binding.module().and_then(ModuleS::def_id) { self.used_crates.insert(krate); } @@ -2372,7 +2370,7 @@ impl<'a> Resolver<'a> { match binding { Ok(binding) => { - if let Ok(next_module) = binding.module() { + if let Some(next_module) = binding.module() { module = Some(next_module); } else if binding.def() == Def::Err { return PathResult::NonModule(err_path_resolution()); @@ -2980,7 +2978,7 @@ impl<'a> Resolver<'a> { } // collect submodules to explore - if let Ok(module) = name_binding.module() { + if let Some(module) = name_binding.module() { // form the path let mut path_segments = path_segments.clone(); path_segments.push(PathSegment { @@ -3141,8 +3139,8 @@ impl<'a> Resolver<'a> { (ValueNS, _) => "a value", (MacroNS, _) => "a macro", (TypeNS, _) if old_binding.is_extern_crate() => "an extern crate", - (TypeNS, Ok(module)) if module.is_normal() => "a module", - (TypeNS, Ok(module)) if module.is_trait() => "a trait", + (TypeNS, Some(module)) if module.is_normal() => "a module", + (TypeNS, Some(module)) if module.is_trait() => "a trait", (TypeNS, _) => "a type", }; format!("{} named `{}` has already been {} in this {}", From c871637e4374a7c59d39a70487a7ac46d3efa89a Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Tue, 29 Nov 2016 00:33:59 +0000 Subject: [PATCH 6/6] Remove `resolver.record_resolution()`. --- src/librustc/hir/lowering.rs | 30 +++++------------------------- src/librustc_driver/driver.rs | 3 --- src/librustc_resolve/lib.rs | 6 +----- 3 files changed, 6 insertions(+), 33 deletions(-) diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index d113f6b5e4e76..e5a61908e0f56 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -82,9 +82,6 @@ pub trait Resolver { // Obtain the resolution for a node id fn get_resolution(&mut self, id: NodeId) -> Option; - // Record the resolution of a path or binding generated by the lowerer when expanding. - fn record_resolution(&mut self, id: NodeId, def: Def); - // We must keep the set of definitions up to date as we add nodes that weren't in the AST. // This should only return `None` during testing. fn definitions(&mut self) -> &mut Definitions; @@ -351,12 +348,7 @@ impl<'a> LoweringContext<'a> { // Otherwise, the base path is an implicit `Self` type path, // e.g. `Vec` in `Vec::new` or `::Item` in // `::Item::default`. - let ty = self.ty(p.span, hir::TyPath(hir::QPath::Resolved(qself, path))); - - // Associate that innermost path type with the base Def. - self.resolver.record_resolution(ty.id, resolution.base_def); - - ty + self.ty(p.span, hir::TyPath(hir::QPath::Resolved(qself, path))) }; // Anything after the base path are associated "extensions", @@ -1902,10 +1894,8 @@ impl<'a> LoweringContext<'a> { def: def, segments: hir_vec![hir::PathSegment::from_name(id)], }))); - let expr = self.expr(span, expr_path, ThinVec::new()); - self.resolver.record_resolution(expr.id, def); - expr + self.expr(span, expr_path, ThinVec::new()) } fn expr_mut_addr_of(&mut self, span: Span, e: P) -> hir::Expr { @@ -1918,10 +1908,7 @@ impl<'a> LoweringContext<'a> { attrs: ThinVec) -> hir::Expr { let path = self.std_path(span, components, true); - let def = path.def; - let expr = self.expr(span, hir::ExprPath(hir::QPath::Resolved(None, P(path))), attrs); - self.resolver.record_resolution(expr.id, def); - expr + self.expr(span, hir::ExprPath(hir::QPath::Resolved(None, P(path))), attrs) } fn expr_match(&mut self, @@ -1948,11 +1935,8 @@ impl<'a> LoweringContext<'a> { e: Option>, attrs: ThinVec) -> hir::Expr { let path = self.std_path(span, components, false); - let def = path.def; let qpath = hir::QPath::Resolved(None, P(path)); - let expr = self.expr(span, hir::ExprStruct(qpath, fields, e), attrs); - self.resolver.record_resolution(expr.id, def); - expr + self.expr(span, hir::ExprStruct(qpath, fields, e), attrs) } fn expr(&mut self, span: Span, node: hir::Expr_, attrs: ThinVec) -> hir::Expr { @@ -2021,16 +2005,13 @@ impl<'a> LoweringContext<'a> { subpats: hir::HirVec>) -> P { let path = self.std_path(span, components, true); - let def = path.def; let qpath = hir::QPath::Resolved(None, P(path)); let pt = if subpats.is_empty() { hir::PatKind::Path(qpath) } else { hir::PatKind::TupleStruct(qpath, subpats, None) }; - let pat = self.pat(span, pt); - self.resolver.record_resolution(pat.id, def); - pat + self.pat(span, pt) } fn pat_ident(&mut self, span: Span, name: Name) -> P { @@ -2047,7 +2028,6 @@ impl<'a> LoweringContext<'a> { let def_index = defs.create_def_with_parent(parent_def, id, def_path_data); DefId::local(def_index) }; - self.resolver.record_resolution(id, Def::Local(def_id)); P(hir::Pat { id: id, diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index 7dc71f8189b86..bee79103b4185 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -10,7 +10,6 @@ use rustc::hir; use rustc::hir::{map as hir_map, FreevarMap, TraitMap}; -use rustc::hir::def::DefMap; use rustc::hir::lowering::lower_crate; use rustc_data_structures::blake2b::Blake2bHasher; use rustc_data_structures::fmt_wrap::FmtWrap; @@ -63,7 +62,6 @@ use derive_registrar; #[derive(Clone)] pub struct Resolutions { - pub def_map: DefMap, pub freevars: FreevarMap, pub trait_map: TraitMap, pub maybe_unused_trait_imports: NodeSet, @@ -794,7 +792,6 @@ pub fn phase_2_configure_and_expand(sess: &Session, hir_ty_to_ty: NodeMap(), }, resolutions: Resolutions { - def_map: resolver.def_map, freevars: resolver.freevars, trait_map: resolver.trait_map, maybe_unused_trait_imports: resolver.maybe_unused_trait_imports, diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 89da3c221c674..e1200149dcc41 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1050,7 +1050,7 @@ pub struct Resolver<'a> { // The idents for the primitive types. primitive_type_table: PrimitiveTypeTable, - pub def_map: DefMap, + def_map: DefMap, pub freevars: FreevarMap, freevars_seen: NodeMap>, pub export_map: ExportMap, @@ -1183,10 +1183,6 @@ impl<'a> hir::lowering::Resolver for Resolver<'a> { self.def_map.get(&id).cloned() } - fn record_resolution(&mut self, id: NodeId, def: Def) { - self.def_map.insert(id, PathResolution::new(def)); - } - fn definitions(&mut self) -> &mut Definitions { &mut self.definitions }