Auto merge of #32240 - jseyfried:cleanup_resolve, r=nikomatsakis

Cleanup resolve

This is a collection of small refactorings and improvements in `resolve`, most of which could stand alone.
r? @nikomatsakis

src/librustc_resolve/build_reduced_graph.rs

@@ -19,12 +19,11 @@ use resolve_imports::ImportDirectiveSubclass::{self, SingleImport, GlobImport};
 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};
 
-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,30 +41,8 @@ use rustc_front::hir::{ItemForeignMod, ItemImpl, ItemMod, ItemStatic, ItemDefaul
 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;
-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>;
 }
@@ -83,12 +60,12 @@ impl<'a> ToNameBinding<'a> for (Def, Span, DefModifiers) {
     }
 }
 
-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);
     }
@@ -124,7 +101,8 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
     }
 
     /// 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;
@@ -244,7 +222,6 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                                                     is_prelude);
                     }
                 }
-                parent
             }
 
             ItemExternCrate(_) => {
@@ -262,7 +239,6 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
 
                     self.build_reduced_graph_for_external_crate(module);
                 }
-                parent
             }
 
             ItemMod(..) => {
@@ -271,34 +247,30 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                 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, _) => {
@@ -317,7 +289,6 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                     self.build_reduced_graph_for_variant(variant, item_def_id,
                                                          module, variant_modifiers);
                 }
-                parent
             }
 
             // These items live in both the type and value namespaces.
@@ -340,12 +311,9 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                                             .collect();
                 let item_def_id = self.ast_map.local_def_id(item.id);
                 self.structs.insert(item_def_id, field_names);
-
-                parent
             }
 
-            ItemDefaultImpl(_, _) |
+            ItemDefaultImpl(_, _) | ItemImpl(..) => {}
-            ItemImpl(..) => parent,
 
             ItemTrait(_, _, _, ref items) => {
                 let def_id = self.ast_map.local_def_id(item.id);
@@ -370,8 +338,6 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
 
                     self.trait_item_map.insert((item.name, def_id), item_def_id);
                 }
-
-                parent
             }
         }
     }
@@ -422,7 +388,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
         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;
 
@@ -433,48 +399,52 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
             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
+            *parent = new_module;
-        } else {
-            parent
         }
     }
 
-    fn handle_external_def(&mut self,
+    /// Builds the reduced graph for a single item in an external crate.
-                           def: Def,
+    fn build_reduced_graph_for_external_crate_def(&mut self, parent: Module<'b>, xcdef: ChildItem) {
-                           vis: Visibility,
+        let def = match xcdef.def {
-                           final_ident: &str,
+            DlDef(def) => def,
-                           name: Name,
+            _ => return,
-                           new_parent: Module<'b>) {
+        };
-        debug!("(building reduced graph for external crate) building external def {}, priv {:?}",
+
-               final_ident,
+        if let Def::ForeignMod(def_id) = def {
-               vis);
+            // Foreign modules have no names. Recur and populate eagerly.
-        let is_public = vis == hir::Public || new_parent.is_trait();
+            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 {}",
+                debug!("(building reduced graph for external crate) building variant {}", name);
-                       final_ident);
                 // 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(parent, name, TypeNS, (def, DUMMY_SP, modifiers));
-                self.try_define(new_parent, name, ValueNS, (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 +456,11 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
             Def::AssociatedConst(..) |
             Def::Method(..) => {
                 debug!("(building reduced graph for external crate) building value (fn/static) {}",
-                       final_ident);
+                       name);
-                self.try_define(new_parent, name, ValueNS, (def, DUMMY_SP, modifiers));
+                self.try_define(parent, name, ValueNS, (def, DUMMY_SP, modifiers));
             }
             Def::Trait(def_id) => {
-                debug!("(building reduced graph for external crate) building type {}",
+                debug!("(building reduced graph for external crate) building type {}", name);
-                       final_ident);
 
                 // If this is a trait, add all the trait item names to the trait
                 // info.
@@ -508,24 +477,22 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                     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 {}",
+                debug!("(building reduced graph for external crate) building type {}", name);
-                       final_ident);
+                self.try_define(parent, name, TypeNS, (def, DUMMY_SP, modifiers));
-                self.try_define(new_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 \
+                debug!("(building reduced graph for external crate) building type and value for {}",
-                        {}",
+                       name);
-                       final_ident);
+                self.try_define(parent, name, TypeNS, (def, DUMMY_SP, modifiers));
-                self.try_define(new_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,69 +512,6 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
         }
     }
 
-    /// 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 {}",
-               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)
-        }
-        assert!(module.populated.get())
-    }
-
     /// 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>) {
@@ -649,42 +553,43 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
         module_.add_import_directive(directive);
         self.unresolved_imports += 1;
     }
+
+    /// 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<'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)
+    }
 }
 
 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 p = self.builder.build_reduced_graph_for_item(item, &self.parent);
+        let old_parent = self.parent;
-        let old_parent = replace(&mut self.parent, p);
+        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 np = self.builder.build_reduced_graph_for_block(block, &self.parent);
+        let old_parent = self.parent;
-        let old_parent = replace(&mut self.parent, np);
+        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);
-}
-
-pub fn populate_module_if_necessary<'a, 'tcx>(resolver: &mut Resolver<'a, 'tcx>,
-                                              module: Module<'a>) {
-    GraphBuilder { resolver: resolver }.populate_module_if_necessary(module);
-}

src/librustc_resolve/lib.rs

@@ -51,7 +51,7 @@ use rustc::dep_graph::DepNode;
 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;
@@ -95,7 +95,7 @@ use resolve_imports::{ImportDirective, NameResolution};
 
 // 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<Path>,
 }
 
-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
@@ -756,7 +756,7 @@ enum BareIdentifierPatternResolution {
 /// One local scope.
 #[derive(Debug)]
 struct Rib<'a> {
-    bindings: HashMap<Name, DefLike>,
+    bindings: HashMap<Name, Def>,
     kind: RibKind<'a>,
 }
 
@@ -784,6 +784,31 @@ impl LocalDef {
     }
 }
 
+enum LexicalScopeBinding<'a> {
+    Item(&'a NameBinding<'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<Module<'a>> {
+        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> {
@@ -1106,7 +1131,7 @@ pub struct Resolver<'a, 'tcx: 'a> {
     arenas: &'a ResolverArenas<'a>,
 }
 
-pub struct ResolverArenas<'a> {
+struct ResolverArenas<'a> {
     modules: arena::TypedArena<ModuleS<'a>>,
     name_bindings: arena::TypedArena<NameBinding<'a>>,
     import_directives: arena::TypedArena<ImportDirective>,
@@ -1340,9 +1365,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
 
     /// 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,
@@ -1357,28 +1379,11 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                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");
@@ -1399,20 +1404,13 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                         // 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],
+                        let ident = hir::Ident::from_name(module_path[0]);
-                                                                 TypeNS,
+                        match self.resolve_ident_in_lexical_scope(ident, TypeNS, true)
-                                                                 true) {
+                                  .and_then(LexicalScopeBinding::module) {
-                            Failed(err) => return Failed(err),
+                            None => return Failed(None),
-                            Indeterminate => {
+                            Some(containing_module) => {
-                                debug!("(resolving module path for import) indeterminate; bailing");
+                                search_module = containing_module;
-                                return Indeterminate;
+                                start_index = 1;
-                            }
-                            Success(binding) => match binding.module() {
-                                Some(containing_module) => {
-                                    search_module = containing_module;
-                                    start_index = 1;
-                                }
-                                None => return Failed(None),
                             }
                         }
                     }
@@ -1430,40 +1428,54 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                                            span)
     }
 
-    /// This function resolves `name` in `namespace` in the current lexical scope, returning
+    /// This resolves the identifier `ident` in the namespace `ns` in the current lexical scope.
-    /// Success(binding) if `name` resolves to an item, or Failed(None) if `name` does not resolve
+    /// More specifically, we proceed up the hierarchy of scopes and return the binding for
-    /// or resolves to a type parameter or local variable.
+    /// `ident` in the first scope that defines it (or None if no scopes define it).
-    /// n.b. `resolve_identifier_in_local_ribs` also resolves names in the current lexical scope.
+    ///
+    /// 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_item_in_lexical_scope(&mut self,
+    fn resolve_ident_in_lexical_scope(&mut self,
-                                     name: Name,
+                                      ident: hir::Ident,
-                                     namespace: Namespace,
+                                      ns: Namespace,
-                                     record_used: bool)
+                                      record_used: bool)
-                                     -> ResolveResult<&'a NameBinding<'a>> {
+                                      -> Option<LexicalScopeBinding<'a>> {
+        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(namespace).len()).rev() {
+        for i in (0 .. self.get_ribs(ns).len()).rev() {
-            if let Some(_) = self.get_ribs(namespace)[i].bindings.get(&name).cloned() {
+            if let Some(def) = self.get_ribs(ns)[i].bindings.get(&name).cloned() {
-                // The name resolves to a type parameter or local variable, so return Failed(None).
+                // The ident resolves to a type parameter or local variable.
-                return Failed(None);
+                return Some(LexicalScopeBinding::LocalDef(LocalDef {
+                    ribs: Some((ns, i)),
+                    def: def,
+                }));
             }
 
-            if let ModuleRibKind(module) = self.get_ribs(namespace)[i].kind {
+            if let ModuleRibKind(module) = self.get_ribs(ns)[i].kind {
-                if let Success(binding) = self.resolve_name_in_module(module,
+                let name = ident.unhygienic_name;
-                                                                      name,
+                let item = self.resolve_name_in_module(module, name, ns, true, record_used);
-                                                                      namespace,
+                if let Success(binding) = item {
-                                                                      true,
+                    // The ident resolves to an item.
-                                                                      record_used) {
+                    return Some(LexicalScopeBinding::Item(binding));
-                    // The name resolves to an item.
-                    return Success(binding);
                 }
+
                 // We can only see through anonymous modules
-                if module.def.is_some() { return Failed(None); }
+                if module.def.is_some() { return None; }
             }
         }
 
-        Failed(None)
+        None
     }
 
     /// Returns the nearest normal module parent of the given module.
@@ -1499,7 +1511,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
     /// 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<ModulePrefixResult<'a>> {
         // Start at the current module if we see `self` or `super`, or at the
         // top of the crate otherwise.
@@ -1516,7 +1528,10 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             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;
@@ -1542,7 +1557,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                               -> 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)),
@@ -1594,7 +1609,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
 
     /// Searches the current set of local scopes for labels.
     /// Stops after meeting a closure.
-    fn search_label(&self, name: Name) -> Option<DefLike> {
+    fn search_label(&self, name: Name) -> Option<Def> {
         for rib in self.label_ribs.iter().rev() {
             match rib.kind {
                 NormalRibKind => {
@@ -1753,13 +1768,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                     seen_bindings.insert(name);
 
                     // plain insert (no renaming)
-                    function_type_rib.bindings
+                    let def_id = self.ast_map.local_def_id(type_parameter.id);
-                                     .insert(name,
+                    let def = Def::TyParam(space, index as u32, def_id, name);
-                                             DlDef(Def::TyParam(space,
+                    function_type_rib.bindings.insert(name, def);
-                                                              index as u32,
-                                                              self.ast_map
-                                                                  .local_def_id(type_parameter.id),
-                                                              name)));
                 }
                 self.type_ribs.push(function_type_rib);
             }
@@ -1948,7 +1959,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
 
         // 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 {
@@ -2261,8 +2272,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                     let ident = path1.node;
                     let renamed = ident.name;
 
-                    match self.resolve_bare_identifier_pattern(ident.unhygienic_name,
+                    match self.resolve_bare_identifier_pattern(ident, pattern.span) {
-                                                               pattern.span) {
                         FoundStructOrEnumVariant(def) if const_ok => {
                             debug!("(resolving pattern) resolving `{}` to struct or enum variant",
                                    renamed);
@@ -2328,7 +2338,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                             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) {
@@ -2513,49 +2523,21 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         });
     }
 
-    fn resolve_bare_identifier_pattern(&mut self,
+    fn resolve_bare_identifier_pattern(&mut self, ident: hir::Ident, span: Span)
-                                       name: Name,
-                                       span: Span)
                                        -> BareIdentifierPatternResolution {
-        match self.resolve_item_in_lexical_scope(name, ValueNS, true) {
+        match self.resolve_ident_in_lexical_scope(ident, ValueNS, true)
-            Success(binding) => {
+                  .map(LexicalScopeBinding::def) {
-                debug!("(resolve bare identifier pattern) succeeded in finding {} at {:?}",
+            Some(def @ Def::Variant(..)) | Some(def @ Def::Struct(..)) => {
-                       name,
+                FoundStructOrEnumVariant(def)
-                       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
-                }
             }
-
+            Some(def @ Def::Const(..)) | Some(def @ Def::AssociatedConst(..)) => {
-            Indeterminate => return BareIdentifierPatternUnresolved,
+                FoundConst(def, ident.unhygienic_name)
-            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::Static(..)) => {
+                resolve_error(self, span, ResolutionError::StaticVariableReference);
+                BareIdentifierPatternUnresolved
+            }
+            _ => BareIdentifierPatternUnresolved,
         }
     }
 
@@ -2602,12 +2584,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
 
     /// 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,
+    fn resolve_path(&mut self, id: NodeId, path: &Path, path_depth: usize, namespace: Namespace)
-                        id: NodeId,
+                    -> Option<PathResolution> {
-                        path: &Path,
-                        path_depth: usize,
-                        namespace: Namespace)
-                        -> Option<PathResolution> {
         let span = path.span;
         let segments = &path.segments[..path.segments.len() - path_depth];
 
@@ -2676,7 +2654,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             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.
@@ -2860,54 +2839,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         })
     }
 
-    fn resolve_identifier_in_local_ribs(&mut self,
-                                        ident: hir::Ident,
-                                        namespace: Namespace,
-                                        record_used: bool)
-                                        -> Option<LocalDef> {
-        // 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_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 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
-    }
-
     fn with_no_errors<T, F>(&mut self, f: F) -> T
         where F: FnOnce(&mut Resolver) -> T
     {
@@ -3230,11 +3161,11 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
 
             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 +3180,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                                       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 {
@@ -3302,18 +3233,15 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         }
 
         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 }
@@ -3363,7 +3291,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         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| {
 
@@ -3723,18 +3651,18 @@ 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,
+fn create_resolver<'a, 'tcx>(session: &'a Session,
-                                 ast_map: &'a hir_map::Map<'tcx>,
+                             ast_map: &'a hir_map::Map<'tcx>,
-                                 krate: &'a Crate,
+                             krate: &'a Crate,
-                                 make_glob_map: MakeGlobMap,
+                             make_glob_map: MakeGlobMap,
-                                 arenas: &'a ResolverArenas<'a>,
+                             arenas: &'a ResolverArenas<'a>,
-                                 callback: Option<Box<Fn(hir_map::Node, &mut bool) -> bool>>)
+                             callback: Option<Box<Fn(hir_map::Node, &mut bool) -> bool>>)
-                                 -> Resolver<'a, 'tcx> {
+                             -> Resolver<'a, 'tcx> {
     let mut resolver = Resolver::new(session, ast_map, make_glob_map, arenas);
 
     resolver.callback = callback;
 
-    build_reduced_graph::build_reduced_graph(&mut resolver, krate);
+    resolver.build_reduced_graph(krate);
 
     resolve_imports::resolve_imports(&mut resolver);
 

src/librustc_resolve/resolve_imports.rs

@@ -17,12 +17,10 @@ use {NameBinding, NameBindingKind, PrivacyError};
 use ResolveResult;
 use ResolveResult::*;
 use Resolver;
-use UseLexicalScopeFlag;
+use UseLexicalScopeFlag::DontUseLexicalScope;
 use {names_to_string, module_to_string};
 use {resolve_error, ResolutionError};
 
-use build_reduced_graph;
-
 use rustc::lint;
 use rustc::middle::def::*;
 
@@ -60,12 +58,12 @@ impl ImportDirectiveSubclass {
 /// One import directive.
 #[derive(Debug,Clone)]
 pub struct ImportDirective {
-    pub module_path: Vec<Name>,
+    module_path: Vec<Name>,
-    pub subclass: ImportDirectiveSubclass,
+    subclass: ImportDirectiveSubclass,
-    pub span: Span,
+    span: Span,
-    pub id: NodeId,
+    id: NodeId,
-    pub is_public: bool, // see note in ImportResolutionPerNamespace about how to use this
+    is_public: bool, // see note in ImportResolutionPerNamespace about how to use this
-    pub is_prelude: bool,
+    is_prelude: bool,
 }
 
 impl ImportDirective {
@@ -285,7 +283,6 @@ impl<'a> ::ModuleS<'a> {
 
     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));
         }
@@ -384,7 +381,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
         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() {
@@ -393,29 +390,31 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
     }
 
     /// Attempts to resolve imports for the given module only.
-    fn resolve_imports_for_module(&mut self,
+    fn resolve_imports_in_current_module(&mut self, errors: &mut Vec<ImportResolvingError<'b>>) {
-                                  module: Module<'b>,
-                                  errors: &mut Vec<ImportResolvingError<'b>>) {
         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(&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,
                     });
                 }
                 Indeterminate => unresolved_imports.push(import_directive),
-                Success(()) => {}
+                Success(()) => {
+                    // Decrement the count of unresolved imports.
+                    assert!(self.resolver.unresolved_imports >= 1);
+                    self.resolver.unresolved_imports -= 1;
+                }
             }
         }
     }
@@ -425,43 +424,27 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
     /// 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,
+    fn resolve_import(&mut self, directive: &'b ImportDirective) -> ResolveResult<()> {
-                                 module_: Module<'b>,
-                                 import_directive: &'b ImportDirective)
-                                 -> ResolveResult<()> {
         debug!("(resolving import for module) resolving import `{}::...` in `{}`",
-               names_to_string(&import_directive.module_path),
+               names_to_string(&directive.module_path),
-               module_to_string(&module_));
+               module_to_string(self.resolver.current_module));
 
-        self.resolver
+        let target_module = match self.resolver.resolve_module_path(&directive.module_path,
-            .resolve_module_path(&import_directive.module_path,
+                                                                    DontUseLexicalScope,
-                                 UseLexicalScopeFlag::DontUseLexicalScope,
+                                                                    directive.span) {
-                                 import_directive.span)
+            Success(module) => module,
-            .and_then(|containing_module| {
+            Indeterminate => return Indeterminate,
-                // We found the module that the target is contained
+            Failed(err) => return Failed(err),
-                // within. Attempt to resolve the import within it.
+        };
-                self.resolve_import(module_, containing_module, import_directive)
-            })
-            .and_then(|()| {
-                // Decrement the count of unresolved imports.
-                assert!(self.resolver.unresolved_imports >= 1);
-                self.resolver.unresolved_imports -= 1;
-                Success(())
-            })
-    }
 
-    fn resolve_import(&mut self,
-                      module_: Module<'b>,
-                      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
@@ -596,21 +579,19 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
     // 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,
+    fn resolve_glob_import(&mut self, target_module: Module<'b>, directive: &'b ImportDirective)
-                           module_: Module<'b>,
-                           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();
             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);

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() {
 }