From 917a0fbc1b73fe7dd5c65fd4b38759366e96c251 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 4 May 2019 15:18:58 +0300 Subject: [PATCH 1/3] Rename `PathResolution` to `PartialRes` Don't use `PartialRes` when `Res` is enough --- src/librustc/hir/def.rs | 29 ++--- src/librustc/hir/lowering.rs | 39 +++--- src/librustc/hir/mod.rs | 2 +- src/librustc_resolve/lib.rs | 156 +++++++++++------------- src/librustc_resolve/resolve_imports.rs | 7 +- 5 files changed, 103 insertions(+), 130 deletions(-) diff --git a/src/librustc/hir/def.rs b/src/librustc/hir/def.rs index 6366c1f93e671..6ff0c0fbb5008 100644 --- a/src/librustc/hir/def.rs +++ b/src/librustc/hir/def.rs @@ -1,5 +1,5 @@ use crate::hir::def_id::DefId; -use crate::util::nodemap::{NodeMap, DefIdMap}; +use crate::util::nodemap::DefIdMap; use syntax::ast; use syntax::ext::base::MacroKind; use syntax::ast::NodeId; @@ -151,7 +151,9 @@ pub enum Res { Err, } -/// The result of resolving a path before lowering to HIR. +/// The result of resolving a path before lowering to HIR, +/// with "module" segments resolved and associated item +/// segments deferred to type checking. /// `base_res` is the resolution of the resolved part of the /// path, `unresolved_segments` is the number of unresolved /// segments. @@ -166,19 +168,21 @@ pub enum Res { /// base_res unresolved_segments = 2 /// ``` #[derive(Copy, Clone, Debug)] -pub struct PathResolution { +pub struct PartialRes { base_res: Res, unresolved_segments: usize, } -impl PathResolution { - pub fn new(res: Res) -> Self { - PathResolution { base_res: res, unresolved_segments: 0 } +impl PartialRes { + #[inline] + pub fn new(base_res: Res) -> Self { + PartialRes { base_res, unresolved_segments: 0 } } - pub fn with_unresolved_segments(res: Res, mut unresolved_segments: usize) -> Self { - if res == Res::Err { unresolved_segments = 0 } - PathResolution { base_res: res, unresolved_segments: unresolved_segments } + #[inline] + pub fn with_unresolved_segments(base_res: Res, mut unresolved_segments: usize) -> Self { + if base_res == Res::Err { unresolved_segments = 0 } + PartialRes { base_res, unresolved_segments } } #[inline] @@ -269,17 +273,10 @@ impl PerNS> { } } -/// Definition mapping -pub type ResMap = NodeMap; - /// This is the replacement export map. It maps a module to all of the exports /// within. pub type ExportMap = DefIdMap>>; -/// Map used to track the `use` statements within a scope, matching it with all the items in every -/// namespace. -pub type ImportMap = NodeMap>>; - #[derive(Copy, Clone, Debug, RustcEncodable, RustcDecodable, HashStable)] pub struct Export { /// The name of the target. diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 8361a62c07e48..9ae9995be43ba 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -37,7 +37,7 @@ use crate::hir::{self, ParamName}; use crate::hir::HirVec; use crate::hir::map::{DefKey, DefPathData, Definitions}; use crate::hir::def_id::{DefId, DefIndex, DefIndexAddressSpace, CRATE_DEF_INDEX}; -use crate::hir::def::{Res, DefKind, PathResolution, PerNS}; +use crate::hir::def::{Res, DefKind, PartialRes, PerNS}; use crate::hir::{GenericArg, ConstArg}; use crate::lint::builtin::{self, PARENTHESIZED_PARAMS_IN_TYPES_AND_MODULES, ELIDED_LIFETIMES_IN_PATHS}; @@ -145,11 +145,11 @@ pub trait Resolver { is_value: bool, ) -> hir::Path; - /// Obtain the resolution for a `NodeId`. - fn get_resolution(&mut self, id: NodeId) -> Option; + /// Obtain resolution for a `NodeId` with a single resolution. + fn get_partial_res(&mut self, id: NodeId) -> Option; - /// Obtain the possible resolutions for the given `use` statement. - fn get_import(&mut self, id: NodeId) -> PerNS>; + /// Obtain per-namespace resolutions for `use` statement with the given `NoedId`. + fn get_import_res(&mut self, id: NodeId) -> PerNS>>; /// 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. @@ -821,7 +821,7 @@ impl<'a> LoweringContext<'a> { } fn expect_full_res(&mut self, id: NodeId) -> Res { - self.resolver.get_resolution(id).map_or(Res::Err, |pr| { + self.resolver.get_partial_res(id).map_or(Res::Err, |pr| { if pr.unresolved_segments() != 0 { bug!("path not fully resolved: {:?}", pr); } @@ -830,12 +830,7 @@ impl<'a> LoweringContext<'a> { } fn expect_full_res_from_use(&mut self, id: NodeId) -> impl Iterator> { - self.resolver.get_import(id).present_items().map(|pr| { - if pr.unresolved_segments() != 0 { - bug!("path not fully resolved: {:?}", pr); - } - pr.base_res() - }) + self.resolver.get_import_res(id).present_items() } fn diagnostic(&self) -> &errors::Handler { @@ -1842,13 +1837,13 @@ impl<'a> LoweringContext<'a> { let qself_position = qself.as_ref().map(|q| q.position); let qself = qself.as_ref().map(|q| self.lower_ty(&q.ty, itctx.reborrow())); - let resolution = self.resolver - .get_resolution(id) - .unwrap_or_else(|| PathResolution::new(Res::Err)); + let partial_res = self.resolver + .get_partial_res(id) + .unwrap_or_else(|| PartialRes::new(Res::Err)); - let proj_start = p.segments.len() - resolution.unresolved_segments(); + let proj_start = p.segments.len() - partial_res.unresolved_segments(); let path = P(hir::Path { - res: self.lower_res(resolution.base_res()), + res: self.lower_res(partial_res.base_res()), segments: p.segments[..proj_start] .iter() .enumerate() @@ -1869,7 +1864,7 @@ impl<'a> LoweringContext<'a> { krate: def_id.krate, index: this.def_key(def_id).parent.expect("missing parent"), }; - let type_def_id = match resolution.base_res() { + let type_def_id = match partial_res.base_res() { Res::Def(DefKind::AssociatedTy, def_id) if i + 2 == proj_start => { Some(parent_def_id(self, def_id)) } @@ -1886,7 +1881,7 @@ impl<'a> LoweringContext<'a> { } _ => None, }; - let parenthesized_generic_args = match resolution.base_res() { + let parenthesized_generic_args = match partial_res.base_res() { // `a::b::Trait(Args)` Res::Def(DefKind::Trait, _) if i + 1 == proj_start => ParenthesizedGenericArgs::Ok, @@ -1940,7 +1935,7 @@ impl<'a> LoweringContext<'a> { // Simple case, either no projections, or only fully-qualified. // E.g., `std::mem::size_of` or `::Item`. - if resolution.unresolved_segments() == 0 { + if partial_res.unresolved_segments() == 0 { return hir::QPath::Resolved(qself, path); } @@ -2792,7 +2787,7 @@ impl<'a> LoweringContext<'a> { && bound_pred.bound_generic_params.is_empty() => { if let Some(Res::Def(DefKind::TyParam, def_id)) = self.resolver - .get_resolution(bound_pred.bounded_ty.id) + .get_partial_res(bound_pred.bounded_ty.id) .map(|d| d.base_res()) { if let Some(node_id) = @@ -3946,7 +3941,7 @@ impl<'a> LoweringContext<'a> { let node = match p.node { PatKind::Wild => hir::PatKind::Wild, PatKind::Ident(ref binding_mode, ident, ref sub) => { - match self.resolver.get_resolution(p.id).map(|d| d.base_res()) { + match self.resolver.get_partial_res(p.id).map(|d| d.base_res()) { // `None` can occur in body-less function signatures res @ None | res @ Some(Res::Local(_)) => { let canonical_id = match res { diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index ae7358df9d8fa..7399245b0a00f 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -2142,7 +2142,7 @@ pub enum UseKind { /// resolve maps each TraitRef's ref_id to its defining trait; that's all /// that the ref_id is for. Note that ref_id's value is not the NodeId of the /// trait being referred to but just a unique NodeId that serves as a key -/// within the ResMap. +/// within the resolution map. #[derive(Clone, RustcEncodable, RustcDecodable, Debug, HashStable)] pub struct TraitRef { pub path: Path, diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index e0892f98d3147..131f26663c70b 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -24,7 +24,7 @@ use rustc::middle::cstore::CrateStore; use rustc::session::Session; use rustc::lint; use rustc::hir::def::{ - self, DefKind, PathResolution, CtorKind, CtorOf, NonMacroAttrKind, ResMap, ImportMap, ExportMap + self, DefKind, PartialRes, CtorKind, CtorOf, NonMacroAttrKind, ExportMap }; use rustc::hir::def::Namespace::*; use rustc::hir::def_id::{CRATE_DEF_INDEX, LOCAL_CRATE, DefId}; @@ -821,7 +821,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Resolver<'a> { let self_ty = keywords::SelfUpper.ident(); let res = self.resolve_ident_in_lexical_scope(self_ty, TypeNS, Some(ty.id), ty.span) .map_or(Res::Err, |d| d.res()); - self.record_res(ty.id, PathResolution::new(res)); + self.record_partial_res(ty.id, PartialRes::new(res)); } _ => (), } @@ -1146,7 +1146,7 @@ impl ModuleOrUniformRoot<'_> { #[derive(Clone, Debug)] enum PathResult<'a> { Module(ModuleOrUniformRoot<'a>), - NonModule(PathResolution), + NonModule(PartialRes), Indeterminate, Failed { span: Span, @@ -1659,8 +1659,11 @@ pub struct Resolver<'a> { /// The idents for the primitive types. primitive_type_table: PrimitiveTypeTable, - res_map: ResMap, - import_map: ImportMap, + /// Resolutions for nodes that have a single resolution. + partial_res_map: NodeMap, + /// Resolutions for import nodes, which have multiple resolutions in different namespaces. + import_res_map: NodeMap>>, + pub freevars: FreevarMap, freevars_seen: NodeMap>, pub export_map: ExportMap, @@ -1830,12 +1833,12 @@ impl<'a> hir::lowering::Resolver for Resolver<'a> { self.resolve_hir_path(&path, is_value) } - fn get_resolution(&mut self, id: NodeId) -> Option { - self.res_map.get(&id).cloned() + fn get_partial_res(&mut self, id: NodeId) -> Option { + self.partial_res_map.get(&id).cloned() } - fn get_import(&mut self, id: NodeId) -> PerNS> { - self.import_map.get(&id).cloned().unwrap_or_default() + fn get_import_res(&mut self, id: NodeId) -> PerNS> { + self.import_res_map.get(&id).cloned().unwrap_or_default() } fn definitions(&mut self) -> &mut Definitions { @@ -1919,7 +1922,7 @@ impl<'a> Resolver<'a> { let segments: Vec<_> = segments.iter().map(|seg| { let mut hir_seg = hir::PathSegment::from_ident(seg.ident); - hir_seg.res = Some(self.res_map.get(&seg.id).map_or(def::Res::Err, |p| { + hir_seg.res = Some(self.partial_res_map.get(&seg.id).map_or(def::Res::Err, |p| { p.base_res().map_id(|_| panic!("unexpected node_id")) })); hir_seg @@ -2019,8 +2022,8 @@ impl<'a> Resolver<'a> { primitive_type_table: PrimitiveTypeTable::new(), - res_map: Default::default(), - import_map: Default::default(), + partial_res_map: Default::default(), + import_res_map: Default::default(), freevars: Default::default(), freevars_seen: Default::default(), export_map: FxHashMap::default(), @@ -2705,7 +2708,7 @@ impl<'a> Resolver<'a> { self.definitions.local_def_id(param.id), ); function_type_rib.bindings.insert(ident, res); - self.record_res(param.id, PathResolution::new(res)); + self.record_partial_res(param.id, PartialRes::new(res)); } GenericParamKind::Const { .. } => { let ident = param.ident.modern(); @@ -2726,7 +2729,7 @@ impl<'a> Resolver<'a> { self.definitions.local_def_id(param.id), ); function_value_rib.bindings.insert(ident, res); - self.record_res(param.id, PathResolution::new(res)); + self.record_partial_res(param.id, PartialRes::new(res)); } } } @@ -2994,7 +2997,8 @@ impl<'a> Resolver<'a> { pat.walk(&mut |pat| { if let PatKind::Ident(binding_mode, ident, ref sub_pat) = pat.node { - if sub_pat.is_some() || match self.res_map.get(&pat.id).map(|res| res.base_res()) { + if sub_pat.is_some() || match self.partial_res_map.get(&pat.id) + .map(|res| res.base_res()) { Some(Res::Local(..)) => true, _ => false, } { @@ -3146,7 +3150,7 @@ impl<'a> Resolver<'a> { outer_pat_id: NodeId, pat_src: PatternSource, bindings: &mut FxHashMap) - -> PathResolution { + -> Res { // Add the binding to the local ribs, if it // doesn't already exist in the bindings map. (We // must not add it if it's in the bindings map @@ -3193,7 +3197,7 @@ impl<'a> Resolver<'a> { } } - PathResolution::new(res) + res } fn resolve_pattern(&mut self, @@ -3213,7 +3217,7 @@ impl<'a> Resolver<'a> { let binding = self.resolve_ident_in_lexical_scope(ident, ValueNS, None, pat.span) .and_then(LexicalScopeBinding::item); - let resolution = binding.map(NameBinding::res).and_then(|res| { + let res = binding.map(NameBinding::res).and_then(|res| { let is_syntactic_ambiguity = opt_pat.is_none() && bmode == BindingMode::ByValue(Mutability::Immutable); match res { @@ -3222,7 +3226,7 @@ impl<'a> Resolver<'a> { // Disambiguate in favor of a unit struct/variant // or constant pattern. self.record_use(ident, ValueNS, binding.unwrap(), false); - Some(PathResolution::new(res)) + Some(res) } Res::Def(DefKind::Ctor(..), _) | Res::Def(DefKind::Const, _) @@ -3254,7 +3258,7 @@ impl<'a> Resolver<'a> { self.fresh_binding(ident, pat.id, outer_pat_id, pat_src, bindings) }); - self.record_res(pat.id, resolution); + self.record_partial_res(pat.id, PartialRes::new(res)); } PatKind::TupleStruct(ref path, ..) => { @@ -3286,35 +3290,15 @@ impl<'a> Resolver<'a> { id: NodeId, qself: Option<&QSelf>, path: &Path, - source: PathSource<'_>) - -> PathResolution { - self.smart_resolve_path_with_crate_lint(id, qself, path, source, CrateLint::SimplePath(id)) - } - - /// A variant of `smart_resolve_path` where you also specify extra - /// information about where the path came from; this extra info is - /// sometimes needed for the lint that recommends rewriting - /// absolute paths to `crate`, so that it knows how to frame the - /// suggestion. If you are just resolving a path like `foo::bar` - /// that appears in an arbitrary location, then you just want - /// `CrateLint::SimplePath`, which is what `smart_resolve_path` - /// already provides. - fn smart_resolve_path_with_crate_lint( - &mut self, - id: NodeId, - qself: Option<&QSelf>, - path: &Path, - source: PathSource<'_>, - crate_lint: CrateLint - ) -> PathResolution { + source: PathSource<'_>) { self.smart_resolve_path_fragment( id, qself, &Segment::from_path(path), path.span, source, - crate_lint, - ) + CrateLint::SimplePath(id), + ); } fn smart_resolve_path_fragment(&mut self, @@ -3324,7 +3308,7 @@ impl<'a> Resolver<'a> { span: Span, source: PathSource<'_>, crate_lint: CrateLint) - -> PathResolution { + -> PartialRes { let ns = source.namespace(); let is_expected = &|res| source.is_expected(res); @@ -3334,10 +3318,10 @@ impl<'a> Resolver<'a> { let node_id = this.definitions.as_local_node_id(def_id).unwrap(); let better = res.is_some(); this.use_injections.push(UseError { err, candidates, node_id, better }); - err_path_resolution() + PartialRes::new(Res::Err) }; - let resolution = match self.resolve_qpath_anywhere( + let partial_res = match self.resolve_qpath_anywhere( id, qself, path, @@ -3347,14 +3331,14 @@ impl<'a> Resolver<'a> { source.global_by_default(), crate_lint, ) { - Some(resolution) if resolution.unresolved_segments() == 0 => { - if is_expected(resolution.base_res()) || resolution.base_res() == Res::Err { - resolution + Some(partial_res) if partial_res.unresolved_segments() == 0 => { + if is_expected(partial_res.base_res()) || partial_res.base_res() == Res::Err { + partial_res } else { // Add a temporary hack to smooth the transition to new struct ctor // visibility rules. See #38932 for more details. let mut res = None; - if let Res::Def(DefKind::Struct, def_id) = resolution.base_res() { + if let Res::Def(DefKind::Struct, def_id) = partial_res.base_res() { if let Some((ctor_res, ctor_vis)) = self.struct_constructors.get(&def_id).cloned() { if is_expected(ctor_res) && self.is_accessible(ctor_vis) { @@ -3363,15 +3347,15 @@ impl<'a> Resolver<'a> { "private struct constructors are not usable through \ re-exports in outer modules", ); - res = Some(PathResolution::new(ctor_res)); + res = Some(PartialRes::new(ctor_res)); } } } - res.unwrap_or_else(|| report_errors(self, Some(resolution.base_res()))) + res.unwrap_or_else(|| report_errors(self, Some(partial_res.base_res()))) } } - Some(resolution) if source.defer_to_typeck() => { + Some(partial_res) if source.defer_to_typeck() => { // Not fully resolved associated item `T::A::B` or `::A::B` // or `::A::B`. If `B` should be resolved in value namespace then // it needs to be added to the trait map. @@ -3399,16 +3383,16 @@ impl<'a> Resolver<'a> { hm.insert(span, span); } } - resolution + partial_res } _ => report_errors(self, None) }; if let PathSource::TraitItem(..) = source {} else { // Avoid recording definition of `A::B` in `::B::C`. - self.record_res(id, resolution); + self.record_partial_res(id, partial_res); } - resolution + partial_res } /// Only used in a specific case of type ascription suggestions @@ -3523,7 +3507,7 @@ impl<'a> Resolver<'a> { defer_to_typeck: bool, global_by_default: bool, crate_lint: CrateLint, - ) -> Option { + ) -> Option { let mut fin_res = None; // FIXME: can't resolve paths in macro namespace yet, macros are // processed by the little special hack below. @@ -3532,9 +3516,10 @@ impl<'a> Resolver<'a> { match self.resolve_qpath(id, qself, path, ns, span, global_by_default, crate_lint) { // If defer_to_typeck, then resolution > no resolution, // otherwise full resolution > partial resolution > no resolution. - Some(res) if res.unresolved_segments() == 0 || defer_to_typeck => - return Some(res), - res => if fin_res.is_none() { fin_res = res }, + Some(partial_res) if partial_res.unresolved_segments() == 0 || + defer_to_typeck => + return Some(partial_res), + partial_res => if fin_res.is_none() { fin_res = partial_res }, }; } } @@ -3545,7 +3530,7 @@ impl<'a> Resolver<'a> { self.macro_use_prelude.get(&path[0].ident.name).cloned() .and_then(NameBinding::macro_kind) == Some(MacroKind::Bang)) { // Return some dummy definition, it's enough for error reporting. - return Some(PathResolution::new(Res::Def( + return Some(PartialRes::new(Res::Def( DefKind::Macro(MacroKind::Bang), DefId::local(CRATE_DEF_INDEX), ))); @@ -3563,7 +3548,7 @@ impl<'a> Resolver<'a> { span: Span, global_by_default: bool, crate_lint: CrateLint, - ) -> Option { + ) -> Option { debug!( "resolve_qpath(id={:?}, qself={:?}, path={:?}, \ ns={:?}, span={:?}, global_by_default={:?})", @@ -3580,7 +3565,7 @@ impl<'a> Resolver<'a> { // This is a case like `::B`, where there is no // trait to resolve. In that case, we leave the `B` // segment to be resolved by type-check. - return Some(PathResolution::with_unresolved_segments( + return Some(PartialRes::with_unresolved_segments( Res::Def(DefKind::Mod, DefId::local(CRATE_DEF_INDEX)), path.len() )); } @@ -3600,7 +3585,7 @@ impl<'a> Resolver<'a> { // name from a fully qualified path, and this also // contains the full span (the `CrateLint::QPathTrait`). let ns = if qself.position + 1 == path.len() { ns } else { TypeNS }; - let res = self.smart_resolve_path_fragment( + let partial_res = self.smart_resolve_path_fragment( id, None, &path[..=qself.position], @@ -3615,8 +3600,9 @@ impl<'a> Resolver<'a> { // The remaining segments (the `C` in our example) will // have to be resolved by type-check, since that requires doing // trait resolution. - return Some(PathResolution::with_unresolved_segments( - res.base_res(), res.unresolved_segments() + path.len() - qself.position - 1 + return Some(PartialRes::with_unresolved_segments( + partial_res.base_res(), + partial_res.unresolved_segments() + path.len() - qself.position - 1, )); } @@ -3629,7 +3615,7 @@ impl<'a> Resolver<'a> { ) { PathResult::NonModule(path_res) => path_res, PathResult::Module(ModuleOrUniformRoot::Module(module)) if !module.is_normal() => { - PathResolution::new(module.res().unwrap()) + PartialRes::new(module.res().unwrap()) } // 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. @@ -3649,13 +3635,13 @@ impl<'a> Resolver<'a> { self.primitive_type_table.primitive_types .contains_key(&path[0].ident.name) => { let prim = self.primitive_type_table.primitive_types[&path[0].ident.name]; - PathResolution::with_unresolved_segments(Res::PrimTy(prim), path.len() - 1) + PartialRes::with_unresolved_segments(Res::PrimTy(prim), path.len() - 1) } PathResult::Module(ModuleOrUniformRoot::Module(module)) => - PathResolution::new(module.res().unwrap()), + PartialRes::new(module.res().unwrap()), PathResult::Failed { is_error_from_last_segment: false, span, label, suggestion } => { resolve_error(self, span, ResolutionError::FailedToResolve { label, suggestion }); - err_path_resolution() + PartialRes::new(Res::Err) } PathResult::Module(..) | PathResult::Failed { .. } => return None, PathResult::Indeterminate => bug!("indetermined path result in resolve_qpath"), @@ -3731,9 +3717,9 @@ impl<'a> Resolver<'a> { let record_segment_res = |this: &mut Self, res| { if record_used { if let Some(id) = id { - if !this.res_map.contains_key(&id) { + if !this.partial_res_map.contains_key(&id) { assert!(id != ast::DUMMY_NODE_ID, "Trying to resolve dummy id"); - this.record_res(id, PathResolution::new(res)); + this.record_partial_res(id, PartialRes::new(res)); } } } @@ -3837,7 +3823,7 @@ impl<'a> Resolver<'a> { Some(LexicalScopeBinding::Res(res)) if opt_ns == Some(TypeNS) || opt_ns == Some(ValueNS) => { record_segment_res(self, res); - return PathResult::NonModule(PathResolution::with_unresolved_segments( + return PathResult::NonModule(PartialRes::with_unresolved_segments( res, path.len() - 1 )); } @@ -3864,9 +3850,9 @@ impl<'a> Resolver<'a> { ).emit(); } let res = Res::NonMacroAttr(NonMacroAttrKind::Tool); - return PathResult::NonModule(PathResolution::new(res)); + return PathResult::NonModule(PartialRes::new(res)); } else if res == Res::Err { - return PathResult::NonModule(err_path_resolution()); + return PathResult::NonModule(PartialRes::new(Res::Err)); } else if opt_ns.is_some() && (is_last || maybe_assoc) { self.lint_if_path_starts_with_module( crate_lint, @@ -3874,7 +3860,7 @@ impl<'a> Resolver<'a> { path_span, second_binding, ); - return PathResult::NonModule(PathResolution::with_unresolved_segments( + return PathResult::NonModule(PartialRes::with_unresolved_segments( res, path.len() - i - 1 )); } else { @@ -3897,7 +3883,7 @@ impl<'a> Resolver<'a> { Err(Determined) => { if let Some(ModuleOrUniformRoot::Module(module)) = module { if opt_ns.is_some() && !module.is_normal() { - return PathResult::NonModule(PathResolution::with_unresolved_segments( + return PathResult::NonModule(PartialRes::with_unresolved_segments( module.res().unwrap(), path.len() - i )); } @@ -3928,7 +3914,7 @@ impl<'a> Resolver<'a> { (format!("maybe a missing `extern crate {};`?", ident), None) } else { // the parser will already have complained about the keyword being used - return PathResult::NonModule(err_path_resolution()); + return PathResult::NonModule(PartialRes::new(Res::Err)); } } else if i == 0 { (format!("use of undeclared type or module `{}`", ident), None) @@ -4177,7 +4163,7 @@ impl<'a> Resolver<'a> { if filter_fn(Res::Local(ast::DUMMY_NODE_ID)) { if let Some(node_id) = self.current_self_type.as_ref().and_then(extract_node_id) { // Look for a field with the same name in the current self_type. - if let Some(resolution) = self.res_map.get(&node_id) { + if let Some(resolution) = self.partial_res_map.get(&node_id) { match resolution.base_res() { Res::Def(DefKind::Struct, did) | Res::Def(DefKind::Union, did) if resolution.unresolved_segments() == 0 => { @@ -4398,7 +4384,7 @@ impl<'a> Resolver<'a> { }); find_best_match_for_name(names, &*ident.as_str(), None) }); - self.record_res(expr.id, err_path_resolution()); + self.record_partial_res(expr.id, PartialRes::new(Res::Err)); resolve_error(self, label.ident.span, ResolutionError::UndeclaredLabel(&label.ident.as_str(), @@ -4406,7 +4392,7 @@ impl<'a> Resolver<'a> { } Some(Res::Label(id)) => { // Since this res is a label, it is never read. - self.record_res(expr.id, PathResolution::new(Res::Label(id))); + self.record_partial_res(expr.id, PartialRes::new(Res::Label(id))); self.unused_labels.remove(&id); } Some(_) => { @@ -4858,9 +4844,9 @@ impl<'a> Resolver<'a> { }) } - fn record_res(&mut self, node_id: NodeId, resolution: PathResolution) { + fn record_partial_res(&mut self, node_id: NodeId, resolution: PartialRes) { debug!("(recording res) recording {:?} for {}", resolution, node_id); - if let Some(prev_res) = self.res_map.insert(node_id, resolution) { + if let Some(prev_res) = self.partial_res_map.insert(node_id, resolution) { panic!("path resolved multiple times ({:?} before, {:?} now)", prev_res, resolution); } } @@ -5483,10 +5469,6 @@ fn module_to_string(module: Module<'_>) -> Option { .collect::>())) } -fn err_path_resolution() -> PathResolution { - PathResolution::new(Res::Err) -} - #[derive(Copy, Clone, Debug)] enum CrateLint { /// Do not issue the lint. diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 522a49ee2c1ea..9e6b8d035458d 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -21,7 +21,7 @@ use rustc::lint::builtin::{ UNUSED_IMPORTS, }; use rustc::hir::def_id::{CrateNum, DefId}; -use rustc::hir::def::{self, DefKind, PathResolution, Export}; +use rustc::hir::def::{self, DefKind, PartialRes, Export}; use rustc::session::DiagnosticMessageId; use rustc::util::nodemap::FxHashSet; use rustc::{bug, span_bug}; @@ -1233,8 +1233,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { res = Res::Err; } } - let import = this.import_map.entry(directive.id).or_default(); - import[ns] = Some(PathResolution::new(res)); + this.import_res_map.entry(directive.id).or_default()[ns] = Some(res); }); self.check_for_redundant_imports( @@ -1371,7 +1370,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { } // Record the destination of this import - self.record_res(directive.id, PathResolution::new(module.res().unwrap())); + self.record_partial_res(directive.id, PartialRes::new(module.res().unwrap())); } // Miscellaneous post-processing, including recording re-exports, From 85ddd1dc587615a0dc76cef80ff7d88a1b4bc28e Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 4 May 2019 15:22:00 +0300 Subject: [PATCH 2/3] Rename `Res::kind_name` to `Res::descr` for consistency --- src/librustc/hir/def.rs | 2 +- src/librustc_mir/hair/pattern/check_match.rs | 2 +- src/librustc_resolve/diagnostics.rs | 2 +- src/librustc_resolve/lib.rs | 8 ++++---- src/librustc_resolve/macros.rs | 4 ++-- src/librustc_typeck/check/_match.rs | 4 ++-- src/librustc_typeck/check/mod.rs | 2 +- src/librustdoc/passes/collect_intra_doc_links.rs | 10 +++++----- 8 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/librustc/hir/def.rs b/src/librustc/hir/def.rs index 6ff0c0fbb5008..87af450451e7e 100644 --- a/src/librustc/hir/def.rs +++ b/src/librustc/hir/def.rs @@ -370,7 +370,7 @@ impl Res { } /// A human readable name for the res kind ("function", "module", etc.). - pub fn kind_name(&self) -> &'static str { + pub fn descr(&self) -> &'static str { match *self { Res::Def(kind, _) => kind.descr(), Res::SelfCtor(..) => "self constructor", diff --git a/src/librustc_mir/hair/pattern/check_match.rs b/src/librustc_mir/hair/pattern/check_match.rs index b08499b981cd7..ed183acc93b74 100644 --- a/src/librustc_mir/hair/pattern/check_match.rs +++ b/src/librustc_mir/hair/pattern/check_match.rs @@ -286,7 +286,7 @@ impl<'a, 'tcx> MatchVisitor<'a, 'tcx> { PatKind::Path(hir::QPath::Resolved(None, ref path)) if path.segments.len() == 1 && path.segments[0].args.is_none() => { format!("interpreted as {} {} pattern, not new variable", - path.res.article(), path.res.kind_name()) + path.res.article(), path.res.descr()) } _ => format!("pattern `{}` not covered", pattern_string), }; diff --git a/src/librustc_resolve/diagnostics.rs b/src/librustc_resolve/diagnostics.rs index bbfc39fc6eaf0..509aa95bb61df 100644 --- a/src/librustc_resolve/diagnostics.rs +++ b/src/librustc_resolve/diagnostics.rs @@ -41,7 +41,7 @@ impl<'a> Resolver<'a> { let item_str = path.last().unwrap().ident; let code = source.error_code(res.is_some()); let (base_msg, fallback_label, base_span) = if let Some(res) = res { - (format!("expected {}, found {} `{}`", expected, res.kind_name(), path_str), + (format!("expected {}, found {} `{}`", expected, res.descr(), path_str), format!("not a {}", expected), span) } else { diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 131f26663c70b..88465535c2cb6 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1532,7 +1532,7 @@ impl<'a> NameBinding<'a> { } fn descr(&self) -> &'static str { - if self.is_extern_crate() { "extern crate" } else { self.res().kind_name() } + if self.is_extern_crate() { "extern crate" } else { self.res().descr() } } fn article(&self) -> &'static str { @@ -3868,7 +3868,7 @@ impl<'a> Resolver<'a> { "`{}` is {} {}, not a module", ident, res.article(), - res.kind_name(), + res.descr(), ); return PathResult::Failed { @@ -4220,7 +4220,7 @@ impl<'a> Resolver<'a> { names.push(TypoSuggestion { candidate: ident.name, article: binding.res().article(), - kind: binding.res().kind_name(), + kind: binding.res().descr(), }); } } @@ -4238,7 +4238,7 @@ impl<'a> Resolver<'a> { names.push(TypoSuggestion { candidate: ident.name, article: res.article(), - kind: res.kind_name(), + kind: res.descr(), }); } } diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index 18573a4594f4c..f1706a4616b06 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -333,7 +333,7 @@ impl<'a> Resolver<'a> { // Not only attributes, but anything in macro namespace can result in // `Res::NonMacroAttr` definition (e.g., `inline!()`), so we must report // an error for those cases. - let msg = format!("expected a macro, found {}", res.kind_name()); + let msg = format!("expected a macro, found {}", res.descr()); self.session.span_err(path.span, &msg); return Err(Determinacy::Determined); } @@ -913,7 +913,7 @@ impl<'a> Resolver<'a> { // (which is a best effort error recovery tool, basically), so we can't // promise their resolution won't change later. let msg = format!("inconsistent resolution for a macro: first {}, then {}", - initial_res.kind_name(), res.kind_name()); + initial_res.descr(), res.descr()); this.session.span_err(span, &msg); } else { span_bug!(span, "inconsistent resolution for a macro"); diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index 62afbc44d07b6..e9e202ce37996 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -884,7 +884,7 @@ https://doc.rust-lang.org/reference/types.html#trait-objects"); }; let report_unexpected_res = |res: Res| { let msg = format!("expected tuple struct/variant, found {} `{}`", - res.kind_name(), + res.descr(), hir::print::to_string(tcx.hir(), |s| s.print_qpath(qpath, false))); struct_span_err!(tcx.sess, pat.span, E0164, "{}", msg) .span_label(pat.span, "not a tuple variant or struct").emit(); @@ -947,7 +947,7 @@ https://doc.rust-lang.org/reference/types.html#trait-objects"); let fields_ending = if variant.fields.len() == 1 { "" } else { "s" }; struct_span_err!(tcx.sess, pat.span, E0023, "this pattern has {} field{}, but the corresponding {} has {} field{}", - subpats.len(), subpats_ending, res.kind_name(), + subpats.len(), subpats_ending, res.descr(), variant.fields.len(), fields_ending) .span_label(pat.span, format!("expected {} field{}, found {}", variant.fields.len(), fields_ending, subpats.len())) diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 4ae75511322b6..e404a8e6972c8 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1902,7 +1902,7 @@ fn report_unexpected_variant_res<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>, qpath: &QPath) { span_err!(tcx.sess, span, E0533, "expected unit struct/variant or constant, found {} `{}`", - res.kind_name(), + res.descr(), hir::print::to_string(tcx.hir(), |s| s.print_qpath(qpath, false))); } diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 705b222efe805..abf19a0a5efa4 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -513,18 +513,18 @@ fn ambiguity_error( msg += &format!( "both {} {} and {} {}", first_def.article(), - first_def.kind_name(), + first_def.descr(), second_def.article(), - second_def.kind_name(), + second_def.descr(), ); } _ => { let mut candidates = candidates.iter().peekable(); while let Some((res, _)) = candidates.next() { if candidates.peek().is_some() { - msg += &format!("{} {}, ", res.article(), res.kind_name()); + msg += &format!("{} {}, ", res.article(), res.descr()); } else { - msg += &format!("and {} {}", res.article(), res.kind_name()); + msg += &format!("and {} {}", res.article(), res.descr()); } } } @@ -575,7 +575,7 @@ fn ambiguity_error( diag.span_suggestion( sp, - &format!("to link to the {}, {}", res.kind_name(), action), + &format!("to link to the {}, {}", res.descr(), action), suggestion, Applicability::MaybeIncorrect, ); From 7da9250fb51e71ecd3dbb4c760c24e836ed3530f Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 4 May 2019 17:22:00 +0300 Subject: [PATCH 3/3] Remove `Res::Label` Paths can never resolve to labels --- src/librustc/hir/def.rs | 4 ---- src/librustc/hir/lowering.rs | 5 ++++- src/librustc_resolve/lib.rs | 35 +++++++++++++++++-------------- src/librustc_save_analysis/lib.rs | 1 - src/librustc_save_analysis/sig.rs | 2 +- 5 files changed, 24 insertions(+), 23 deletions(-) diff --git a/src/librustc/hir/def.rs b/src/librustc/hir/def.rs index 87af450451e7e..0719eb701a984 100644 --- a/src/librustc/hir/def.rs +++ b/src/librustc/hir/def.rs @@ -142,7 +142,6 @@ pub enum Res { Upvar(Id, // `HirId` of closed over local usize, // index in the `freevars` list of the closure ast::NodeId), // expr node that creates the closure - Label(ast::NodeId), // Macro namespace NonMacroAttr(NonMacroAttrKind), // e.g., `#[inline]` or `#[rustfmt::skip]` @@ -349,7 +348,6 @@ impl Res { Res::Local(..) | Res::Upvar(..) | - Res::Label(..) | Res::PrimTy(..) | Res::SelfTy(..) | Res::SelfCtor(..) | @@ -377,7 +375,6 @@ impl Res { Res::PrimTy(..) => "builtin type", Res::Local(..) => "local variable", Res::Upvar(..) => "closure capture", - Res::Label(..) => "label", Res::SelfTy(..) => "self type", Res::ToolMod => "tool module", Res::NonMacroAttr(attr_kind) => attr_kind.descr(), @@ -405,7 +402,6 @@ impl Res { index, closure ), - Res::Label(id) => Res::Label(id), Res::SelfTy(a, b) => Res::SelfTy(a, b), Res::ToolMod => Res::ToolMod, Res::NonMacroAttr(attr_kind) => Res::NonMacroAttr(attr_kind), diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 9ae9995be43ba..fc3987a4b1918 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -151,6 +151,9 @@ pub trait Resolver { /// Obtain per-namespace resolutions for `use` statement with the given `NoedId`. fn get_import_res(&mut self, id: NodeId) -> PerNS>>; + /// Obtain resolution for a label with the given `NodeId`. + fn get_label_res(&mut self, id: NodeId) -> Option; + /// 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; @@ -1246,7 +1249,7 @@ impl<'a> LoweringContext<'a> { fn lower_loop_destination(&mut self, destination: Option<(NodeId, Label)>) -> hir::Destination { let target_id = match destination { Some((id, _)) => { - if let Res::Label(loop_id) = self.expect_full_res(id) { + if let Some(loop_id) = self.resolver.get_label_res(id) { Ok(self.lower_node_id(loop_id)) } else { Err(hir::LoopIdError::UnresolvedLabel) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 88465535c2cb6..0be26451ae4ae 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1071,13 +1071,13 @@ enum RibKind<'a> { /// The resolution keeps a separate stack of ribs as it traverses the AST for each namespace. When /// resolving, the name is looked up from inside out. #[derive(Debug)] -struct Rib<'a> { - bindings: FxHashMap, +struct Rib<'a, R = Res> { + bindings: FxHashMap, kind: RibKind<'a>, } -impl<'a> Rib<'a> { - fn new(kind: RibKind<'a>) -> Rib<'a> { +impl<'a, R> Rib<'a, R> { + fn new(kind: RibKind<'a>) -> Rib<'a, R> { Rib { bindings: Default::default(), kind, @@ -1638,7 +1638,7 @@ pub struct Resolver<'a> { ribs: PerNS>>, /// The current set of local scopes, for labels. - label_ribs: Vec>, + label_ribs: Vec>, /// The trait that the current context can refer to. current_trait_ref: Option<(Module<'a>, TraitRef)>, @@ -1663,6 +1663,8 @@ pub struct Resolver<'a> { partial_res_map: NodeMap, /// Resolutions for import nodes, which have multiple resolutions in different namespaces. import_res_map: NodeMap>>, + /// Resolutions for labels (node IDs of their corresponding blocks or loops). + label_res_map: NodeMap, pub freevars: FreevarMap, freevars_seen: NodeMap>, @@ -1841,6 +1843,10 @@ impl<'a> hir::lowering::Resolver for Resolver<'a> { self.import_res_map.get(&id).cloned().unwrap_or_default() } + fn get_label_res(&mut self, id: NodeId) -> Option { + self.label_res_map.get(&id).cloned() + } + fn definitions(&mut self) -> &mut Definitions { &mut self.definitions } @@ -2024,6 +2030,7 @@ impl<'a> Resolver<'a> { partial_res_map: Default::default(), import_res_map: Default::default(), + label_res_map: Default::default(), freevars: Default::default(), freevars_seen: Default::default(), export_map: FxHashMap::default(), @@ -2490,7 +2497,7 @@ impl<'a> Resolver<'a> { /// /// Stops after meeting a closure. fn search_label(&self, mut ident: Ident, pred: P) -> Option - where P: Fn(&Rib<'_>, Ident) -> Option + where P: Fn(&Rib<'_, NodeId>, Ident) -> Option { for rib in self.label_ribs.iter().rev() { match rib.kind { @@ -4332,10 +4339,9 @@ impl<'a> Resolver<'a> { { if let Some(label) = label { self.unused_labels.insert(id, label.ident.span); - let res = Res::Label(id); self.with_label_rib(|this| { let ident = label.ident.modern_and_legacy(); - this.label_ribs.last_mut().unwrap().bindings.insert(ident, res); + this.label_ribs.last_mut().unwrap().bindings.insert(ident, id); f(this); }); } else { @@ -4366,10 +4372,10 @@ impl<'a> Resolver<'a> { } ExprKind::Break(Some(label), _) | ExprKind::Continue(Some(label)) => { - let res = self.search_label(label.ident, |rib, ident| { + let node_id = self.search_label(label.ident, |rib, ident| { rib.bindings.get(&ident.modern_and_legacy()).cloned() }); - match res { + match node_id { None => { // Search again for close matches... // Picks the first label that is "close enough", which is not necessarily @@ -4390,13 +4396,10 @@ impl<'a> Resolver<'a> { ResolutionError::UndeclaredLabel(&label.ident.as_str(), close_match)); } - Some(Res::Label(id)) => { + Some(node_id) => { // Since this res is a label, it is never read. - self.record_partial_res(expr.id, PartialRes::new(Res::Label(id))); - self.unused_labels.remove(&id); - } - Some(_) => { - span_bug!(expr.span, "label wasn't mapped to a label res!"); + self.label_res_map.insert(expr.id, node_id); + self.unused_labels.remove(&node_id); } } diff --git a/src/librustc_save_analysis/lib.rs b/src/librustc_save_analysis/lib.rs index c242b4d6a4123..f3e0fb32ec2dc 100644 --- a/src/librustc_save_analysis/lib.rs +++ b/src/librustc_save_analysis/lib.rs @@ -796,7 +796,6 @@ impl<'l, 'tcx: 'l> SaveContext<'l, 'tcx> { } Res::PrimTy(..) | Res::SelfTy(..) | - Res::Label(..) | Res::Def(HirDefKind::Macro(..), _) | Res::ToolMod | Res::NonMacroAttr(..) | diff --git a/src/librustc_save_analysis/sig.rs b/src/librustc_save_analysis/sig.rs index 4f759b8a73fcc..fa12d9c49dfc3 100644 --- a/src/librustc_save_analysis/sig.rs +++ b/src/librustc_save_analysis/sig.rs @@ -579,7 +579,7 @@ impl Sig for ast::Path { let res = scx.get_path_res(id.ok_or("Missing id for Path")?); let (name, start, end) = match res { - Res::Label(..) | Res::PrimTy(..) | Res::SelfTy(..) | Res::Err => { + Res::PrimTy(..) | Res::SelfTy(..) | Res::Err => { return Ok(Signature { text: pprust::path_to_string(self), defs: vec![],