From 7bc6c75d0f6ec012e8baa1f59f80d2c53faefd3b Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Tue, 1 Mar 2016 02:29:06 +0000 Subject: [PATCH 01/15] Refactor away handle_external_def --- src/librustc_resolve/build_reduced_graph.rs | 103 +++++++------------- 1 file changed, 36 insertions(+), 67 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 479fc5ebf90..44eba1a14d6 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -24,7 +24,7 @@ use Resolver; use {resolve_error, resolve_struct_error, ResolutionError}; -use rustc::middle::cstore::{CrateStore, ChildItem, DlDef, DlField, DlImpl}; +use rustc::middle::cstore::{CrateStore, ChildItem, DlDef}; use rustc::middle::def::*; use rustc::middle::def_id::{CRATE_DEF_INDEX, DefId}; use rustc::middle::ty::VariantKind; @@ -42,7 +42,6 @@ use rustc_front::hir::{ItemStruct, ItemTrait, ItemTy, ItemUse}; use rustc_front::hir::{PathListIdent, PathListMod, StmtDecl}; use rustc_front::hir::{Variant, ViewPathGlob, ViewPathList, ViewPathSimple}; -use rustc_front::hir::Visibility; use rustc_front::intravisit::{self, Visitor}; use std::mem::replace; @@ -439,42 +438,48 @@ fn build_reduced_graph_for_block(&mut self, block: &Block, parent: Module<'b>) - } } - fn handle_external_def(&mut self, - def: Def, - vis: Visibility, - final_ident: &str, - name: Name, - new_parent: Module<'b>) { - debug!("(building reduced graph for external crate) building external def {}, priv {:?}", - final_ident, - vis); - let is_public = vis == hir::Public || new_parent.is_trait(); + /// Builds the reduced graph for a single item in an external crate. + fn build_reduced_graph_for_external_crate_def(&mut self, parent: Module<'b>, xcdef: ChildItem) { + let def = match xcdef.def { + DlDef(def) => def, + _ => return, + }; + + if let Def::ForeignMod(def_id) = def { + // Foreign modules have no names. Recur and populate eagerly. + for child in self.session.cstore.item_children(def_id) { + self.build_reduced_graph_for_external_crate_def(parent, child); + } + return; + } + + let name = xcdef.name; + let is_public = xcdef.vis == hir::Public || parent.is_trait(); let mut modifiers = DefModifiers::empty(); if is_public { modifiers = modifiers | DefModifiers::PUBLIC; } - if new_parent.is_normal() { + if parent.is_normal() { modifiers = modifiers | DefModifiers::IMPORTABLE; } match def { Def::Mod(_) | Def::ForeignMod(_) | Def::Enum(..) => { debug!("(building reduced graph for external crate) building module {} {}", - final_ident, + name, is_public); - let parent_link = ModuleParentLink(new_parent, name); + let parent_link = ModuleParentLink(parent, name); let module = self.new_module(parent_link, Some(def), true, is_public); - self.try_define(new_parent, name, TypeNS, (module, DUMMY_SP)); + self.try_define(parent, name, TypeNS, (module, DUMMY_SP)); } Def::Variant(_, variant_id) => { - debug!("(building reduced graph for external crate) building variant {}", - final_ident); + debug!("(building reduced graph for external crate) building variant {}", name); // 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 modifiers = DefModifiers::PUBLIC | DefModifiers::IMPORTABLE; - self.try_define(new_parent, name, TypeNS, (def, DUMMY_SP, modifiers)); - self.try_define(new_parent, name, ValueNS, (def, DUMMY_SP, modifiers)); + self.try_define(parent, name, TypeNS, (def, DUMMY_SP, modifiers)); + self.try_define(parent, name, ValueNS, (def, DUMMY_SP, modifiers)); if self.session.cstore.variant_kind(variant_id) == Some(VariantKind::Struct) { // Not adding fields for variants as they are not accessed with a self receiver self.structs.insert(variant_id, Vec::new()); @@ -486,12 +491,11 @@ fn handle_external_def(&mut self, Def::AssociatedConst(..) | Def::Method(..) => { debug!("(building reduced graph for external crate) building value (fn/static) {}", - final_ident); - self.try_define(new_parent, name, ValueNS, (def, DUMMY_SP, modifiers)); + name); + self.try_define(parent, name, ValueNS, (def, DUMMY_SP, modifiers)); } Def::Trait(def_id) => { - debug!("(building reduced graph for external crate) building type {}", - final_ident); + debug!("(building reduced graph for external crate) building type {}", name); // If this is a trait, add all the trait item names to the trait // info. @@ -508,24 +512,22 @@ fn handle_external_def(&mut self, self.trait_item_map.insert((trait_item_name, def_id), trait_item_def.def_id()); } - let parent_link = ModuleParentLink(new_parent, name); + let parent_link = ModuleParentLink(parent, name); let module = self.new_module(parent_link, Some(def), true, is_public); - self.try_define(new_parent, name, TypeNS, (module, DUMMY_SP)); + self.try_define(parent, name, TypeNS, (module, DUMMY_SP)); } Def::TyAlias(..) | Def::AssociatedTy(..) => { - debug!("(building reduced graph for external crate) building type {}", - final_ident); - self.try_define(new_parent, name, TypeNS, (def, DUMMY_SP, modifiers)); + debug!("(building reduced graph for external crate) building type {}", name); + self.try_define(parent, name, TypeNS, (def, DUMMY_SP, modifiers)); } Def::Struct(def_id) if self.session.cstore.tuple_struct_definition_if_ctor(def_id).is_none() => { - debug!("(building reduced graph for external crate) building type and value for \ - {}", - final_ident); - self.try_define(new_parent, name, TypeNS, (def, DUMMY_SP, modifiers)); + debug!("(building reduced graph for external crate) building type and value for {}", + name); + self.try_define(parent, name, TypeNS, (def, DUMMY_SP, modifiers)); if let Some(ctor_def_id) = self.session.cstore.struct_ctor_def_id(def_id) { let def = Def::Struct(ctor_def_id); - self.try_define(new_parent, name, ValueNS, (def, DUMMY_SP, modifiers)); + self.try_define(parent, name, ValueNS, (def, DUMMY_SP, modifiers)); } // Record the def ID and fields of this struct. @@ -545,39 +547,6 @@ fn handle_external_def(&mut self, } } - /// Builds the reduced graph for a single item in an external crate. - fn build_reduced_graph_for_external_crate_def(&mut self, - root: Module<'b>, - xcdef: ChildItem) { - match xcdef.def { - DlDef(def) => { - // Add the new child item, if necessary. - match def { - Def::ForeignMod(def_id) => { - // Foreign modules have no names. Recur and populate - // eagerly. - for child in self.session.cstore.item_children(def_id) { - self.build_reduced_graph_for_external_crate_def(root, child) - } - } - _ => { - self.handle_external_def(def, - xcdef.vis, - &xcdef.name.as_str(), - xcdef.name, - root); - } - } - } - DlImpl(_) => { - debug!("(building reduced graph for external crate) ignoring impl"); - } - DlField => { - debug!("(building reduced graph for external crate) ignoring field"); - } - } - } - /// Builds the reduced graph rooted at the given external module. fn populate_external_module(&mut self, module: Module<'b>) { debug!("(populating external module) attempting to populate {}", From ff014a3a10c3ae2bcf52744736d7c0e3f095612a Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Tue, 1 Mar 2016 02:36:31 +0000 Subject: [PATCH 02/15] Refactor away populate_external_module --- src/librustc_resolve/build_reduced_graph.rs | 29 +++------------------ 1 file changed, 4 insertions(+), 25 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 44eba1a14d6..9e955a7f379 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -19,7 +19,6 @@ use Module; use Namespace::{self, TypeNS, ValueNS}; use {NameBinding, NameBindingKind}; -use module_to_string; use ParentLink::{ModuleParentLink, BlockParentLink}; use Resolver; use {resolve_error, resolve_struct_error, ResolutionError}; @@ -547,34 +546,14 @@ fn build_reduced_graph_for_external_crate_def(&mut self, parent: Module<'b>, xcd } } - /// Builds the reduced graph rooted at the given external module. - fn populate_external_module(&mut self, module: Module<'b>) { - debug!("(populating external module) attempting to populate {}", - module_to_string(module)); - - let def_id = match module.def_id() { - None => { - debug!("(populating external module) ... no def ID!"); - return; - } - Some(def_id) => def_id, - }; - - for child in self.session.cstore.item_children(def_id) { - debug!("(populating external module) ... found ident: {}", - child.name); - self.build_reduced_graph_for_external_crate_def(module, child); - } - module.populated.set(true) - } - /// Ensures that the reduced graph rooted at the given external module /// is built, building it if it is not. fn populate_module_if_necessary(&mut self, module: Module<'b>) { - if !module.populated.get() { - self.populate_external_module(module) + if module.populated.get() { return } + for child in self.session.cstore.item_children(module.def_id().unwrap()) { + self.build_reduced_graph_for_external_crate_def(module, child); } - assert!(module.populated.get()) + module.populated.set(true) } /// Builds the reduced graph rooted at the 'use' directive for an external From 77f0f4a624fbff057514d6ee1bc20cf49145aa50 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Wed, 2 Mar 2016 10:21:09 +0000 Subject: [PATCH 03/15] Avoid repeating parent --- src/librustc_resolve/build_reduced_graph.rs | 36 +++++++-------------- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 9e955a7f379..4551363136d 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -43,7 +43,6 @@ use rustc_front::hir::{Variant, ViewPathGlob, ViewPathList, ViewPathSimple}; use rustc_front::intravisit::{self, Visitor}; -use std::mem::replace; use std::ops::{Deref, DerefMut}; struct GraphBuilder<'a, 'b: 'a, 'tcx: 'b> { @@ -122,7 +121,8 @@ fn is_item(statement: &hir::Stmt) -> bool { } /// Constructs the reduced graph for one item. - fn build_reduced_graph_for_item(&mut self, item: &Item, parent: Module<'b>) -> Module<'b> { + fn build_reduced_graph_for_item(&mut self, item: &Item, parent_ref: &mut Module<'b>) { + let parent = *parent_ref; let name = item.name; let sp = item.span; let is_public = item.vis == hir::Public; @@ -242,7 +242,6 @@ fn build_reduced_graph_for_item(&mut self, item: &Item, parent: Module<'b>) -> M is_prelude); } } - parent } ItemExternCrate(_) => { @@ -260,7 +259,6 @@ fn build_reduced_graph_for_item(&mut self, item: &Item, parent: Module<'b>) -> M self.build_reduced_graph_for_external_crate(module); } - parent } ItemMod(..) => { @@ -269,34 +267,30 @@ fn build_reduced_graph_for_item(&mut self, item: &Item, parent: Module<'b>) -> M let module = self.new_module(parent_link, Some(def), false, is_public); self.define(parent, name, TypeNS, (module, sp)); parent.module_children.borrow_mut().insert(item.id, module); - module + *parent_ref = module; } - ItemForeignMod(..) => parent, + ItemForeignMod(..) => {} // These items live in the value namespace. ItemStatic(_, m, _) => { let mutbl = m == hir::MutMutable; let def = Def::Static(self.ast_map.local_def_id(item.id), mutbl); self.define(parent, name, ValueNS, (def, sp, modifiers)); - parent } ItemConst(_, _) => { let def = Def::Const(self.ast_map.local_def_id(item.id)); self.define(parent, name, ValueNS, (def, sp, modifiers)); - parent } ItemFn(_, _, _, _, _, _) => { let def = Def::Fn(self.ast_map.local_def_id(item.id)); self.define(parent, name, ValueNS, (def, sp, modifiers)); - parent } // These items live in the type namespace. ItemTy(..) => { let def = Def::TyAlias(self.ast_map.local_def_id(item.id)); self.define(parent, name, TypeNS, (def, sp, modifiers)); - parent } ItemEnum(ref enum_definition, _) => { @@ -315,7 +309,6 @@ fn build_reduced_graph_for_item(&mut self, item: &Item, parent: Module<'b>) -> M self.build_reduced_graph_for_variant(variant, item_def_id, module, variant_modifiers); } - parent } // These items live in both the type and value namespaces. @@ -338,12 +331,9 @@ fn build_reduced_graph_for_item(&mut self, item: &Item, parent: Module<'b>) -> M .collect(); let item_def_id = self.ast_map.local_def_id(item.id); self.structs.insert(item_def_id, field_names); - - parent } - ItemDefaultImpl(_, _) | - ItemImpl(..) => parent, + ItemDefaultImpl(_, _) | ItemImpl(..) => {} ItemTrait(_, _, _, ref items) => { let def_id = self.ast_map.local_def_id(item.id); @@ -368,8 +358,6 @@ fn build_reduced_graph_for_item(&mut self, item: &Item, parent: Module<'b>) -> M self.trait_item_map.insert((item.name, def_id), item_def_id); } - - parent } } } @@ -420,7 +408,7 @@ fn build_reduced_graph_for_foreign_item(&mut self, self.define(parent, name, ValueNS, (def, foreign_item.span, modifiers)); } - fn build_reduced_graph_for_block(&mut self, block: &Block, parent: Module<'b>) -> Module<'b> { + fn build_reduced_graph_for_block(&mut self, block: &Block, parent: &mut Module<'b>) { if self.block_needs_anonymous_module(block) { let block_id = block.id; @@ -431,9 +419,7 @@ fn build_reduced_graph_for_block(&mut self, block: &Block, parent: Module<'b>) - let parent_link = BlockParentLink(parent, block_id); let new_module = self.new_module(parent_link, None, false, false); parent.module_children.borrow_mut().insert(block_id, new_module); - new_module - } else { - parent + *parent = new_module; } } @@ -610,8 +596,8 @@ fn visit_nested_item(&mut self, item: hir::ItemId) { } fn visit_item(&mut self, item: &Item) { - let p = self.builder.build_reduced_graph_for_item(item, &self.parent); - let old_parent = replace(&mut self.parent, p); + let old_parent = self.parent; + self.builder.build_reduced_graph_for_item(item, &mut self.parent); intravisit::walk_item(self, item); self.parent = old_parent; } @@ -621,8 +607,8 @@ fn visit_foreign_item(&mut self, foreign_item: &ForeignItem) { } fn visit_block(&mut self, block: &Block) { - let np = self.builder.build_reduced_graph_for_block(block, &self.parent); - let old_parent = replace(&mut self.parent, np); + let old_parent = self.parent; + self.builder.build_reduced_graph_for_block(block, &mut self.parent); intravisit::walk_block(self, block); self.parent = old_parent; } From 0bed9aea2db3ab8338d8fe1ff7583395ceeb1c87 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Wed, 2 Mar 2016 09:18:47 +0000 Subject: [PATCH 04/15] Make populate_module_if_necessary a method of resolver --- src/librustc_resolve/build_reduced_graph.rs | 28 ++++++++++----------- src/librustc_resolve/lib.rs | 4 +-- src/librustc_resolve/resolve_imports.rs | 4 +-- 3 files changed, 16 insertions(+), 20 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 4551363136d..8d9ce36a148 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -532,16 +532,6 @@ fn build_reduced_graph_for_external_crate_def(&mut self, parent: Module<'b>, xcd } } - /// Ensures that the reduced graph rooted at the given external module - /// is built, building it if it is not. - fn populate_module_if_necessary(&mut self, module: Module<'b>) { - if module.populated.get() { return } - for child in self.session.cstore.item_children(module.def_id().unwrap()) { - self.build_reduced_graph_for_external_crate_def(module, child); - } - module.populated.set(true) - } - /// Builds the reduced graph rooted at the 'use' directive for an external /// crate. fn build_reduced_graph_for_external_crate(&mut self, root: Module<'b>) { @@ -585,6 +575,19 @@ fn build_import_directive(&mut self, } } +impl<'a, 'tcx> Resolver<'a, 'tcx> { + /// Ensures that the reduced graph rooted at the given external module + /// is built, building it if it is not. + pub fn populate_module_if_necessary(&mut self, module: Module<'a>) { + if module.populated.get() { return } + let mut builder = GraphBuilder { resolver: self }; + for child in self.session.cstore.item_children(module.def_id().unwrap()) { + builder.build_reduced_graph_for_external_crate_def(module, child); + } + module.populated.set(true) + } +} + struct BuildReducedGraphVisitor<'a, 'b: 'a, 'tcx: 'b> { builder: GraphBuilder<'a, 'b, 'tcx>, parent: Module<'b>, @@ -617,8 +620,3 @@ fn visit_block(&mut self, block: &Block) { pub fn build_reduced_graph(resolver: &mut Resolver, krate: &hir::Crate) { GraphBuilder { resolver: resolver }.build_reduced_graph(krate); } - -pub fn populate_module_if_necessary<'a, 'tcx>(resolver: &mut Resolver<'a, 'tcx>, - module: Module<'a>) { - GraphBuilder { resolver: resolver }.populate_module_if_necessary(module); -} diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index ab1d708f952..dcb60addc75 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1542,7 +1542,7 @@ fn resolve_name_in_module(&mut self, -> ResolveResult<&'a NameBinding<'a>> { debug!("(resolving name in module) resolving `{}` in `{}`", name, module_to_string(module)); - build_reduced_graph::populate_module_if_necessary(self, module); + self.populate_module_if_necessary(module); match use_lexical_scope { true => module.resolve_name_in_lexical_scope(name, namespace) .map(Success).unwrap_or(Failed(None)), @@ -3363,7 +3363,7 @@ fn lookup_candidates(&mut self, while let Some((in_module, path_segments, in_module_is_extern)) = worklist.pop() { - build_reduced_graph::populate_module_if_necessary(self, &in_module); + self.populate_module_if_necessary(in_module); in_module.for_each_child(|name, ns, name_binding| { diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 3af5031cc47..351dc8e3192 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -21,8 +21,6 @@ use {names_to_string, module_to_string}; use {resolve_error, ResolutionError}; -use build_reduced_graph; - use rustc::lint; use rustc::middle::def::*; @@ -610,7 +608,7 @@ fn resolve_glob_import(&mut self, let msg = "Cannot glob-import a module into itself.".into(); return Failed(Some((directive.span, msg))); } - build_reduced_graph::populate_module_if_necessary(self.resolver, target_module); + self.resolver.populate_module_if_necessary(target_module); if directive.is_prelude { *module_.prelude.borrow_mut() = Some(target_module); From 1744f55ad7452e72cea2b875e4f53387575ce461 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Wed, 2 Mar 2016 10:22:24 +0000 Subject: [PATCH 05/15] Refactor away GraphBuilder --- src/librustc_resolve/build_reduced_graph.rs | 47 +++++---------------- src/librustc_resolve/lib.rs | 2 +- 2 files changed, 11 insertions(+), 38 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 8d9ce36a148..fac79eb8a28 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -43,26 +43,6 @@ use rustc_front::hir::{Variant, ViewPathGlob, ViewPathList, ViewPathSimple}; use rustc_front::intravisit::{self, Visitor}; -use std::ops::{Deref, DerefMut}; - -struct GraphBuilder<'a, 'b: 'a, 'tcx: 'b> { - resolver: &'a mut Resolver<'b, 'tcx>, -} - -impl<'a, 'b:'a, 'tcx:'b> Deref for GraphBuilder<'a, 'b, 'tcx> { - type Target = Resolver<'b, 'tcx>; - - fn deref(&self) -> &Resolver<'b, 'tcx> { - &*self.resolver - } -} - -impl<'a, 'b:'a, 'tcx:'b> DerefMut for GraphBuilder<'a, 'b, 'tcx> { - fn deref_mut(&mut self) -> &mut Resolver<'b, 'tcx> { - &mut *self.resolver - } -} - trait ToNameBinding<'a> { fn to_name_binding(self) -> NameBinding<'a>; } @@ -80,12 +60,12 @@ fn to_name_binding(self) -> NameBinding<'a> { } } -impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> { +impl<'b, 'tcx:'b> Resolver<'b, 'tcx> { /// Constructs the reduced graph for the entire crate. - fn build_reduced_graph(self, krate: &hir::Crate) { + pub fn build_reduced_graph(&mut self, krate: &hir::Crate) { let mut visitor = BuildReducedGraphVisitor { parent: self.graph_root, - builder: self, + resolver: self, }; intravisit::walk_crate(&mut visitor, krate); } @@ -573,50 +553,43 @@ fn build_import_directive(&mut self, module_.add_import_directive(directive); self.unresolved_imports += 1; } -} -impl<'a, 'tcx> Resolver<'a, 'tcx> { /// Ensures that the reduced graph rooted at the given external module /// is built, building it if it is not. - pub fn populate_module_if_necessary(&mut self, module: Module<'a>) { + pub fn populate_module_if_necessary(&mut self, module: Module<'b>) { if module.populated.get() { return } - let mut builder = GraphBuilder { resolver: self }; for child in self.session.cstore.item_children(module.def_id().unwrap()) { - builder.build_reduced_graph_for_external_crate_def(module, child); + self.build_reduced_graph_for_external_crate_def(module, child); } module.populated.set(true) } } struct BuildReducedGraphVisitor<'a, 'b: 'a, 'tcx: 'b> { - builder: GraphBuilder<'a, 'b, 'tcx>, + resolver: &'a mut Resolver<'b, 'tcx>, parent: Module<'b>, } impl<'a, 'b, 'v, 'tcx> Visitor<'v> for BuildReducedGraphVisitor<'a, 'b, 'tcx> { fn visit_nested_item(&mut self, item: hir::ItemId) { - self.visit_item(self.builder.resolver.ast_map.expect_item(item.id)) + self.visit_item(self.resolver.ast_map.expect_item(item.id)) } fn visit_item(&mut self, item: &Item) { let old_parent = self.parent; - self.builder.build_reduced_graph_for_item(item, &mut self.parent); + self.resolver.build_reduced_graph_for_item(item, &mut self.parent); intravisit::walk_item(self, item); self.parent = old_parent; } fn visit_foreign_item(&mut self, foreign_item: &ForeignItem) { - self.builder.build_reduced_graph_for_foreign_item(foreign_item, &self.parent); + self.resolver.build_reduced_graph_for_foreign_item(foreign_item, &self.parent); } fn visit_block(&mut self, block: &Block) { let old_parent = self.parent; - self.builder.build_reduced_graph_for_block(block, &mut self.parent); + self.resolver.build_reduced_graph_for_block(block, &mut self.parent); intravisit::walk_block(self, block); self.parent = old_parent; } } - -pub fn build_reduced_graph(resolver: &mut Resolver, krate: &hir::Crate) { - GraphBuilder { resolver: resolver }.build_reduced_graph(krate); -} diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index dcb60addc75..e4a2c15d788 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -3734,7 +3734,7 @@ pub fn create_resolver<'a, 'tcx>(session: &'a Session, resolver.callback = callback; - build_reduced_graph::build_reduced_graph(&mut resolver, krate); + resolver.build_reduced_graph(krate); resolve_imports::resolve_imports(&mut resolver); From 07fecf80980c8df846bc63a29ca712b18fdbcfa7 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Sat, 12 Mar 2016 04:49:25 +0000 Subject: [PATCH 06/15] Replace uses of `DefLike` with `Def` (only the `DlDef` variant of `DefLike` was being used) --- src/librustc_resolve/lib.rs | 50 ++++++++++++------------------------- 1 file changed, 16 insertions(+), 34 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index e4a2c15d788..df2caa02812 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -51,7 +51,7 @@ use rustc::front::map as hir_map; use rustc::session::Session; use rustc::lint; -use rustc::middle::cstore::{CrateStore, DefLike, DlDef}; +use rustc::middle::cstore::CrateStore; use rustc::middle::def::*; use rustc::middle::def_id::DefId; use rustc::middle::pat_util::pat_bindings; @@ -756,7 +756,7 @@ enum BareIdentifierPatternResolution { /// One local scope. #[derive(Debug)] struct Rib<'a> { - bindings: HashMap, + bindings: HashMap, kind: RibKind<'a>, } @@ -1594,7 +1594,7 @@ fn with_scope(&mut self, id: NodeId, f: F) /// Searches the current set of local scopes for labels. /// Stops after meeting a closure. - fn search_label(&self, name: Name) -> Option { + fn search_label(&self, name: Name) -> Option { for rib in self.label_ribs.iter().rev() { match rib.kind { NormalRibKind => { @@ -1753,13 +1753,9 @@ fn with_type_parameter_rib<'b, F>(&'b mut self, type_parameters: TypeParameters< seen_bindings.insert(name); // plain insert (no renaming) - function_type_rib.bindings - .insert(name, - DlDef(Def::TyParam(space, - index as u32, - self.ast_map - .local_def_id(type_parameter.id), - name))); + let def_id = self.ast_map.local_def_id(type_parameter.id); + let def = Def::TyParam(space, index as u32, def_id, name); + function_type_rib.bindings.insert(name, def); } self.type_ribs.push(function_type_rib); } @@ -1948,7 +1944,7 @@ fn with_self_rib(&mut self, self_def: Def, f: F) // plain insert (no renaming, types are not currently hygienic....) let name = special_names::type_self; - self_type_rib.bindings.insert(name, DlDef(self_def)); + self_type_rib.bindings.insert(name, self_def); self.type_ribs.push(self_type_rib); f(self); if !self.resolved { @@ -2328,7 +2324,7 @@ fn resolve_pattern(&mut self, if !bindings_list.contains_key(&renamed) { let this = &mut *self; let last_rib = this.value_ribs.last_mut().unwrap(); - last_rib.bindings.insert(renamed, DlDef(def)); + last_rib.bindings.insert(renamed, def); bindings_list.insert(renamed, pat_id); } else if mode == ArgumentIrrefutableMode && bindings_list.contains_key(&renamed) { @@ -2869,25 +2865,11 @@ fn resolve_identifier_in_local_ribs(&mut self, let name = match namespace { ValueNS => ident.name, TypeNS => ident.unhygienic_name }; for i in (0 .. self.get_ribs(namespace).len()).rev() { - if let Some(def_like) = self.get_ribs(namespace)[i].bindings.get(&name).cloned() { - match def_like { - DlDef(def) => { - debug!("(resolving path in local ribs) resolved `{}` to {:?} at {}", - name, - def, - i); - return Some(LocalDef { - ribs: Some((namespace, i)), - def: def, - }); - } - def_like => { - debug!("(resolving path in local ribs) resolved `{}` to pseudo-def {:?}", - name, - def_like); - return None; - } - } + if let Some(def) = self.get_ribs(namespace)[i].bindings.get(&name).cloned() { + return Some(LocalDef { + ribs: Some((namespace, i)), + def: def, + }); } if let ModuleRibKind(module) = self.get_ribs(namespace)[i].kind { @@ -3230,11 +3212,11 @@ fn resolve_expr(&mut self, expr: &Expr) { ExprLoop(_, Some(label)) | ExprWhile(_, _, Some(label)) => { self.with_label_rib(|this| { - let def_like = DlDef(Def::Label(expr.id)); + let def = Def::Label(expr.id); { let rib = this.label_ribs.last_mut().unwrap(); - rib.bindings.insert(label.name, def_like); + rib.bindings.insert(label.name, def); } intravisit::walk_expr(this, expr); @@ -3249,7 +3231,7 @@ fn resolve_expr(&mut self, expr: &Expr) { label.span, ResolutionError::UndeclaredLabel(&label.node.name.as_str())) } - Some(DlDef(def @ Def::Label(_))) => { + Some(def @ Def::Label(_)) => { // Since this def is a label, it is never read. self.record_def(expr.id, PathResolution { From 73417853e40646406a82ea39e606fb6199fa6b5d Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Sat, 12 Mar 2016 08:03:13 +0000 Subject: [PATCH 07/15] Refactor out the common functionality of `resolve_item_in_lexical_scope` and `resolve_identifier_in_local_ribs` into a new function `resolve_ident_in_lexical_scope`. --- src/librustc_resolve/lib.rs | 113 +++++++++++++++++++----------------- 1 file changed, 61 insertions(+), 52 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index df2caa02812..67279207e5b 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -784,6 +784,11 @@ fn from_def(def: Def) -> Self { } } +enum LexicalScopeBinding<'a> { + Item(&'a NameBinding<'a>), + LocalDef(LocalDef), +} + /// The link from a module up to its nearest parent node. #[derive(Clone,Debug)] enum ParentLink<'a> { @@ -1430,40 +1435,66 @@ fn resolve_module_path(&mut self, span) } - /// This function resolves `name` in `namespace` in the current lexical scope, returning - /// Success(binding) if `name` resolves to an item, or Failed(None) if `name` does not resolve - /// or resolves to a type parameter or local variable. - /// n.b. `resolve_identifier_in_local_ribs` also resolves names in the current lexical scope. + /// 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). + /// + /// A block's items are above its local variables in the scope hierarchy, regardless of where + /// the items are defined in the block. For example, + /// ```rust + /// fn f() { + /// g(); // Since there are no local variables in scope yet, this resolves to the item. + /// let g = || {}; + /// fn g() {} + /// g(); // This resolves to the local variable `g` since it shadows the item. + /// } + /// ``` /// /// Invariant: This must only be called during main resolution, not during /// import resolution. + fn resolve_ident_in_lexical_scope(&mut self, + ident: hir::Ident, + ns: Namespace, + record_used: bool) + -> Option> { + let name = match ns { ValueNS => ident.name, TypeNS => ident.unhygienic_name }; + + // Walk backwards up the ribs in scope. + for i in (0 .. self.get_ribs(ns).len()).rev() { + if let Some(def) = self.get_ribs(ns)[i].bindings.get(&name).cloned() { + // The ident resolves to a type parameter or local variable. + return Some(LexicalScopeBinding::LocalDef(LocalDef { + ribs: Some((ns, i)), + def: def, + })); + } + + if let ModuleRibKind(module) = self.get_ribs(ns)[i].kind { + let name = ident.unhygienic_name; + let item = self.resolve_name_in_module(module, name, ns, true, record_used); + if let Success(binding) = item { + // The ident resolves to an item. + return Some(LexicalScopeBinding::Item(binding)); + } + + // We can only see through anonymous modules + if module.def.is_some() { return None; } + } + } + + None + } + fn resolve_item_in_lexical_scope(&mut self, name: Name, namespace: Namespace, record_used: bool) -> ResolveResult<&'a NameBinding<'a>> { - // Walk backwards up the ribs in scope. - for i in (0 .. self.get_ribs(namespace).len()).rev() { - if let Some(_) = self.get_ribs(namespace)[i].bindings.get(&name).cloned() { - // The name resolves to a type parameter or local variable, so return Failed(None). - return Failed(None); - } - - if let ModuleRibKind(module) = self.get_ribs(namespace)[i].kind { - if let Success(binding) = self.resolve_name_in_module(module, - name, - namespace, - true, - record_used) { - // The name resolves to an item. - return Success(binding); - } - // We can only see through anonymous modules - if module.def.is_some() { return Failed(None); } - } + let ident = hir::Ident::from_name(name); + match self.resolve_ident_in_lexical_scope(ident, namespace, record_used) { + Some(LexicalScopeBinding::Item(binding)) => Success(binding), + _ => Failed(None), } - - Failed(None) } /// Returns the nearest normal module parent of the given module. @@ -2861,33 +2892,11 @@ fn resolve_identifier_in_local_ribs(&mut self, namespace: Namespace, record_used: bool) -> Option { - // Check the local set of ribs. - let name = match namespace { ValueNS => ident.name, TypeNS => ident.unhygienic_name }; - - for i in (0 .. self.get_ribs(namespace).len()).rev() { - if let Some(def) = self.get_ribs(namespace)[i].bindings.get(&name).cloned() { - return Some(LocalDef { - ribs: Some((namespace, i)), - def: def, - }); - } - - if let ModuleRibKind(module) = self.get_ribs(namespace)[i].kind { - if let Success(binding) = self.resolve_name_in_module(module, - ident.unhygienic_name, - namespace, - true, - record_used) { - if let Some(def) = binding.def() { - return Some(LocalDef::from_def(def)); - } - } - // We can only see through anonymous modules - if module.def.is_some() { return None; } - } - } - - None + Some(match self.resolve_ident_in_lexical_scope(ident, namespace, record_used) { + Some(LexicalScopeBinding::LocalDef(local_def)) => local_def, + Some(LexicalScopeBinding::Item(binding)) => LocalDef::from_def(binding.def().unwrap()), + None => return None, + }) } fn with_no_errors(&mut self, f: F) -> T From bb09ae28c03d7357f22dd1683e9c78393b5c1e43 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Sat, 12 Mar 2016 08:17:56 +0000 Subject: [PATCH 08/15] Refactor away `resolve_name_in_lexical_scope` and `resolve_identifier_in_local_ribs`. --- src/librustc_resolve/lib.rs | 123 ++++++++++++------------------------ 1 file changed, 42 insertions(+), 81 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 67279207e5b..a1b21da7cfc 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -789,6 +789,26 @@ enum LexicalScopeBinding<'a> { LocalDef(LocalDef), } +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().unwrap()), + } + } + + fn def(self) -> Def { + self.local_def().def + } + + fn module(self) -> Option> { + match self { + LexicalScopeBinding::Item(binding) => binding.module(), + _ => None, + } + } +} + /// The link from a module up to its nearest parent node. #[derive(Clone,Debug)] enum ParentLink<'a> { @@ -1404,20 +1424,13 @@ fn resolve_module_path(&mut self, // 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. - match self.resolve_item_in_lexical_scope(module_path[0], - TypeNS, - true) { - Failed(err) => return Failed(err), - Indeterminate => { - debug!("(resolving module path for import) indeterminate; bailing"); - return Indeterminate; - } - Success(binding) => match binding.module() { - Some(containing_module) => { - search_module = containing_module; - start_index = 1; - } - None => return Failed(None), + let ident = hir::Ident::from_name(module_path[0]); + match self.resolve_ident_in_lexical_scope(ident, TypeNS, true) + .and_then(LexicalScopeBinding::module) { + None => return Failed(None), + Some(containing_module) => { + search_module = containing_module; + start_index = 1; } } } @@ -1485,18 +1498,6 @@ fn resolve_ident_in_lexical_scope(&mut self, None } - fn resolve_item_in_lexical_scope(&mut self, - name: Name, - namespace: Namespace, - record_used: bool) - -> ResolveResult<&'a NameBinding<'a>> { - let ident = hir::Ident::from_name(name); - match self.resolve_ident_in_lexical_scope(ident, namespace, record_used) { - Some(LexicalScopeBinding::Item(binding)) => Success(binding), - _ => Failed(None), - } - } - /// Returns the nearest normal module parent of the given module. fn get_nearest_normal_module_parent(&mut self, module_: Module<'a>) -> Option> { let mut module_ = module_; @@ -2288,8 +2289,7 @@ fn resolve_pattern(&mut self, let ident = path1.node; let renamed = ident.name; - match self.resolve_bare_identifier_pattern(ident.unhygienic_name, - pattern.span) { + match self.resolve_bare_identifier_pattern(ident, pattern.span) { FoundStructOrEnumVariant(def) if const_ok => { debug!("(resolving pattern) resolving `{}` to struct or enum variant", renamed); @@ -2540,49 +2540,21 @@ fn resolve_pattern(&mut self, }); } - fn resolve_bare_identifier_pattern(&mut self, - name: Name, - span: Span) + fn resolve_bare_identifier_pattern(&mut self, ident: hir::Ident, span: Span) -> BareIdentifierPatternResolution { - match self.resolve_item_in_lexical_scope(name, ValueNS, true) { - Success(binding) => { - debug!("(resolve bare identifier pattern) succeeded in finding {} at {:?}", - name, - binding); - match binding.def() { - None => { - panic!("resolved name in the value namespace to a set of name bindings \ - with no def?!"); - } - // For the two success cases, this lookup can be - // considered as not having a private component because - // the lookup happened only within the current module. - Some(def @ Def::Variant(..)) | Some(def @ Def::Struct(..)) => { - return FoundStructOrEnumVariant(def); - } - Some(def @ Def::Const(..)) | Some(def @ Def::AssociatedConst(..)) => { - return FoundConst(def, name); - } - Some(Def::Static(..)) => { - resolve_error(self, span, ResolutionError::StaticVariableReference); - return BareIdentifierPatternUnresolved; - } - _ => return BareIdentifierPatternUnresolved - } + match self.resolve_ident_in_lexical_scope(ident, ValueNS, true) + .map(LexicalScopeBinding::def) { + Some(def @ Def::Variant(..)) | Some(def @ Def::Struct(..)) => { + FoundStructOrEnumVariant(def) } - - Indeterminate => return BareIdentifierPatternUnresolved, - Failed(err) => { - match err { - Some((span, msg)) => { - resolve_error(self, span, ResolutionError::FailedToResolve(&msg)); - } - None => (), - } - - debug!("(resolve bare identifier pattern) failed to find {}", name); - return BareIdentifierPatternUnresolved; + Some(def @ Def::Const(..)) | Some(def @ Def::AssociatedConst(..)) => { + FoundConst(def, ident.unhygienic_name) } + Some(Def::Static(..)) => { + resolve_error(self, span, ResolutionError::StaticVariableReference); + BareIdentifierPatternUnresolved + } + _ => BareIdentifierPatternUnresolved, } } @@ -2703,7 +2675,8 @@ fn resolve_identifier(&mut self, return Some(LocalDef::from_def(Def::Err)); } - self.resolve_identifier_in_local_ribs(identifier, namespace, record_used) + self.resolve_ident_in_lexical_scope(identifier, namespace, record_used) + .map(LexicalScopeBinding::local_def) } // Resolve a local definition, potentially adjusting for closures. @@ -2887,18 +2860,6 @@ fn resolve_crate_relative_path(&mut self, }) } - fn resolve_identifier_in_local_ribs(&mut self, - ident: hir::Ident, - namespace: Namespace, - record_used: bool) - -> Option { - Some(match self.resolve_ident_in_lexical_scope(ident, namespace, record_used) { - Some(LexicalScopeBinding::LocalDef(local_def)) => local_def, - Some(LexicalScopeBinding::Item(binding)) => LocalDef::from_def(binding.def().unwrap()), - None => return None, - }) - } - fn with_no_errors(&mut self, f: F) -> T where F: FnOnce(&mut Resolver) -> T { From 43dffc329402d99a93e224bba1a30f587c70dc7d Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Sun, 13 Mar 2016 02:53:22 +0000 Subject: [PATCH 09/15] Avoid passing around the current module as an argument in `resolve_imports` --- src/librustc_resolve/resolve_imports.rs | 48 +++++++++---------------- 1 file changed, 17 insertions(+), 31 deletions(-) diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 351dc8e3192..769033e11cc 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -17,7 +17,7 @@ use ResolveResult; use ResolveResult::*; use Resolver; -use UseLexicalScopeFlag; +use UseLexicalScopeFlag::DontUseLexicalScope; use {names_to_string, module_to_string}; use {resolve_error, ResolutionError}; @@ -382,7 +382,7 @@ fn resolve_imports_for_module_subtree(&mut self, debug!("(resolving imports for module subtree) resolving {}", module_to_string(&module_)); let orig_module = replace(&mut self.resolver.current_module, module_); - self.resolve_imports_for_module(module_, errors); + self.resolve_imports_in_current_module(errors); self.resolver.current_module = orig_module; for (_, child_module) in module_.module_children.borrow().iter() { @@ -391,22 +391,20 @@ fn resolve_imports_for_module_subtree(&mut self, } /// Attempts to resolve imports for the given module only. - fn resolve_imports_for_module(&mut self, - module: Module<'b>, - errors: &mut Vec>) { + fn resolve_imports_in_current_module(&mut self, errors: &mut Vec>) { let mut imports = Vec::new(); - let mut unresolved_imports = module.unresolved_imports.borrow_mut(); + let mut unresolved_imports = self.resolver.current_module.unresolved_imports.borrow_mut(); ::std::mem::swap(&mut imports, &mut unresolved_imports); for import_directive in imports { - match self.resolve_import_for_module(module, &import_directive) { + match self.resolve_import_for_module(&import_directive) { Failed(err) => { let (span, help) = match err { Some((span, msg)) => (span, format!(". {}", msg)), None => (import_directive.span, String::new()), }; errors.push(ImportResolvingError { - source_module: module, + source_module: self.resolver.current_module, import_directive: import_directive, span: span, help: help, @@ -423,23 +421,15 @@ fn resolve_imports_for_module(&mut self, /// don't know whether the name exists at the moment due to other /// currently-unresolved imports, or success if we know the name exists. /// If successful, the resolved bindings are written into the module. - fn resolve_import_for_module(&mut self, - module_: Module<'b>, - import_directive: &'b ImportDirective) - -> ResolveResult<()> { + fn resolve_import_for_module(&mut self, directive: &'b ImportDirective) -> ResolveResult<()> { debug!("(resolving import for module) resolving import `{}::...` in `{}`", - names_to_string(&import_directive.module_path), - module_to_string(&module_)); + names_to_string(&directive.module_path), + module_to_string(self.resolver.current_module)); self.resolver - .resolve_module_path(&import_directive.module_path, - UseLexicalScopeFlag::DontUseLexicalScope, - import_directive.span) - .and_then(|containing_module| { - // We found the module that the target is contained - // within. Attempt to resolve the import within it. - self.resolve_import(module_, containing_module, import_directive) - }) + .resolve_module_path(&directive.module_path, DontUseLexicalScope, directive.span) + // Once we have the module that contains the target, we can resolve the import. + .and_then(|containing_module| self.resolve_import(containing_module, directive)) .and_then(|()| { // Decrement the count of unresolved imports. assert!(self.resolver.unresolved_imports >= 1); @@ -448,18 +438,16 @@ fn resolve_import_for_module(&mut self, }) } - fn resolve_import(&mut self, - module_: Module<'b>, - target_module: Module<'b>, - directive: &'b ImportDirective) + fn resolve_import(&mut self, target_module: Module<'b>, directive: &'b ImportDirective) -> ResolveResult<()> { let (source, target, value_determined, type_determined) = match directive.subclass { SingleImport { source, target, ref value_determined, ref type_determined } => (source, target, value_determined, type_determined), - GlobImport => return self.resolve_glob_import(module_, target_module, directive), + GlobImport => return self.resolve_glob_import(target_module, directive), }; // We need to resolve both namespaces for this to succeed. + let module_ = self.resolver.current_module; let (value_result, type_result) = { let mut resolve_in_ns = |ns, determined: bool| { // Temporarily count the directive as determined so that the resolution fails @@ -594,15 +582,13 @@ fn resolve_import(&mut self, // succeeds or bails out (as importing * from an empty module or a module // that exports nothing is valid). target_module is the module we are // actually importing, i.e., `foo` in `use foo::*`. - fn resolve_glob_import(&mut self, - module_: Module<'b>, - target_module: Module<'b>, - directive: &'b ImportDirective) + fn resolve_glob_import(&mut self, target_module: Module<'b>, directive: &'b ImportDirective) -> ResolveResult<()> { if let Some(Def::Trait(_)) = target_module.def { self.resolver.session.span_err(directive.span, "items in traits are not importable."); } + let module_ = self.resolver.current_module; if module_.def_id() == target_module.def_id() { // This means we are trying to glob import a module into itself, and it is a no-go let msg = "Cannot glob-import a module into itself.".into(); From 8988c4538e30bce6985cc0b8458521d9a4a87277 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Sun, 13 Mar 2016 02:53:22 +0000 Subject: [PATCH 10/15] Refactor away `resolve_import_for_module` --- src/librustc_resolve/resolve_imports.rs | 30 ++++++++++++------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 769033e11cc..b6fbfe17406 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -397,7 +397,7 @@ fn resolve_imports_in_current_module(&mut self, errors: &mut Vec { let (span, help) = match err { Some((span, msg)) => (span, format!(". {}", msg)), @@ -411,7 +411,11 @@ fn resolve_imports_in_current_module(&mut self, errors: &mut Vec unresolved_imports.push(import_directive), - Success(()) => {} + Success(()) => { + // Decrement the count of unresolved imports. + assert!(self.resolver.unresolved_imports >= 1); + self.resolver.unresolved_imports -= 1; + } } } } @@ -421,25 +425,19 @@ fn resolve_imports_in_current_module(&mut self, errors: &mut Vec ResolveResult<()> { + fn resolve_import(&mut self, directive: &'b ImportDirective) -> ResolveResult<()> { debug!("(resolving import for module) resolving import `{}::...` in `{}`", names_to_string(&directive.module_path), module_to_string(self.resolver.current_module)); - self.resolver - .resolve_module_path(&directive.module_path, DontUseLexicalScope, directive.span) - // Once we have the module that contains the target, we can resolve the import. - .and_then(|containing_module| self.resolve_import(containing_module, directive)) - .and_then(|()| { - // Decrement the count of unresolved imports. - assert!(self.resolver.unresolved_imports >= 1); - self.resolver.unresolved_imports -= 1; - Success(()) - }) - } + let target_module = match self.resolver.resolve_module_path(&directive.module_path, + DontUseLexicalScope, + directive.span) { + Success(module) => module, + Indeterminate => return Indeterminate, + Failed(err) => return Failed(err), + }; - fn resolve_import(&mut self, target_module: Module<'b>, directive: &'b ImportDirective) - -> ResolveResult<()> { let (source, target, value_determined, type_determined) = match directive.subclass { SingleImport { source, target, ref value_determined, ref type_determined } => (source, target, value_determined, type_determined), From 4b6b506ef4b62d5099833b11cb8a639644933c6f Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Sun, 13 Mar 2016 06:00:54 +0000 Subject: [PATCH 11/15] Improve the error message for paths with too many initial `super`s --- src/librustc_resolve/lib.rs | 26 +++++---------------- src/test/compile-fail/super-at-top-level.rs | 2 +- 2 files changed, 7 insertions(+), 21 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index a1b21da7cfc..fba9bdc2cf0 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1382,28 +1382,11 @@ fn resolve_module_path(&mut self, module_to_string(self.current_module)); // Resolve the module prefix, if any. - let module_prefix_result = self.resolve_module_prefix(module_path); + let module_prefix_result = self.resolve_module_prefix(module_path, span); let search_module; let start_index; match module_prefix_result { - Failed(None) => { - let mpath = names_to_string(module_path); - let mpath = &mpath[..]; - match mpath.rfind(':') { - Some(idx) => { - let msg = format!("Could not find `{}` in `{}`", - // idx +- 1 to account for the - // colons on either side - &mpath[idx + 1..], - &mpath[..idx - 1]); - return Failed(Some((span, msg))); - } - None => { - return Failed(None); - } - } - } Failed(err) => return Failed(err), Indeterminate => { debug!("(resolving module path for import) indeterminate; bailing"); @@ -1531,7 +1514,7 @@ fn get_nearest_normal_module_parent_or_self(&mut self, module_: Module<'a>) -> M /// 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: &[Name]) + fn resolve_module_prefix(&mut self, module_path: &[Name], span: Span) -> ResolveResult> { // Start at the current module if we see `self` or `super`, or at the // top of the crate otherwise. @@ -1548,7 +1531,10 @@ fn resolve_module_prefix(&mut self, module_path: &[Name]) debug!("(resolving module prefix) resolving `super` at {}", module_to_string(&containing_module)); match self.get_nearest_normal_module_parent(containing_module) { - None => return Failed(None), + None => { + let msg = "There are too many initial `super`s.".into(); + return Failed(Some((span, msg))); + } Some(new_module) => { containing_module = new_module; i += 1; diff --git a/src/test/compile-fail/super-at-top-level.rs b/src/test/compile-fail/super-at-top-level.rs index 309b6773f60..f59caef4631 100644 --- a/src/test/compile-fail/super-at-top-level.rs +++ b/src/test/compile-fail/super-at-top-level.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use super::f; //~ ERROR unresolved import `super::f` +use super::f; //~ ERROR unresolved import `super::f`. There are too many initial `super`s. fn main() { } From 3e7a22e3bcc264ad5d5a601f106f6fb9ad564d8e Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Mon, 14 Mar 2016 05:15:56 +0000 Subject: [PATCH 12/15] Remove outdated comment --- src/librustc_resolve/lib.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index fba9bdc2cf0..eacea848d94 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1365,9 +1365,6 @@ fn search_parent_externals(needle: Name, module: Module) -> Option { /// Attempts to resolve the module part of an import directive or path /// rooted at the given module. - /// - /// On success, returns the resolved module, and the closest *private* - /// module found to the destination when resolving this path. fn resolve_module_path(&mut self, module_path: &[Name], use_lexical_scope: UseLexicalScopeFlag, From 227cc5cd263514fb3c8811e70ef3c94d0add04d3 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Mon, 14 Mar 2016 07:10:58 +0000 Subject: [PATCH 13/15] Remove an `if` statement with an condition that is always false --- src/librustc_resolve/resolve_imports.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index b6fbfe17406..f958ca2a441 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -283,7 +283,6 @@ fn update_resolution(&self, name: Name, ns: Namespace, update: F) -> T fn define_in_glob_importers(&self, name: Name, ns: Namespace, binding: &'a NameBinding<'a>) { if !binding.defined_with(DefModifiers::PUBLIC | DefModifiers::IMPORTABLE) { return } - if binding.is_extern_crate() { return } for &(importer, directive) in self.glob_importers.borrow_mut().iter() { let _ = importer.try_define_child(name, ns, directive.import(binding, None)); } From 60a836fc930d808e19b39ece8982e46912964aa1 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Thu, 17 Mar 2016 01:05:29 +0000 Subject: [PATCH 14/15] Remove unnecessary `pub`s --- src/librustc_resolve/lib.rs | 32 +++++++++++-------------- src/librustc_resolve/resolve_imports.rs | 12 +++++----- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index eacea848d94..c5a65a658ba 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -95,7 +95,7 @@ // NB: This module needs to be declared first so diagnostics are // registered before they are used. -pub mod diagnostics; +mod diagnostics; mod check_unused; mod build_reduced_graph; @@ -119,12 +119,12 @@ enum SuggestionType { } /// Candidates for a name resolution failure -pub struct SuggestedCandidates { +struct SuggestedCandidates { name: String, candidates: Vec, } -pub enum ResolutionError<'a> { +enum ResolutionError<'a> { /// error E0401: can't use type parameters from outer function TypeParametersFromOuterFunction, /// error E0402: cannot use an outer type parameter in this context @@ -201,7 +201,7 @@ pub enum ResolutionError<'a> { /// Context of where `ResolutionError::UnresolvedName` arose. #[derive(Clone, PartialEq, Eq, Debug)] -pub enum UnresolvedNameContext { +enum UnresolvedNameContext { /// `PathIsMod(id)` indicates that a given path, used in /// expression context, actually resolved to a module rather than /// a value. The `id` attached to the variant is the node id of @@ -1131,7 +1131,7 @@ pub struct Resolver<'a, 'tcx: 'a> { arenas: &'a ResolverArenas<'a>, } -pub struct ResolverArenas<'a> { +struct ResolverArenas<'a> { modules: arena::TypedArena>, name_bindings: arena::TypedArena>, import_directives: arena::TypedArena, @@ -2584,12 +2584,8 @@ fn resolve_possibly_assoc_item(&mut self, /// Skips `path_depth` trailing segments, which is also reflected in the /// returned value. See `middle::def::PathResolution` for more info. - pub fn resolve_path(&mut self, - id: NodeId, - path: &Path, - path_depth: usize, - namespace: Namespace) - -> Option { + fn resolve_path(&mut self, id: NodeId, path: &Path, path_depth: usize, namespace: Namespace) + -> Option { let span = path.span; let segments = &path.segments[..path.segments.len() - path_depth]; @@ -3658,13 +3654,13 @@ pub fn resolve_crate<'a, 'tcx>(session: &'a Session, /// preserving the ribs + current module. This allows resolve_path /// calls to be made with the correct scope info. The node in the /// callback corresponds to the current node in the walk. -pub fn create_resolver<'a, 'tcx>(session: &'a Session, - ast_map: &'a hir_map::Map<'tcx>, - krate: &'a Crate, - make_glob_map: MakeGlobMap, - arenas: &'a ResolverArenas<'a>, - callback: Option bool>>) - -> Resolver<'a, 'tcx> { +fn create_resolver<'a, 'tcx>(session: &'a Session, + ast_map: &'a hir_map::Map<'tcx>, + krate: &'a Crate, + make_glob_map: MakeGlobMap, + arenas: &'a ResolverArenas<'a>, + callback: Option bool>>) + -> Resolver<'a, 'tcx> { let mut resolver = Resolver::new(session, ast_map, make_glob_map, arenas); resolver.callback = callback; diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index f958ca2a441..7c5d131dbc5 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -58,12 +58,12 @@ pub fn single(target: Name, source: Name) -> Self { /// One import directive. #[derive(Debug,Clone)] pub struct ImportDirective { - pub module_path: Vec, - pub subclass: ImportDirectiveSubclass, - pub span: Span, - pub id: NodeId, - pub is_public: bool, // see note in ImportResolutionPerNamespace about how to use this - pub is_prelude: bool, + module_path: Vec, + subclass: ImportDirectiveSubclass, + span: Span, + id: NodeId, + is_public: bool, // see note in ImportResolutionPerNamespace about how to use this + is_prelude: bool, } impl ImportDirective { From e011ae5ea9c48d71772c054771ead2d0f053c8c7 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Mon, 7 Mar 2016 23:10:53 +0000 Subject: [PATCH 15/15] Cleanup trait search --- src/librustc_resolve/lib.rs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index c5a65a658ba..d9fc678554f 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -3233,18 +3233,15 @@ fn add_trait_info(found_traits: &mut Vec, trait_def_id: DefId, name: Name } let mut found_traits = Vec::new(); + // Look for the current trait. + if let Some((trait_def_id, _)) = self.current_trait_ref { + if self.trait_item_map.contains_key(&(name, trait_def_id)) { + add_trait_info(&mut found_traits, trait_def_id, name); + } + } + let mut search_module = self.current_module; loop { - // Look for the current trait. - match self.current_trait_ref { - Some((trait_def_id, _)) => { - if self.trait_item_map.contains_key(&(name, trait_def_id)) { - add_trait_info(&mut found_traits, trait_def_id, name); - } - } - None => {} // Nothing to do. - } - // Look for trait children. let mut search_in_module = |module: Module<'a>| module.for_each_child(|_, ns, binding| { if ns != TypeNS { return }