auto merge of #13685 : Ryman/rust/issue7575, r=alexcrichton

Closes #7575.

I don't think the change from a contains lookup to an iteration of the HashSet in the resolver should be much of a burden as the set of methods with the same name should be relatively small.

Author: bors

src/librustc/middle/resolve.rs

@@ -16,7 +16,7 @@ use metadata::decoder::{DefLike, DlDef, DlField, DlImpl};
 use middle::lang_items::LanguageItems;
 use middle::lint::{UnnecessaryQualification, UnusedImports};
 use middle::pat_util::pat_bindings;
-use util::nodemap::{NodeMap, DefIdSet, FnvHashSet};
+use util::nodemap::{NodeMap, DefIdSet, FnvHashMap};
 
 use syntax::ast::*;
 use syntax::ast;
@@ -821,7 +821,7 @@ fn Resolver<'a>(session: &'a Session,
 
         graph_root: graph_root,
 
-        method_set: RefCell::new(FnvHashSet::new()),
+        method_map: RefCell::new(FnvHashMap::new()),
         structs: HashSet::new(),
 
         unresolved_imports: 0,
@@ -860,7 +860,7 @@ struct Resolver<'a> {
 
     graph_root: NameBindings,
 
-    method_set: RefCell<FnvHashSet<(Name, DefId)>>,
+    method_map: RefCell<FnvHashMap<(Name, DefId), ast::ExplicitSelf_>>,
     structs: HashSet<DefId>,
 
     // The number of imports that are currently unresolved.
@@ -1371,10 +1371,8 @@ impl<'a> Resolver<'a> {
                                        ty_m.span);
                     method_name_bindings.define_value(def, ty_m.span, true);
 
-                    // Add it to the trait info if not static.
-                    if ty_m.explicit_self.node != SelfStatic {
-                        self.method_set.borrow_mut().insert((ident.name, def_id));
-                    }
+                    self.method_map.borrow_mut().insert((ident.name, def_id),
+                                                        ty_m.explicit_self.node);
                 }
 
                 name_bindings.define_type(DefTrait(def_id), sp, is_public);
@@ -1660,10 +1658,8 @@ impl<'a> Resolver<'a> {
                           trait method '{}'",
                          token::get_ident(method_name));
 
-                  // Add it to the trait info if not static.
-                  if explicit_self != SelfStatic {
-                      self.method_set.borrow_mut().insert((method_name.name, def_id));
-                  }
+                  self.method_map.borrow_mut().insert((method_name.name, def_id), explicit_self);
+
                   if is_exported {
                       self.external_exports.insert(method_def_id);
                   }
@@ -4718,10 +4714,16 @@ impl<'a> Resolver<'a> {
         match containing_module.kind.get() {
             TraitModuleKind | ImplModuleKind => {
                 match containing_module.def_id.get() {
-                    Some(def_id) if self.method_set.borrow().contains(&(ident.name, def_id)) => {
-                        debug!("containing module was a trait or impl \
+                    Some(def_id) => {
+                        match self.method_map.borrow().find(&(ident.name, def_id)) {
+                            Some(x) if *x == SelfStatic => (),
+                            None => (),
+                            _ => {
+                                debug!("containing module was a trait or impl \
                                 and name was a method -> not resolved");
-                        return None;
+                                return None;
+                            }
+                        }
                     },
                     _ => (),
                 }
@@ -5110,9 +5112,9 @@ impl<'a> Resolver<'a> {
             // Look for the current trait.
             match self.current_trait_refs {
                 Some(ref trait_def_ids) => {
-                    let method_set = self.method_set.borrow();
+                    let method_map = self.method_map.borrow();
                     for &trait_def_id in trait_def_ids.iter() {
-                        if method_set.contains(&(name, trait_def_id)) {
+                        if method_map.contains_key(&(name, trait_def_id)) {
                             add_trait_info(&mut found_traits, trait_def_id, name);
                         }
                     }
@@ -5126,7 +5128,7 @@ impl<'a> Resolver<'a> {
             self.populate_module_if_necessary(&search_module);
 
             {
-                let method_set = self.method_set.borrow();
+                let method_map = self.method_map.borrow();
                 for (_, child_names) in search_module.children.borrow().iter() {
                     let def = match child_names.def_for_namespace(TypeNS) {
                         Some(def) => def,
@@ -5136,7 +5138,7 @@ impl<'a> Resolver<'a> {
                         DefTrait(trait_def_id) => trait_def_id,
                         _ => continue,
                     };
-                    if method_set.contains(&(name, trait_def_id)) {
+                    if method_map.contains_key(&(name, trait_def_id)) {
                         add_trait_info(&mut found_traits, trait_def_id, name);
                     }
                 }
@@ -5152,7 +5154,7 @@ impl<'a> Resolver<'a> {
                     Some(DefTrait(trait_def_id)) => trait_def_id,
                     Some(..) | None => continue,
                 };
-                if self.method_set.borrow().contains(&(name, did)) {
+                if self.method_map.borrow().contains_key(&(name, did)) {
                     add_trait_info(&mut found_traits, did, name);
                     self.used_imports.insert((import.type_id, TypeNS));
                 }

src/librustc/middle/typeck/check/method.rs

@@ -118,6 +118,12 @@ pub enum AutoderefReceiverFlag {
     DontAutoderefReceiver,
 }
 
+#[deriving(Eq)]
+pub enum StaticMethodsFlag {
+    ReportStaticMethods,
+    IgnoreStaticMethods,
+}
+
 pub fn lookup<'a>(
         fcx: &'a FnCtxt<'a>,
 
@@ -129,7 +135,8 @@ pub fn lookup<'a>(
         supplied_tps: &'a [ty::t],          // The list of types X, Y, ... .
         deref_args: check::DerefArgs,       // Whether we autopointer first.
         check_traits: CheckTraitsFlag,      // Whether we check traits only.
-        autoderef_receiver: AutoderefReceiverFlag)
+        autoderef_receiver: AutoderefReceiverFlag,
+        report_statics: StaticMethodsFlag)
      -> Option<MethodCallee> {
     let mut lcx = LookupContext {
         fcx: fcx,
@@ -143,6 +150,7 @@ pub fn lookup<'a>(
         deref_args: deref_args,
         check_traits: check_traits,
         autoderef_receiver: autoderef_receiver,
+        report_statics: report_statics,
     };
 
     debug!("method lookup(self_ty={}, expr={}, self_expr={})",
@@ -173,7 +181,8 @@ pub fn lookup_in_trait<'a>(
         trait_did: DefId,                   // The trait to limit the lookup to.
         self_ty: ty::t,                     // The type of `a`.
         supplied_tps: &'a [ty::t],          // The list of types X, Y, ... .
-        autoderef_receiver: AutoderefReceiverFlag)
+        autoderef_receiver: AutoderefReceiverFlag,
+        report_statics: StaticMethodsFlag)
      -> Option<MethodCallee> {
     let mut lcx = LookupContext {
         fcx: fcx,
@@ -187,6 +196,7 @@ pub fn lookup_in_trait<'a>(
         deref_args: check::DoDerefArgs,
         check_traits: CheckTraitsOnly,
         autoderef_receiver: autoderef_receiver,
+        report_statics: report_statics,
     };
 
     debug!("method lookup_in_trait(self_ty={}, self_expr={})",
@@ -197,8 +207,6 @@ pub fn lookup_in_trait<'a>(
     lcx.search(self_ty)
 }
 
-
-
 // Determine the index of a method in the list of all methods belonging
 // to a trait and its supertraits.
 fn get_method_index(tcx: &ty::ctxt,
@@ -301,6 +309,7 @@ struct LookupContext<'a> {
     deref_args: check::DerefArgs,
     check_traits: CheckTraitsFlag,
     autoderef_receiver: AutoderefReceiverFlag,
+    report_statics: StaticMethodsFlag,
 }
 
 /**
@@ -661,7 +670,17 @@ impl<'a> LookupContext<'a> {
                                  impl_did: DefId,
                                  impl_methods: &[DefId],
                                  is_extension: bool) {
-        if !self.impl_dups.insert(impl_did) {
+        let did = if self.report_statics == ReportStaticMethods {
+            // we only want to report each base trait once
+            match ty::impl_trait_ref(self.tcx(), impl_did) {
+                Some(trait_ref) => trait_ref.def_id,
+                None => impl_did
+            }
+        } else {
+            impl_did
+        };
+
+        if !self.impl_dups.insert(did) {
             return; // already visited
         }
 
@@ -1018,6 +1037,25 @@ impl<'a> LookupContext<'a> {
             return None;
         }
 
+        if self.report_statics == ReportStaticMethods {
+            // lookup should only be called with ReportStaticMethods if a regular lookup failed
+            assert!(relevant_candidates.iter().all(|c| c.method_ty.explicit_self == SelfStatic));
+
+            self.tcx().sess.fileline_note(self.span,
+                                "found defined static methods, maybe a `self` is missing?");
+
+            for (idx, candidate) in relevant_candidates.iter().enumerate() {
+                self.report_candidate(idx, &candidate.origin);
+            }
+
+            // return something so we don't get errors for every mutability
+            return Some(MethodCallee {
+                origin: relevant_candidates.get(0).origin,
+                ty: ty::mk_err(),
+                substs: substs::empty()
+            });
+        }
+
         if relevant_candidates.len() > 1 {
             self.tcx().sess.span_err(
                 self.span,
@@ -1305,7 +1343,7 @@ impl<'a> LookupContext<'a> {
         return match candidate.method_ty.explicit_self {
             SelfStatic => {
                 debug!("(is relevant?) explicit self is static");
-                false
+                self.report_statics == ReportStaticMethods
             }
 
             SelfValue => {
@@ -1391,11 +1429,20 @@ impl<'a> LookupContext<'a> {
     fn report_candidate(&self, idx: uint, origin: &MethodOrigin) {
         match *origin {
             MethodStatic(impl_did) => {
-                // If it is an instantiated default method, use the original
-                // default method for error reporting.
-                let did = match provided_source(self.tcx(), impl_did) {
-                    None => impl_did,
-                    Some(did) => did
+                let did = if self.report_statics == ReportStaticMethods {
+                    // If we're reporting statics, we want to report the trait
+                    // definition if possible, rather than an impl
+                    match ty::trait_method_of_method(self.tcx(), impl_did) {
+                        None => {debug!("(report candidate) No trait method found"); impl_did},
+                        Some(trait_did) => {debug!("(report candidate) Found trait ref"); trait_did}
+                    }
+                } else {
+                    // If it is an instantiated default method, use the original
+                    // default method for error reporting.
+                    match provided_source(self.tcx(), impl_did) {
+                        None => impl_did,
+                        Some(did) => did
+                    }
                 };
                 self.report_static_candidate(idx, did)
             }

src/librustc/middle/typeck/check/mod.rs

@@ -97,6 +97,7 @@ use middle::typeck::check::method::{AutoderefReceiver};
 use middle::typeck::check::method::{AutoderefReceiverFlag};
 use middle::typeck::check::method::{CheckTraitsAndInherentMethods};
 use middle::typeck::check::method::{DontAutoderefReceiver};
+use middle::typeck::check::method::{IgnoreStaticMethods, ReportStaticMethods};
 use middle::typeck::check::regionmanip::replace_late_bound_regions_in_fn_sig;
 use middle::typeck::check::regionmanip::relate_free_regions;
 use middle::typeck::check::vtable::VtableContext;
@@ -1360,7 +1361,7 @@ fn try_overloaded_deref(fcx: &FnCtxt,
         (PreferMutLvalue, Some(trait_did)) => {
             method::lookup_in_trait(fcx, span, base_expr.map(|x| &*x),
                                     token::intern("deref_mut"), trait_did,
-                                    base_ty, [], DontAutoderefReceiver)
+                                    base_ty, [], DontAutoderefReceiver, IgnoreStaticMethods)
         }
         _ => None
     };
@@ -1370,7 +1371,7 @@ fn try_overloaded_deref(fcx: &FnCtxt,
         (None, Some(trait_did)) => {
             method::lookup_in_trait(fcx, span, base_expr.map(|x| &*x),
                                     token::intern("deref"), trait_did,
-                                    base_ty, [], DontAutoderefReceiver)
+                                    base_ty, [], DontAutoderefReceiver, IgnoreStaticMethods)
         }
         (method, _) => method
     };
@@ -1956,7 +1957,7 @@ fn check_expr_with_unifier(fcx: &FnCtxt,
                                          expr_t, tps.as_slice(),
                                          DontDerefArgs,
                                          CheckTraitsAndInherentMethods,
-                                         AutoderefReceiver) {
+                                         AutoderefReceiver, IgnoreStaticMethods) {
             Some(method) => {
                 let method_ty = method.ty;
                 let method_call = MethodCall::expr(expr.id);
@@ -1976,6 +1977,15 @@ fn check_expr_with_unifier(fcx: &FnCtxt,
 
                 // Add error type for the result
                 fcx.write_error(expr.id);
+
+                // Check for potential static matches (missing self parameters)
+                method::lookup(fcx, expr, rcvr,
+                                    method_name.node.name,
+                                    expr_t, tps.as_slice(),
+                                    DontDerefArgs,
+                                    CheckTraitsAndInherentMethods,
+                                    DontAutoderefReceiver, ReportStaticMethods);
+
                 ty::mk_err()
             }
         };
@@ -2040,7 +2050,8 @@ fn check_expr_with_unifier(fcx: &FnCtxt,
         let method = match trait_did {
             Some(trait_did) => {
                 method::lookup_in_trait(fcx, op_ex.span, Some(&*args[0]), opname,
-                                        trait_did, self_t, [], autoderef_receiver)
+                                        trait_did, self_t, [], autoderef_receiver,
+                                        IgnoreStaticMethods)
             }
             None => None
         };
@@ -2367,7 +2378,8 @@ fn check_expr_with_unifier(fcx: &FnCtxt,
                              tps.as_slice(),
                              DontDerefArgs,
                              CheckTraitsAndInherentMethods,
-                             AutoderefReceiver) {
+                             AutoderefReceiver,
+                             IgnoreStaticMethods) {
             Some(_) => {
                 fcx.type_error_message(
                     expr.span,