resolve: diagnostics improvement and groundwork for RFC 1560

src/librustc/traits/object_safety.rs

@@ -17,7 +17,6 @@
 //!   - not reference the erased type `Self` except for in this receiver;
 //!   - not have generic type parameters
 
-use super::supertraits;
 use super::elaborate_predicates;
 
 use hir::def_id::DefId;

src/librustc_resolve/build_reduced_graph.rs

@@ -16,7 +16,7 @@
 use resolve_imports::ImportDirectiveSubclass::{self, GlobImport};
 use Module;
 use Namespace::{self, TypeNS, ValueNS};
-use {NameBinding, NameBindingKind};
+use {NameBinding, NameBindingKind, ToNameBinding};
 use ParentLink::{ModuleParentLink, BlockParentLink};
 use Resolver;
 use {resolve_error, resolve_struct_error, ResolutionError};
@@ -39,10 +39,6 @@ use syntax::visit::{self, Visitor};
 
 use syntax_pos::{Span, DUMMY_SP};
 
-trait ToNameBinding<'a> {
-    fn to_name_binding(self) -> NameBinding<'a>;
-}
-
 impl<'a> ToNameBinding<'a> for (Module<'a>, Span, ty::Visibility) {
     fn to_name_binding(self) -> NameBinding<'a> {
         NameBinding { kind: NameBindingKind::Module(self.0), span: self.1, vis: self.2 }
@@ -68,18 +64,13 @@ impl<'b> Resolver<'b> {
         visit::walk_crate(&mut visitor, krate);
     }
 
-    /// Defines `name` in namespace `ns` of module `parent` to be `def` if it is not yet defined.
-    fn try_define<T>(&self, parent: Module<'b>, name: Name, ns: Namespace, def: T)
-        where T: ToNameBinding<'b>
-    {
-        let _ = parent.try_define_child(name, ns, def.to_name_binding());
-    }
-
     /// Defines `name` in namespace `ns` of module `parent` to be `def` if it is not yet defined;
     /// otherwise, reports an error.
-    fn define<T: ToNameBinding<'b>>(&self, parent: Module<'b>, name: Name, ns: Namespace, def: T) {
+    fn define<T>(&mut self, parent: Module<'b>, name: Name, ns: Namespace, def: T)
+        where T: ToNameBinding<'b>,
+    {
         let binding = def.to_name_binding();
-        if let Err(old_binding) = parent.try_define_child(name, ns, binding.clone()) {
+        if let Err(old_binding) = self.try_define(parent, name, ns, binding.clone()) {
             self.report_conflict(parent, name, ns, old_binding, &binding);
         }
     }
@@ -399,14 +390,14 @@ impl<'b> Resolver<'b> {
                        name, vis);
                 let parent_link = ModuleParentLink(parent, name);
                 let module = self.new_module(parent_link, Some(def), true);
-                self.try_define(parent, name, TypeNS, (module, DUMMY_SP, vis));
+                let _ = self.try_define(parent, name, TypeNS, (module, DUMMY_SP, vis));
             }
             Def::Variant(_, variant_id) => {
                 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.
-                self.try_define(parent, name, TypeNS, (def, DUMMY_SP, vis));
-                self.try_define(parent, name, ValueNS, (def, DUMMY_SP, vis));
+                let _ = self.try_define(parent, name, TypeNS, (def, DUMMY_SP, vis));
+                let _ = self.try_define(parent, name, ValueNS, (def, DUMMY_SP, vis));
                 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());
@@ -419,7 +410,7 @@ impl<'b> Resolver<'b> {
             Def::Method(..) => {
                 debug!("(building reduced graph for external crate) building value (fn/static) {}",
                        name);
-                self.try_define(parent, name, ValueNS, (def, DUMMY_SP, vis));
+                let _ = self.try_define(parent, name, ValueNS, (def, DUMMY_SP, vis));
             }
             Def::Trait(def_id) => {
                 debug!("(building reduced graph for external crate) building type {}", name);
@@ -441,20 +432,20 @@ impl<'b> Resolver<'b> {
 
                 let parent_link = ModuleParentLink(parent, name);
                 let module = self.new_module(parent_link, Some(def), true);
-                self.try_define(parent, name, TypeNS, (module, DUMMY_SP, vis));
+                let _ = self.try_define(parent, name, TypeNS, (module, DUMMY_SP, vis));
             }
             Def::TyAlias(..) | Def::AssociatedTy(..) => {
                 debug!("(building reduced graph for external crate) building type {}", name);
-                self.try_define(parent, name, TypeNS, (def, DUMMY_SP, vis));
+                let _ = self.try_define(parent, name, TypeNS, (def, DUMMY_SP, vis));
             }
             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 {}",
                        name);
-                self.try_define(parent, name, TypeNS, (def, DUMMY_SP, vis));
+                let _ = self.try_define(parent, name, TypeNS, (def, DUMMY_SP, vis));
                 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(parent, name, ValueNS, (def, DUMMY_SP, vis));
+                    let _ = self.try_define(parent, name, ValueNS, (def, DUMMY_SP, vis));
                 }
 
                 // Record the def ID and fields of this struct.

src/librustc_resolve/lib.rs

@@ -158,7 +158,7 @@ enum ResolutionError<'a> {
     /// error E0435: attempt to use a non-constant value in a constant
     AttemptToUseNonConstantValueInConstant,
     /// error E0530: X bindings cannot shadow Ys
-    BindingShadowsSomethingUnacceptable(&'a str, &'a str, Name),
+    BindingShadowsSomethingUnacceptable(&'a str, Name, &'a NameBinding<'a>),
     /// error E0531: unresolved pattern path kind `name`
     PatPathUnresolved(&'a str, &'a Path),
     /// error E0532: expected pattern path kind, found another pattern path kind
@@ -422,17 +422,16 @@ fn resolve_struct_error<'b, 'a: 'b, 'c>(resolver: &'b Resolver<'a>,
                              E0435,
                              "attempt to use a non-constant value in a constant")
         }
-        ResolutionError::BindingShadowsSomethingUnacceptable(what_binding, shadows_what, name) => {
+        ResolutionError::BindingShadowsSomethingUnacceptable(what_binding, name, binding) => {
+            let shadows_what = PathResolution::new(binding.def().unwrap()).kind_name();
             let mut err = struct_span_err!(resolver.session,
                                            span,
                                            E0530,
                                            "{}s cannot shadow {}s", what_binding, shadows_what);
             err.span_label(span, &format!("cannot be named the same as a {}", shadows_what));
-            if let Success(binding) = resolver.current_module.resolve_name(name, ValueNS, true) {
-                let participle = if binding.is_import() { "imported" } else { "defined" };
-                err.span_label(binding.span, &format!("a {} `{}` is {} here",
-                                                      shadows_what, name, participle));
-            }
+            let participle = if binding.is_import() { "imported" } else { "defined" };
+            let msg = &format!("a {} `{}` is {} here", shadows_what, name, participle);
+            err.span_label(binding.span, msg);
             err
         }
         ResolutionError::PatPathUnresolved(expected_what, path) => {
@@ -712,12 +711,16 @@ impl<'a> LexicalScopeBinding<'a> {
         }
     }
 
-    fn module(self) -> Option<Module<'a>> {
+    fn item(self) -> Option<&'a NameBinding<'a>> {
         match self {
-            LexicalScopeBinding::Item(binding) => binding.module(),
+            LexicalScopeBinding::Item(binding) => Some(binding),
             _ => None,
         }
     }
+
+    fn module(self) -> Option<Module<'a>> {
+        self.item().and_then(NameBinding::module)
+    }
 }
 
 /// The link from a module up to its nearest parent node.
@@ -818,6 +821,16 @@ pub struct NameBinding<'a> {
     vis: ty::Visibility,
 }
 
+pub trait ToNameBinding<'a> {
+    fn to_name_binding(self) -> NameBinding<'a>;
+}
+
+impl<'a> ToNameBinding<'a> for NameBinding<'a> {
+    fn to_name_binding(self) -> NameBinding<'a> {
+        self
+    }
+}
+
 #[derive(Clone, Debug)]
 enum NameBindingKind<'a> {
     Def(Def),
@@ -1197,34 +1210,27 @@ impl<'a> Resolver<'a> {
         match ns { ValueNS => &mut self.value_ribs, TypeNS => &mut self.type_ribs }
     }
 
-    #[inline]
-    fn record_use(&mut self, name: Name, binding: &'a NameBinding<'a>) {
+    fn record_use(&mut self, name: Name, ns: Namespace, binding: &'a NameBinding<'a>) {
         // track extern crates for unused_extern_crate lint
         if let Some(DefId { krate, .. }) = binding.module().and_then(ModuleS::def_id) {
             self.used_crates.insert(krate);
         }
 
-        let directive = match binding.kind {
-            NameBindingKind::Import { directive, .. } => directive,
-            _ => return,
-        };
-
-        if !self.make_glob_map {
-            return;
-        }
-        if self.glob_map.contains_key(&directive.id) {
-            self.glob_map.get_mut(&directive.id).unwrap().insert(name);
-            return;
+        if let NameBindingKind::Import { directive, .. } = binding.kind {
+            self.used_imports.insert((directive.id, ns));
+            self.add_to_glob_map(directive.id, name);
         }
+    }
 
-        let mut new_set = FnvHashSet();
-        new_set.insert(name);
-        self.glob_map.insert(directive.id, new_set);
+    fn add_to_glob_map(&mut self, id: NodeId, name: Name) {
+        if self.make_glob_map {
+            self.glob_map.entry(id).or_insert_with(FnvHashSet).insert(name);
+        }
     }
 
-    /// Resolves the given module path from the given root `module_`.
+    /// Resolves the given module path from the given root `search_module`.
     fn resolve_module_path_from_root(&mut self,
-                                     module_: Module<'a>,
+                                     mut search_module: Module<'a>,
                                      module_path: &[Name],
                                      index: usize,
                                      span: Span)
@@ -1241,7 +1247,6 @@ impl<'a> Resolver<'a> {
             }
         }
 
-        let mut search_module = module_;
         let mut index = index;
         let module_path_len = module_path.len();
 
@@ -1438,31 +1443,30 @@ impl<'a> Resolver<'a> {
     }
 
     /// Returns the nearest normal module parent of the given module.
-    fn get_nearest_normal_module_parent(&self, module_: Module<'a>) -> Option<Module<'a>> {
-        let mut module_ = module_;
+    fn get_nearest_normal_module_parent(&self, mut module: Module<'a>) -> Option<Module<'a>> {
         loop {
-            match module_.parent_link {
+            match module.parent_link {
                 NoParentLink => return None,
                 ModuleParentLink(new_module, _) |
                 BlockParentLink(new_module, _) => {
                     let new_module = new_module;
                     if new_module.is_normal() {
                         return Some(new_module);
                     }
-                    module_ = new_module;
+                    module = new_module;
                 }
             }
         }
     }
 
     /// Returns the nearest normal module parent of the given module, or the
     /// module itself if it is a normal module.
-    fn get_nearest_normal_module_parent_or_self(&self, module_: Module<'a>) -> Module<'a> {
-        if module_.is_normal() {
-            return module_;
+    fn get_nearest_normal_module_parent_or_self(&self, module: Module<'a>) -> Module<'a> {
+        if module.is_normal() {
+            return module;
         }
-        match self.get_nearest_normal_module_parent(module_) {
-            None => module_,
+        match self.get_nearest_normal_module_parent(module) {
+            None => module,
             Some(new_module) => new_module,
         }
     }
@@ -1479,8 +1483,8 @@ impl<'a> Resolver<'a> {
             "super" => 0,
             _ => return Success(NoPrefixFound),
         };
-        let module_ = self.current_module;
-        let mut containing_module = self.get_nearest_normal_module_parent_or_self(module_);
+        let mut containing_module =
+            self.get_nearest_normal_module_parent_or_self(self.current_module);
 
         // Now loop through all the `super`s we find.
         while i < module_path.len() && "super" == module_path[i].as_str() {
@@ -1519,10 +1523,7 @@ impl<'a> Resolver<'a> {
         self.populate_module_if_necessary(module);
         module.resolve_name(name, namespace, use_lexical_scope).and_then(|binding| {
             if record_used {
-                if let NameBindingKind::Import { directive, .. } = binding.kind {
-                    self.used_imports.insert((directive.id, namespace));
-                }
-                self.record_use(name, binding);
+                self.record_use(name, namespace, binding);
             }
             Success(binding)
         })
@@ -2316,16 +2317,17 @@ impl<'a> Resolver<'a> {
                 PatKind::Ident(bmode, ref ident, ref opt_pat) => {
                     // First try to resolve the identifier as some existing
                     // entity, then fall back to a fresh binding.
-                    let resolution = self.resolve_identifier(ident.node, ValueNS, true)
-                                         .map(|local_def| PathResolution::new(local_def.def))
-                                         .and_then(|resolution| {
+                    let binding = self.resolve_ident_in_lexical_scope(ident.node, ValueNS, false)
+                                      .and_then(LexicalScopeBinding::item);
+                    let resolution = binding.and_then(NameBinding::def).and_then(|def| {
                         let always_binding = !pat_src.is_refutable() || opt_pat.is_some() ||
                                              bmode != BindingMode::ByValue(Mutability::Immutable);
-                        match resolution.base_def {
+                        match def {
                             Def::Struct(..) | Def::Variant(..) |
                             Def::Const(..) | Def::AssociatedConst(..) if !always_binding => {
                                 // A constant, unit variant, etc pattern.
-                                Some(resolution)
+                                self.record_use(ident.node.name, ValueNS, binding.unwrap());
+                                Some(PathResolution::new(def))
                             }
                             Def::Struct(..) | Def::Variant(..) |
                             Def::Const(..) | Def::AssociatedConst(..) | Def::Static(..) => {
@@ -2334,7 +2336,7 @@ impl<'a> Resolver<'a> {
                                     self,
                                     ident.span,
                                     ResolutionError::BindingShadowsSomethingUnacceptable(
-                                        pat_src.descr(), resolution.kind_name(), ident.node.name)
+                                        pat_src.descr(), ident.node.name, binding.unwrap())
                                 );
                                 None
                             }
@@ -3144,10 +3146,10 @@ impl<'a> Resolver<'a> {
                         if let NameBindingKind::Import { directive, .. } = binding.kind {
                             let id = directive.id;
                             this.maybe_unused_trait_imports.insert(id);
+                            this.add_to_glob_map(id, trait_name);
                             import_id = Some(id);
                         }
                         add_trait_info(&mut found_traits, trait_def_id, import_id, name);
-                        this.record_use(trait_name, binding);
                     }
                 }
             };