From c46164b626925083519bfd48733fad48cc1ddec5 Mon Sep 17 00:00:00 2001
From: Jeffrey Seyfried <jeffrey.seyfried@gmail.com>
Date: Tue, 26 Apr 2016 04:20:50 +0000
Subject: [PATCH 1/9] Refactor away field `vis` of `ModuleS`
---
src/librustc_resolve/build_reduced_graph.rs | 39 +++++++++++----------
src/librustc_resolve/lib.rs | 33 ++++-------------
2 files changed, 27 insertions(+), 45 deletions(-)
diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs
index ed473da19176b..6dfd3e00be864 100644
--- a/src/librustc_resolve/build_reduced_graph.rs
+++ b/src/librustc_resolve/build_reduced_graph.rs
@@ -46,9 +46,9 @@ trait ToNameBinding<'a> {
fn to_name_binding(self) -> NameBinding<'a>;
}
-impl<'a> ToNameBinding<'a> for (Module<'a>, Span) {
+impl<'a> ToNameBinding<'a> for (Module<'a>, Span, ty::Visibility) {
fn to_name_binding(self) -> NameBinding<'a> {
- NameBinding::create_from_module(self.0, Some(self.1))
+ NameBinding { kind: NameBindingKind::Module(self.0), span: Some(self.1), vis: self.2 }
}
}
@@ -247,8 +247,8 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
};
let parent_link = ModuleParentLink(parent, name);
let def = Def::Mod(def_id);
- let module = self.new_extern_crate_module(parent_link, def, vis, item.id);
- self.define(parent, name, TypeNS, (module, sp));
+ let module = self.new_extern_crate_module(parent_link, def, item.id);
+ self.define(parent, name, TypeNS, (module, sp, vis));
self.build_reduced_graph_for_external_crate(module);
}
@@ -257,8 +257,8 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
ItemMod(..) => {
let parent_link = ModuleParentLink(parent, name);
let def = Def::Mod(self.ast_map.local_def_id(item.id));
- let module = self.new_module(parent_link, Some(def), false, vis);
- self.define(parent, name, TypeNS, (module, sp));
+ let module = self.new_module(parent_link, Some(def), false);
+ self.define(parent, name, TypeNS, (module, sp, vis));
self.module_map.insert(item.id, module);
*parent_ref = module;
}
@@ -289,12 +289,12 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
ItemEnum(ref enum_definition, _) => {
let parent_link = ModuleParentLink(parent, name);
let def = Def::Enum(self.ast_map.local_def_id(item.id));
- let module = self.new_module(parent_link, Some(def), false, vis);
- self.define(parent, name, TypeNS, (module, sp));
+ let module = self.new_module(parent_link, Some(def), false);
+ self.define(parent, name, TypeNS, (module, sp, vis));
for variant in &(*enum_definition).variants {
let item_def_id = self.ast_map.local_def_id(item.id);
- self.build_reduced_graph_for_variant(variant, item_def_id, module);
+ self.build_reduced_graph_for_variant(variant, item_def_id, module, vis);
}
}
@@ -328,8 +328,8 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
// Add all the items within to a new module.
let parent_link = ModuleParentLink(parent, name);
let def = Def::Trait(def_id);
- let module_parent = self.new_module(parent_link, Some(def), false, vis);
- self.define(parent, name, TypeNS, (module_parent, sp));
+ let module_parent = self.new_module(parent_link, Some(def), false);
+ self.define(parent, name, TypeNS, (module_parent, sp, vis));
// Add the names of all the items to the trait info.
for item in items {
@@ -353,7 +353,8 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
fn build_reduced_graph_for_variant(&mut self,
variant: &Variant,
item_id: DefId,
- parent: Module<'b>) {
+ parent: Module<'b>,
+ vis: ty::Visibility) {
let name = variant.node.name;
if variant.node.data.is_struct() {
// Not adding fields for variants as they are not accessed with a self receiver
@@ -364,8 +365,8 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
// Variants are always treated as importable to allow them to be glob used.
// All variants are defined in both type and value namespaces as future-proofing.
let def = Def::Variant(item_id, self.ast_map.local_def_id(variant.node.data.id()));
- self.define(parent, name, ValueNS, (def, variant.span, parent.vis));
- self.define(parent, name, TypeNS, (def, variant.span, parent.vis));
+ self.define(parent, name, ValueNS, (def, variant.span, vis));
+ self.define(parent, name, TypeNS, (def, variant.span, vis));
}
/// Constructs the reduced graph for one foreign item.
@@ -396,7 +397,7 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
block_id);
let parent_link = BlockParentLink(parent, block_id);
- let new_module = self.new_module(parent_link, None, false, parent.vis);
+ let new_module = self.new_module(parent_link, None, false);
self.module_map.insert(block_id, new_module);
*parent = new_module;
}
@@ -425,8 +426,8 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
debug!("(building reduced graph for external crate) building module {} {:?}",
name, vis);
let parent_link = ModuleParentLink(parent, name);
- let module = self.new_module(parent_link, Some(def), true, vis);
- self.try_define(parent, name, TypeNS, (module, DUMMY_SP));
+ let module = self.new_module(parent_link, Some(def), true);
+ self.try_define(parent, name, TypeNS, (module, DUMMY_SP, vis));
}
Def::Variant(_, variant_id) => {
debug!("(building reduced graph for external crate) building variant {}", name);
@@ -467,8 +468,8 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
}
let parent_link = ModuleParentLink(parent, name);
- let module = self.new_module(parent_link, Some(def), true, vis);
- self.try_define(parent, name, TypeNS, (module, DUMMY_SP));
+ let module = self.new_module(parent_link, Some(def), true);
+ self.try_define(parent, name, TypeNS, (module, DUMMY_SP, vis));
}
Def::TyAlias(..) | Def::AssociatedTy(..) => {
debug!("(building reduced graph for external crate) building type {}", name);
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index 293b4de71fac4..16954f6bc97b4 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -818,7 +818,6 @@ enum ParentLink<'a> {
pub struct ModuleS<'a> {
parent_link: ParentLink<'a>,
def: Option<Def>,
- vis: ty::Visibility,
// If the module is an extern crate, `def` is root of the external crate and `extern_crate_id`
// is the NodeId of the local `extern crate` item (otherwise, `extern_crate_id` is None).
@@ -849,12 +848,10 @@ impl<'a> ModuleS<'a> {
fn new(parent_link: ParentLink<'a>,
def: Option<Def>,
external: bool,
- vis: ty::Visibility,
arenas: &'a ResolverArenas<'a>) -> Self {
ModuleS {
parent_link: parent_link,
def: def,
- vis: vis,
extern_crate_id: None,
resolutions: RefCell::new(HashMap::new()),
unresolved_imports: RefCell::new(Vec::new()),
@@ -895,7 +892,7 @@ impl<'a> ModuleS<'a> {
impl<'a> fmt::Debug for ModuleS<'a> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
- write!(f, "{:?}, {:?}", self.def, self.vis)
+ write!(f, "{:?}", self.def)
}
}
@@ -923,14 +920,6 @@ enum NameBindingKind<'a> {
struct PrivacyError<'a>(Span, Name, &'a NameBinding<'a>);
impl<'a> NameBinding<'a> {
- fn create_from_module(module: Module<'a>, span: Option<Span>) -> Self {
- NameBinding {
- kind: NameBindingKind::Module(module),
- span: span,
- vis: module.vis,
- }
- }
-
fn module(&self) -> Option<Module<'a>> {
match self.kind {
NameBindingKind::Module(module) => Some(module),
@@ -1148,9 +1137,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
arenas: &'a ResolverArenas<'a>)
-> Resolver<'a, 'tcx> {
let root_def_id = ast_map.local_def_id(CRATE_NODE_ID);
- let vis = ty::Visibility::Public;
let graph_root =
- ModuleS::new(NoParentLink, Some(Def::Mod(root_def_id)), false, vis, arenas);
+ ModuleS::new(NoParentLink, Some(Def::Mod(root_def_id)), false, arenas);
let graph_root = arenas.alloc_module(graph_root);
Resolver {
@@ -1208,21 +1196,14 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
}
- fn new_module(&self,
- parent_link: ParentLink<'a>,
- def: Option<Def>,
- external: bool,
- vis: ty::Visibility) -> Module<'a> {
- self.arenas.alloc_module(ModuleS::new(parent_link, def, external, vis, self.arenas))
+ fn new_module(&self, parent_link: ParentLink<'a>, def: Option<Def>, external: bool)
+ -> Module<'a> {
+ self.arenas.alloc_module(ModuleS::new(parent_link, def, external, self.arenas))
}
- fn new_extern_crate_module(&self,
- parent_link: ParentLink<'a>,
- def: Def,
- vis: ty::Visibility,
- local_node_id: NodeId)
+ fn new_extern_crate_module(&self, parent_link: ParentLink<'a>, def: Def, local_node_id: NodeId)
-> Module<'a> {
- let mut module = ModuleS::new(parent_link, Some(def), false, vis, self.arenas);
+ let mut module = ModuleS::new(parent_link, Some(def), false, self.arenas);
module.extern_crate_id = Some(local_node_id);
self.arenas.modules.alloc(module)
}
From c9d8e1493c840a9901f7caecda26de1a8a48f754 Mon Sep 17 00:00:00 2001
From: Jeffrey Seyfried <jeffrey.seyfried@gmail.com>
Date: Wed, 27 Apr 2016 01:13:15 +0000
Subject: [PATCH 2/9] Refactor field `span` of `NameBinding` from
`Option<Span>` to `Span`.
---
src/librustc_resolve/build_reduced_graph.rs | 4 ++--
src/librustc_resolve/lib.rs | 15 +++++++--------
src/librustc_resolve/resolve_imports.rs | 8 ++++----
3 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs
index 6dfd3e00be864..6a8779d79db6f 100644
--- a/src/librustc_resolve/build_reduced_graph.rs
+++ b/src/librustc_resolve/build_reduced_graph.rs
@@ -48,13 +48,13 @@ trait ToNameBinding<'a> {
impl<'a> ToNameBinding<'a> for (Module<'a>, Span, ty::Visibility) {
fn to_name_binding(self) -> NameBinding<'a> {
- NameBinding { kind: NameBindingKind::Module(self.0), span: Some(self.1), vis: self.2 }
+ NameBinding { kind: NameBindingKind::Module(self.0), span: self.1, vis: self.2 }
}
}
impl<'a> ToNameBinding<'a> for (Def, Span, ty::Visibility) {
fn to_name_binding(self) -> NameBinding<'a> {
- NameBinding { kind: NameBindingKind::Def(self.0), span: Some(self.1), vis: self.2 }
+ NameBinding { kind: NameBindingKind::Def(self.0), span: self.1, vis: self.2 }
}
}
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index 16954f6bc97b4..a5831186c8779 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -349,7 +349,7 @@ fn resolve_struct_error<'b, 'a: 'b, 'tcx: 'a>(resolver: &'b Resolver<'a, 'tcx>,
if let Some(binding) = resolver.current_module
.resolve_name_in_lexical_scope(name, ValueNS) {
if binding.is_import() {
- err.span_note(binding.span.unwrap(), "constant imported here");
+ err.span_note(binding.span, "constant imported here");
}
}
err
@@ -900,7 +900,7 @@ impl<'a> fmt::Debug for ModuleS<'a> {
#[derive(Clone, Debug)]
pub struct NameBinding<'a> {
kind: NameBindingKind<'a>,
- span: Option<Span>,
+ span: Span,
vis: ty::Visibility,
}
@@ -3293,7 +3293,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
identifier: ident,
parameters: params,
};
- let span = name_binding.span.unwrap_or(syntax::codemap::DUMMY_SP);
+ let span = name_binding.span;
let mut segms = path_segments.clone();
segms.push(segment);
let segms = HirVec::from_vec(segms);
@@ -3447,7 +3447,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
binding: &NameBinding,
old_binding: &NameBinding) {
// Error on the second of two conflicting names
- if old_binding.span.unwrap().lo > binding.span.unwrap().lo {
+ if old_binding.span.lo > binding.span.lo {
return self.report_conflict(parent, name, ns, old_binding, binding);
}
@@ -3463,7 +3463,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
false => ("defined", "definition"),
};
- let span = binding.span.unwrap();
+ let span = binding.span;
let msg = {
let kind = match (ns, old_binding.module()) {
(ValueNS, _) => "a value",
@@ -3488,9 +3488,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
},
};
- let span = old_binding.span.unwrap();
- if span != codemap::DUMMY_SP {
- err.span_note(span, &format!("previous {} of `{}` here", noun, name));
+ if old_binding.span != codemap::DUMMY_SP {
+ err.span_note(old_binding.span, &format!("previous {} of `{}` here", noun, name));
}
err.emit();
}
diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs
index f335f145a1072..4a87ffe820d4a 100644
--- a/src/librustc_resolve/resolve_imports.rs
+++ b/src/librustc_resolve/resolve_imports.rs
@@ -26,7 +26,7 @@ use rustc::hir::def::*;
use syntax::ast::{NodeId, Name};
use syntax::attr::AttrMetaMethods;
-use syntax::codemap::Span;
+use syntax::codemap::{Span, DUMMY_SP};
use syntax::util::lev_distance::find_best_match_for_name;
use std::cell::{Cell, RefCell};
@@ -76,7 +76,7 @@ impl<'a> ImportDirective<'a> {
directive: self,
privacy_error: privacy_error,
},
- span: Some(self.span),
+ span: self.span,
vis: self.vis,
}
}
@@ -412,7 +412,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
if let SingleImport { target, .. } = e.import_directive.subclass {
let dummy_binding = self.resolver.arenas.alloc_name_binding(NameBinding {
kind: NameBindingKind::Def(Def::Err),
- span: None,
+ span: DUMMY_SP,
vis: ty::Visibility::Public,
});
let dummy_binding = e.import_directive.import(dummy_binding, None);
@@ -696,7 +696,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
(error E0364), consider declaring its enum as `pub`",
name);
let lint = lint::builtin::PRIVATE_IN_PUBLIC;
- self.resolver.session.add_lint(lint, directive.id, binding.span.unwrap(), msg);
+ self.resolver.session.add_lint(lint, directive.id, binding.span, msg);
}
}
}
From 2ccaeed50efc2d55ea05ad4c1178b6ce38bf9aca Mon Sep 17 00:00:00 2001
From: Jeffrey Seyfried <jeffrey.seyfried@gmail.com>
Date: Tue, 26 Apr 2016 07:58:58 +0000
Subject: [PATCH 3/9] Refactor away `FallbackChecks` and remove dead code
---
src/librustc_resolve/lib.rs | 69 ++++---------------------------------
1 file changed, 6 insertions(+), 63 deletions(-)
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index a5831186c8779..e9bbd686a3f60 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -42,7 +42,6 @@ use self::ModulePrefixResult::*;
use self::AssocItemResolveResult::*;
use self::BareIdentifierPatternResolution::*;
use self::ParentLink::*;
-use self::FallbackChecks::*;
use rustc::dep_graph::DepNode;
use rustc::hir::map as hir_map;
@@ -81,7 +80,7 @@ use rustc::hir::{Pat, PatKind, Path, PrimTy};
use rustc::hir::{PathSegment, PathParameters};
use rustc::hir::HirVec;
use rustc::hir::{TraitRef, Ty, TyBool, TyChar, TyFloat, TyInt};
-use rustc::hir::{TyRptr, TyStr, TyUint, TyPath, TyPtr};
+use rustc::hir::{TyRptr, TyStr, TyUint, TyPath};
use std::collections::{HashMap, HashSet};
use std::cell::{Cell, RefCell};
@@ -676,9 +675,7 @@ impl<T> ResolveResult<T> {
enum FallbackSuggestion {
NoSuggestion,
Field,
- Method,
TraitItem,
- StaticMethod(String),
TraitMethod(String),
}
@@ -1124,12 +1121,6 @@ impl<'a> ResolverArenas<'a> {
}
}
-#[derive(PartialEq)]
-enum FallbackChecks {
- Everything,
- OnlyTraitAndStatics,
-}
-
impl<'a, 'tcx> Resolver<'a, 'tcx> {
fn new(session: &'a Session,
ast_map: &'a hir_map::Map<'tcx>,
@@ -2821,13 +2812,10 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
fn find_fallback_in_self_type(&mut self, name: Name) -> FallbackSuggestion {
- fn extract_path_and_node_id(t: &Ty,
- allow: FallbackChecks)
- -> Option<(Path, NodeId, FallbackChecks)> {
+ fn extract_node_id(t: &Ty) -> Option<NodeId> {
match t.node {
- TyPath(None, ref path) => Some((path.clone(), t.id, allow)),
- TyPtr(ref mut_ty) => extract_path_and_node_id(&mut_ty.ty, OnlyTraitAndStatics),
- TyRptr(_, ref mut_ty) => extract_path_and_node_id(&mut_ty.ty, allow),
+ TyPath(None, _) => Some(t.id),
+ TyRptr(_, ref mut_ty) => extract_node_id(&mut_ty.ty),
// This doesn't handle the remaining `Ty` variants as they are not
// that commonly the self_type, it might be interesting to provide
// support for those in future.
@@ -2835,23 +2823,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
}
- fn get_module<'a, 'tcx>(this: &mut Resolver<'a, 'tcx>,
- span: Span,
- name_path: &[ast::Name])
- -> Option<Module<'a>> {
- let last_name = name_path.last().unwrap();
-
- if name_path.len() == 1 {
- match this.primitive_type_table.primitive_types.get(last_name) {
- Some(_) => None,
- None => this.current_module.resolve_name_in_lexical_scope(*last_name, TypeNS)
- .and_then(NameBinding::module)
- }
- } else {
- this.resolve_module_path(&name_path, UseLexicalScope, span).success()
- }
- }
-
fn is_static_method(this: &Resolver, did: DefId) -> bool {
if let Some(node_id) = this.ast_map.as_local_node_id(did) {
let sig = match this.ast_map.get(node_id) {
@@ -2871,15 +2842,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
}
- let (path, node_id, allowed) = match self.current_self_type {
- Some(ref ty) => match extract_path_and_node_id(ty, Everything) {
- Some(x) => x,
- None => return NoSuggestion,
- },
- None => return NoSuggestion,
- };
-
- if allowed == Everything {
+ 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.
match self.def_map.borrow().get(&node_id).map(|d| d.full_def()) {
Some(Def::Enum(did)) |
@@ -2897,24 +2860,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
}
- let name_path = path.segments.iter().map(|seg| seg.identifier.name).collect::<Vec<_>>();
-
- // Look for a method in the current self type's impl module.
- if let Some(module) = get_module(self, path.span, &name_path) {
- if let Some(binding) = module.resolve_name_in_lexical_scope(name, ValueNS) {
- if let Some(Def::Method(did)) = binding.def() {
- if is_static_method(self, did) {
- return StaticMethod(path_names_to_string(&path, 0));
- }
- if self.current_trait_ref.is_some() {
- return TraitItem;
- } else if allowed == Everything {
- return Method;
- }
- }
- }
- }
-
// Look for a method in the current trait.
if let Some((trait_did, ref trait_ref)) = self.current_trait_ref {
if let Some(&did) = self.trait_item_map.get(&(name, trait_did)) {
@@ -3073,10 +3018,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
}
Field => format!("`self.{}`", path_name),
- Method |
TraitItem => format!("to call `self.{}`", path_name),
- TraitMethod(path_str) |
- StaticMethod(path_str) =>
+ TraitMethod(path_str) =>
format!("to call `{}::{}`", path_str, path_name),
};
From 82e0dd5ac1166426f4b1b91cc184ae3797842467 Mon Sep 17 00:00:00 2001
From: Jeffrey Seyfried <jeffrey.seyfried@gmail.com>
Date: Tue, 26 Apr 2016 08:29:13 +0000
Subject: [PATCH 4/9] Refactor away `is_static_method`
---
src/librustc_resolve/build_reduced_graph.rs | 10 ++++++---
src/librustc_resolve/lib.rs | 25 +++------------------
2 files changed, 10 insertions(+), 25 deletions(-)
diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs
index 6a8779d79db6f..4dc19434c8024 100644
--- a/src/librustc_resolve/build_reduced_graph.rs
+++ b/src/librustc_resolve/build_reduced_graph.rs
@@ -334,15 +334,19 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
// Add the names of all the items to the trait info.
for item in items {
let item_def_id = self.ast_map.local_def_id(item.id);
+ let mut is_static_method = false;
let (def, ns) = match item.node {
hir::ConstTraitItem(..) => (Def::AssociatedConst(item_def_id), ValueNS),
- hir::MethodTraitItem(..) => (Def::Method(item_def_id), ValueNS),
+ hir::MethodTraitItem(ref sig, _) => {
+ is_static_method = sig.explicit_self.node == hir::SelfStatic;
+ (Def::Method(item_def_id), ValueNS)
+ }
hir::TypeTraitItem(..) => (Def::AssociatedTy(def_id, item_def_id), TypeNS),
};
self.define(module_parent, item.name, ns, (def, item.span, vis));
- self.trait_item_map.insert((item.name, def_id), item_def_id);
+ self.trait_item_map.insert((item.name, def_id), is_static_method);
}
}
}
@@ -464,7 +468,7 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
'{}'",
trait_item_name);
- self.trait_item_map.insert((trait_item_name, def_id), trait_item_def.def_id());
+ self.trait_item_map.insert((trait_item_name, def_id), false);
}
let parent_link = ModuleParentLink(parent, name);
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index e9bbd686a3f60..16e97e56755e8 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -1016,7 +1016,7 @@ pub struct Resolver<'a, 'tcx: 'a> {
graph_root: Module<'a>,
- trait_item_map: FnvHashMap<(Name, DefId), DefId>,
+ trait_item_map: FnvHashMap<(Name, DefId), bool /* is static method? */>,
structs: FnvHashMap<DefId, Vec<Name>>,
@@ -2823,25 +2823,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
}
- fn is_static_method(this: &Resolver, did: DefId) -> bool {
- if let Some(node_id) = this.ast_map.as_local_node_id(did) {
- let sig = match this.ast_map.get(node_id) {
- hir_map::NodeTraitItem(trait_item) => match trait_item.node {
- hir::MethodTraitItem(ref sig, _) => sig,
- _ => return false,
- },
- hir_map::NodeImplItem(impl_item) => match impl_item.node {
- hir::ImplItemKind::Method(ref sig, _) => sig,
- _ => return false,
- },
- _ => return false,
- };
- sig.explicit_self.node == hir::SelfStatic
- } else {
- this.session.cstore.is_static_method(did)
- }
- }
-
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.
match self.def_map.borrow().get(&node_id).map(|d| d.full_def()) {
@@ -2862,8 +2843,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
// Look for a method in the current trait.
if let Some((trait_did, ref trait_ref)) = self.current_trait_ref {
- if let Some(&did) = self.trait_item_map.get(&(name, trait_did)) {
- if is_static_method(self, did) {
+ if let Some(&is_static_method) = self.trait_item_map.get(&(name, trait_did)) {
+ if is_static_method {
return TraitMethod(path_names_to_string(&trait_ref.path, 0));
} else {
return TraitItem;
From 6da115374ffd344f9bc748df36a50b55e61b4ea8 Mon Sep 17 00:00:00 2001
From: Jeffrey Seyfried <jeffrey.seyfried@gmail.com>
Date: Mon, 25 Apr 2016 05:34:59 +0000
Subject: [PATCH 5/9] Refactor away `get_trait_name`
---
src/librustc_resolve/lib.rs | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index 16e97e56755e8..d4ef3d365d19e 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -829,7 +829,7 @@ pub struct ModuleS<'a> {
globs: RefCell<Vec<&'a ImportDirective<'a>>>,
// Used to memoize the traits in this module for faster searches through all traits in scope.
- traits: RefCell<Option<Box<[&'a NameBinding<'a>]>>>,
+ traits: RefCell<Option<Box<[(Name, &'a NameBinding<'a>)]>>>,
// Whether this module is populated. If not populated, any attempt to
// access the children must be preceded with a
@@ -1234,14 +1234,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
self.glob_map.insert(directive.id, new_set);
}
- fn get_trait_name(&self, did: DefId) -> Name {
- if let Some(node_id) = self.ast_map.as_local_node_id(did) {
- self.ast_map.expect_item(node_id).name
- } else {
- self.session.cstore.item_name(did)
- }
- }
-
/// Resolves the given module path from the given root `module_`.
fn resolve_module_path_from_root(&mut self,
module_: Module<'a>,
@@ -3146,20 +3138,19 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
let mut traits = module.traits.borrow_mut();
if traits.is_none() {
let mut collected_traits = Vec::new();
- module.for_each_child(|_, ns, binding| {
+ module.for_each_child(|name, ns, binding| {
if ns != TypeNS { return }
if let Some(Def::Trait(_)) = binding.def() {
- collected_traits.push(binding);
+ collected_traits.push((name, binding));
}
});
*traits = Some(collected_traits.into_boxed_slice());
}
- for binding in traits.as_ref().unwrap().iter() {
+ for &(trait_name, binding) in traits.as_ref().unwrap().iter() {
let trait_def_id = binding.def().unwrap().def_id();
if self.trait_item_map.contains_key(&(name, trait_def_id)) {
add_trait_info(&mut found_traits, trait_def_id, name);
- let trait_name = self.get_trait_name(trait_def_id);
self.record_use(trait_name, TypeNS, binding);
}
}
From 3bcf818a8ff23711a2d0366d440a6bf27e16da03 Mon Sep 17 00:00:00 2001
From: Jeffrey Seyfried <jeffrey.seyfried@gmail.com>
Date: Wed, 27 Apr 2016 00:59:53 +0000
Subject: [PATCH 6/9] Refactor `resolve_crate_relative_path` and
`resolve_module_relative_path` to return a `NameBinding` instead of a `Def`
---
src/librustc_resolve/lib.rs | 28 ++++++++++++++++------------
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index d4ef3d365d19e..535c0cc7fe191 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -1683,8 +1683,10 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
match self.resolve_crate_relative_path(prefix.span,
&prefix.segments,
TypeNS) {
- Ok(def) =>
- self.record_def(item.id, PathResolution::new(def, 0)),
+ Ok(binding) => {
+ let def = binding.def().unwrap();
+ self.record_def(item.id, PathResolution::new(def, 0));
+ }
Err(true) => self.record_def(item.id, err_path_resolution()),
Err(false) => {
resolve_error(self,
@@ -2547,8 +2549,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
let mk_res = |def| PathResolution::new(def, path_depth);
if path.global {
- let def = self.resolve_crate_relative_path(span, segments, namespace);
- return def.map(mk_res);
+ let binding = self.resolve_crate_relative_path(span, segments, namespace);
+ return binding.map(|binding| mk_res(binding.def().unwrap()));
}
// Try to find a path to an item in a module.
@@ -2584,9 +2586,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
let unqualified_def = resolve_identifier_with_fallback(self, false);
- let def = self.resolve_module_relative_path(span, segments, namespace);
- match (def, unqualified_def) {
- (Ok(d), Some(ref ud)) if d == ud.def => {
+ let qualified_binding = self.resolve_module_relative_path(span, segments, namespace);
+ match (qualified_binding, unqualified_def) {
+ (Ok(binding), Some(ref ud)) if binding.def().unwrap() == ud.def => {
self.session
.add_lint(lint::builtin::UNUSED_QUALIFICATIONS,
id,
@@ -2596,7 +2598,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
_ => {}
}
- def.map(mk_res)
+ qualified_binding.map(|binding| mk_res(binding.def().unwrap()))
}
// Resolve a single identifier
@@ -2707,7 +2709,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
span: Span,
segments: &[hir::PathSegment],
namespace: Namespace)
- -> Result<Def, bool /* true if an error was reported */> {
+ -> Result<&'a NameBinding<'a>,
+ bool /* true if an error was reported */> {
let module_path = segments.split_last()
.unwrap()
.1
@@ -2740,7 +2743,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
let result = self.resolve_name_in_module(containing_module, name, namespace, false, true);
result.success().map(|binding| {
self.check_privacy(name, binding, span);
- binding.def().unwrap()
+ binding
}).ok_or(false)
}
@@ -2750,7 +2753,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
span: Span,
segments: &[hir::PathSegment],
namespace: Namespace)
- -> Result<Def, bool /* true if an error was reported */> {
+ -> Result<&'a NameBinding<'a>,
+ bool /* true if an error was reported */> {
let module_path = segments.split_last()
.unwrap()
.1
@@ -2790,7 +2794,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
let result = self.resolve_name_in_module(containing_module, name, namespace, false, true);
result.success().map(|binding| {
self.check_privacy(name, binding, span);
- binding.def().unwrap()
+ binding
}).ok_or(false)
}
From 33bb26998c3ef4982a4e838a626e6837f602263a Mon Sep 17 00:00:00 2001
From: Jeffrey Seyfried <jeffrey.seyfried@gmail.com>
Date: Wed, 27 Apr 2016 01:18:04 +0000
Subject: [PATCH 7/9] Refactor away a use of `ast_map.span_if_local()`
---
src/librustc_resolve/lib.rs | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index 535c0cc7fe191..c63776ad9ba88 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -1819,11 +1819,21 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
path_depth)));
// If it's a typedef, give a note
- if let Def::TyAlias(did) = path_res.base_def {
+ if let Def::TyAlias(..) = path_res.base_def {
err.fileline_note(trait_path.span,
"`type` aliases cannot be used for traits");
- if let Some(sp) = self.ast_map.span_if_local(did) {
- err.span_note(sp, "type defined here");
+
+ let definition_site = {
+ let segments = &trait_path.segments;
+ if trait_path.global {
+ self.resolve_crate_relative_path(trait_path.span, segments, TypeNS)
+ } else {
+ self.resolve_module_relative_path(trait_path.span, segments, TypeNS)
+ }.map(|binding| binding.span).unwrap_or(codemap::DUMMY_SP)
+ };
+
+ if definition_site != codemap::DUMMY_SP {
+ err.span_note(definition_site, "type defined here");
}
}
err.emit();
From 6aa91457535a1bc5433eec5f2bc5630e13b04895 Mon Sep 17 00:00:00 2001
From: Jeffrey Seyfried <jeffrey.seyfried@gmail.com>
Date: Wed, 27 Apr 2016 02:29:59 +0000
Subject: [PATCH 8/9] Avoid using the hir map when visibility checking in
`resolve`
Refactor `ty::Visibility` methods to use a new trait `NodeIdTree` instead of the ast map.
---
src/librustc/ty/mod.rs | 30 ++++++++++++++++---------
src/librustc_resolve/lib.rs | 21 +++++++++++++++--
src/librustc_resolve/resolve_imports.rs | 9 ++++----
3 files changed, 44 insertions(+), 16 deletions(-)
diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs
index 238856197feed..0c23f5332982d 100644
--- a/src/librustc/ty/mod.rs
+++ b/src/librustc/ty/mod.rs
@@ -283,6 +283,22 @@ pub enum Visibility {
PrivateExternal,
}
+pub trait NodeIdTree {
+ fn is_descendant_of(&self, node: NodeId, ancestor: NodeId) -> bool;
+}
+
+impl<'a> NodeIdTree for ast_map::Map<'a> {
+ fn is_descendant_of(&self, node: NodeId, ancestor: NodeId) -> bool {
+ let mut node_ancestor = node;
+ loop {
+ if node_ancestor == ancestor { return true }
+ let node_ancestor_parent = self.get_module_parent(node_ancestor);
+ if node_ancestor_parent == node_ancestor { return false }
+ node_ancestor = node_ancestor_parent;
+ }
+ }
+}
+
impl Visibility {
pub fn from_hir(visibility: &hir::Visibility, id: NodeId, tcx: &TyCtxt) -> Self {
match *visibility {
@@ -301,7 +317,7 @@ impl Visibility {
}
/// Returns true if an item with this visibility is accessible from the given block.
- pub fn is_accessible_from(self, block: NodeId, map: &ast_map::Map) -> bool {
+ pub fn is_accessible_from<T: NodeIdTree>(self, block: NodeId, tree: &T) -> bool {
let restriction = match self {
// Public items are visible everywhere.
Visibility::Public => return true,
@@ -311,24 +327,18 @@ impl Visibility {
Visibility::Restricted(module) => module,
};
- let mut block_ancestor = block;
- loop {
- if block_ancestor == restriction { return true }
- let block_ancestor_parent = map.get_module_parent(block_ancestor);
- if block_ancestor_parent == block_ancestor { return false }
- block_ancestor = block_ancestor_parent;
- }
+ tree.is_descendant_of(block, restriction)
}
/// Returns true if this visibility is at least as accessible as the given visibility
- pub fn is_at_least(self, vis: Visibility, map: &ast_map::Map) -> bool {
+ pub fn is_at_least<T: NodeIdTree>(self, vis: Visibility, tree: &T) -> bool {
let vis_restriction = match vis {
Visibility::Public => return self == Visibility::Public,
Visibility::PrivateExternal => return true,
Visibility::Restricted(module) => module,
};
- self.is_accessible_from(vis_restriction, map)
+ self.is_accessible_from(vis_restriction, tree)
}
}
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index c63776ad9ba88..fdb834a32fbc6 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -1121,6 +1121,21 @@ impl<'a> ResolverArenas<'a> {
}
}
+impl<'a, 'tcx> ty::NodeIdTree for Resolver<'a, 'tcx> {
+ fn is_descendant_of(&self, node: NodeId, ancestor: NodeId) -> bool {
+ let ancestor = self.ast_map.local_def_id(ancestor);
+ let mut module = *self.module_map.get(&node).unwrap();
+ loop {
+ if module.def_id() == Some(ancestor) { return true; }
+ let module_parent = match self.get_nearest_normal_module_parent(module) {
+ Some(parent) => parent,
+ None => return false,
+ };
+ module = module_parent;
+ }
+ }
+}
+
impl<'a, 'tcx> Resolver<'a, 'tcx> {
fn new(session: &'a Session,
ast_map: &'a hir_map::Map<'tcx>,
@@ -1131,6 +1146,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
let graph_root =
ModuleS::new(NoParentLink, Some(Def::Mod(root_def_id)), false, arenas);
let graph_root = arenas.alloc_module(graph_root);
+ let mut module_map = NodeMap();
+ module_map.insert(CRATE_NODE_ID, graph_root);
Resolver {
session: session,
@@ -1161,7 +1178,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
freevars_seen: NodeMap(),
export_map: NodeMap(),
trait_map: NodeMap(),
- module_map: NodeMap(),
+ module_map: module_map,
used_imports: HashSet::new(),
used_crates: HashSet::new(),
@@ -3343,7 +3360,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
fn is_accessible(&self, vis: ty::Visibility) -> bool {
let current_module = self.get_nearest_normal_module_parent_or_self(self.current_module);
let node_id = self.ast_map.as_local_node_id(current_module.def_id().unwrap()).unwrap();
- vis.is_accessible_from(node_id, &self.ast_map)
+ vis.is_accessible_from(node_id, self)
}
fn check_privacy(&mut self, name: Name, binding: &'a NameBinding<'a>, span: Span) {
diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs
index 4a87ffe820d4a..f0639a8517686 100644
--- a/src/librustc_resolve/resolve_imports.rs
+++ b/src/librustc_resolve/resolve_imports.rs
@@ -552,9 +552,9 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
_ => (),
}
- let ast_map = self.resolver.ast_map;
match (&value_result, &type_result) {
- (&Success(binding), _) if !binding.pseudo_vis().is_at_least(directive.vis, ast_map) &&
+ (&Success(binding), _) if !binding.pseudo_vis()
+ .is_at_least(directive.vis, self.resolver) &&
self.resolver.is_accessible(binding.vis) => {
let msg = format!("`{}` is private, and cannot be reexported", source);
let note_msg = format!("consider marking `{}` as `pub` in the imported module",
@@ -564,7 +564,8 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
.emit();
}
- (_, &Success(binding)) if !binding.pseudo_vis().is_at_least(directive.vis, ast_map) &&
+ (_, &Success(binding)) if !binding.pseudo_vis()
+ .is_at_least(directive.vis, self.resolver) &&
self.resolver.is_accessible(binding.vis) => {
if binding.is_extern_crate() {
let msg = format!("extern crate `{}` is private, and cannot be reexported \
@@ -691,7 +692,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
if let NameBindingKind::Import { binding: orig_binding, directive, .. } = binding.kind {
if ns == TypeNS && orig_binding.is_variant() &&
- !orig_binding.vis.is_at_least(binding.vis, &self.resolver.ast_map) {
+ !orig_binding.vis.is_at_least(binding.vis, self.resolver) {
let msg = format!("variant `{}` is private, and cannot be reexported \
(error E0364), consider declaring its enum as `pub`",
name);
From ac264196e2f4c1f3de8b081f72dc78838e423085 Mon Sep 17 00:00:00 2001
From: Jeffrey Seyfried <jeffrey.seyfried@gmail.com>
Date: Wed, 27 Apr 2016 19:30:16 +0000
Subject: [PATCH 9/9] Address style nits
---
src/librustc/ty/mod.rs | 8 +++++---
src/librustc_resolve/lib.rs | 4 ++--
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs
index 0c23f5332982d..bc342b235dd42 100644
--- a/src/librustc/ty/mod.rs
+++ b/src/librustc/ty/mod.rs
@@ -290,12 +290,14 @@ pub trait NodeIdTree {
impl<'a> NodeIdTree for ast_map::Map<'a> {
fn is_descendant_of(&self, node: NodeId, ancestor: NodeId) -> bool {
let mut node_ancestor = node;
- loop {
- if node_ancestor == ancestor { return true }
+ while node_ancestor != ancestor {
let node_ancestor_parent = self.get_module_parent(node_ancestor);
- if node_ancestor_parent == node_ancestor { return false }
+ if node_ancestor_parent == node_ancestor {
+ return false;
+ }
node_ancestor = node_ancestor_parent;
}
+ true
}
}
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index fdb834a32fbc6..1674a96c8badc 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -1125,14 +1125,14 @@ impl<'a, 'tcx> ty::NodeIdTree for Resolver<'a, 'tcx> {
fn is_descendant_of(&self, node: NodeId, ancestor: NodeId) -> bool {
let ancestor = self.ast_map.local_def_id(ancestor);
let mut module = *self.module_map.get(&node).unwrap();
- loop {
- if module.def_id() == Some(ancestor) { return true; }
+ while module.def_id() != Some(ancestor) {
let module_parent = match self.get_nearest_normal_module_parent(module) {
Some(parent) => parent,
None => return false,
};
module = module_parent;
}
+ true
}
}