Auto merge of #43600 - scalexm:issue-35976, r=nikomatsakis

Add a more precise error message for issue #35976

When trying to perform static dispatch on something which derefs to a trait object, and the target trait is not in scope, we had confusing error messages if the target method had a `Self: Sized` bound. We add a more precise error message in this case: "consider using trait ...".

Fixes #35976.

r? @nikomatsakis

7 files changed, 229 insertions, 49 deletions

diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs
index f245b1503da..eef0bcc3753 100644
--- a/src/librustc/ty/mod.rs
+++ b/src/librustc/ty/mod.rs
@@ -160,7 +160,7 @@ pub struct ImplHeader<'tcx> {
     pub predicates: Vec<Predicate<'tcx>>,
 }
 
-#[derive(Copy, Clone, Debug)]
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
 pub struct AssociatedItem {
     pub def_id: DefId,
     pub name: Name,
diff --git a/src/librustc_typeck/check/method/confirm.rs b/src/librustc_typeck/check/method/confirm.rs
index ad4ee5a9d6d..b6a5ce0a6ce 100644
--- a/src/librustc_typeck/check/method/confirm.rs
+++ b/src/librustc_typeck/check/method/confirm.rs
@@ -38,6 +38,11 @@ impl<'a, 'gcx, 'tcx> Deref for ConfirmContext<'a, 'gcx, 'tcx> {
     }
 }
 
+pub struct ConfirmResult<'tcx> {
+    pub callee: MethodCallee<'tcx>,
+    pub illegal_sized_bound: bool,
+}
+
 impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
     pub fn confirm_method(&self,
                           span: Span,
@@ -46,7 +51,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                           unadjusted_self_ty: Ty<'tcx>,
                           pick: probe::Pick<'tcx>,
                           segment: &hir::PathSegment)
-                          -> MethodCallee<'tcx> {
+                          -> ConfirmResult<'tcx> {
         debug!("confirm(unadjusted_self_ty={:?}, pick={:?}, generic_args={:?})",
                unadjusted_self_ty,
                pick,
@@ -75,7 +80,7 @@ impl<'a, 'gcx, 'tcx> ConfirmContext<'a, 'gcx, 'tcx> {
                unadjusted_self_ty: Ty<'tcx>,
                pick: probe::Pick<'tcx>,
                segment: &hir::PathSegment)
-               -> MethodCallee<'tcx> {
+               -> ConfirmResult<'tcx> {
         // Adjust the self expression the user provided and obtain the adjusted type.
         let self_ty = self.adjust_self_ty(unadjusted_self_ty, &pick);
 
@@ -91,12 +96,26 @@ impl<'a, 'gcx, 'tcx> ConfirmContext<'a, 'gcx, 'tcx> {
         // Create the final signature for the method, replacing late-bound regions.
         let (method_sig, method_predicates) = self.instantiate_method_sig(&pick, all_substs);
 
+        // If there is a `Self: Sized` bound and `Self` is a trait object, it is possible that
+        // something which derefs to `Self` actually implements the trait and the caller
+        // wanted to make a static dispatch on it but forgot to import the trait.
+        // See test `src/test/ui/issue-35976.rs`.
+        //
+        // In that case, we'll error anyway, but we'll also re-run the search with all traits
+        // in scope, and if we find another method which can be used, we'll output an
+        // appropriate hint suggesting to import the trait.
+        let illegal_sized_bound = self.predicates_require_illegal_sized_bound(&method_predicates);
+
         // Unify the (adjusted) self type with what the method expects.
         self.unify_receivers(self_ty, method_sig.inputs()[0]);
 
         // Add any trait/regions obligations specified on the method's type parameters.
-        let method_ty = self.tcx.mk_fn_ptr(ty::Binder(method_sig));
-        self.add_obligations(method_ty, all_substs, &method_predicates);
+        // We won't add these if we encountered an illegal sized bound, so that we can use
+        // a custom error in that case.
+        if !illegal_sized_bound {
+            let method_ty = self.tcx.mk_fn_ptr(ty::Binder(method_sig));
+            self.add_obligations(method_ty, all_substs, &method_predicates);
+        }
 
         // Create the final `MethodCallee`.
         let callee = MethodCallee {
@@ -109,7 +128,7 @@ impl<'a, 'gcx, 'tcx> ConfirmContext<'a, 'gcx, 'tcx> {
             self.convert_lvalue_derefs_to_mutable();
         }
 
-        callee
+        ConfirmResult { callee, illegal_sized_bound }
     }
 
     ///////////////////////////////////////////////////////////////////////////
@@ -533,6 +552,30 @@ impl<'a, 'gcx, 'tcx> ConfirmContext<'a, 'gcx, 'tcx> {
     ///////////////////////////////////////////////////////////////////////////
     // MISCELLANY
 
+    fn predicates_require_illegal_sized_bound(&self,
+                                              predicates: &ty::InstantiatedPredicates<'tcx>)
+                                              -> bool {
+        let sized_def_id = match self.tcx.lang_items.sized_trait() {
+            Some(def_id) => def_id,
+            None => return false,
+        };
+
+        traits::elaborate_predicates(self.tcx, predicates.predicates.clone())
+            .filter_map(|predicate| {
+                match predicate {
+                    ty::Predicate::Trait(trait_pred) if trait_pred.def_id() == sized_def_id =>
+                        Some(trait_pred),
+                    _ => None,
+                }
+            })
+            .any(|trait_pred| {
+                match trait_pred.0.self_ty().sty {
+                    ty::TyDynamic(..) => true,
+                    _ => false,
+                }
+            })
+    }
+
     fn enforce_illegal_method_limitations(&self, pick: &probe::Pick) {
         // Disallow calls to the method `drop` defined in the `Drop` trait.
         match pick.item.container {
diff --git a/src/librustc_typeck/check/method/mod.rs b/src/librustc_typeck/check/method/mod.rs
index c842e47aaf5..dd5b0cdda42 100644
--- a/src/librustc_typeck/check/method/mod.rs
+++ b/src/librustc_typeck/check/method/mod.rs
@@ -33,7 +33,7 @@ mod confirm;
 pub mod probe;
 mod suggest;
 
-use self::probe::IsSuggestion;
+use self::probe::{IsSuggestion, ProbeScope};
 
 #[derive(Clone, Copy, Debug)]
 pub struct MethodCallee<'tcx> {
@@ -60,6 +60,10 @@ pub enum MethodError<'tcx> {
 
     // Found an applicable method, but it is not visible.
     PrivateMatch(Def),
+
+    // Found a `Self: Sized` bound where `Self` is a trait object, also the caller may have
+    // forgotten to import a trait.
+    IllegalSizedBound(Vec<DefId>),
 }
 
 // Contains a list of static methods that may apply, a list of unsatisfied trait predicates which
@@ -106,12 +110,13 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                          -> bool {
         let mode = probe::Mode::MethodCall;
         match self.probe_for_name(span, mode, method_name, IsSuggestion(false),
-                                  self_ty, call_expr_id) {
+                                  self_ty, call_expr_id, ProbeScope::TraitsInScope) {
             Ok(..) => true,
             Err(NoMatch(..)) => false,
             Err(Ambiguity(..)) => true,
             Err(ClosureAmbiguity(..)) => true,
             Err(PrivateMatch(..)) => allow_private,
+            Err(IllegalSizedBound(..)) => true,
         }
     }
 
@@ -142,10 +147,13 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                call_expr,
                self_expr);
 
-        let mode = probe::Mode::MethodCall;
-        let self_ty = self.resolve_type_vars_if_possible(&self_ty);
-        let pick = self.probe_for_name(span, mode, segment.name, IsSuggestion(false),
-                                       self_ty, call_expr.id)?;
+        let pick = self.lookup_probe(
+            span,
+            segment.name,
+            self_ty,
+            call_expr,
+            ProbeScope::TraitsInScope
+        )?;
 
         if let Some(import_id) = pick.import_id {
             let import_def_id = self.tcx.hir.local_def_id(import_id);
@@ -155,12 +163,56 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
 
         self.tcx.check_stability(pick.item.def_id, call_expr.id, span);
 
-        Ok(self.confirm_method(span,
-                               self_expr,
-                               call_expr,
-                               self_ty,
-                               pick,
-                               segment))
+        let result = self.confirm_method(span,
+                                         self_expr,
+                                         call_expr,
+                                         self_ty,
+                                         pick.clone(),
+                                         segment);
+
+        if result.illegal_sized_bound {
+            // We probe again, taking all traits into account (not only those in scope).
+            let candidates =
+                match self.lookup_probe(span,
+                                        segment.name,
+                                        self_ty,
+                                        call_expr,
+                                        ProbeScope::AllTraits) {
+
+                    // If we find a different result the caller probably forgot to import a trait.
+                    Ok(ref new_pick) if *new_pick != pick => vec![new_pick.item.container.id()],
+                    Err(Ambiguity(ref sources)) => {
+                        sources.iter()
+                               .filter_map(|source| {
+                                   match *source {
+                                       // Note: this cannot come from an inherent impl,
+                                       // because the first probing succeeded.
+                                       ImplSource(def) => self.tcx.trait_id_of_impl(def),
+                                       TraitSource(_) => None,
+                                   }
+                               })
+                               .collect()
+                    }
+                    _ => Vec::new(),
+                };
+
+            return Err(IllegalSizedBound(candidates));
+        }
+
+        Ok(result.callee)
+    }
+
+    fn lookup_probe(&self,
+                    span: Span,
+                    method_name: ast::Name,
+                    self_ty: ty::Ty<'tcx>,
+                    call_expr: &'gcx hir::Expr,
+                    scope: ProbeScope)
+                    -> probe::PickResult<'tcx> {
+        let mode = probe::Mode::MethodCall;
+        let self_ty = self.resolve_type_vars_if_possible(&self_ty);
+        self.probe_for_name(span, mode, method_name, IsSuggestion(false),
+                            self_ty, call_expr.id, scope)
     }
 
     /// `lookup_method_in_trait` is used for overloaded operators.
@@ -299,7 +351,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                         -> Result<Def, MethodError<'tcx>> {
         let mode = probe::Mode::Path;
         let pick = self.probe_for_name(span, mode, method_name, IsSuggestion(false),
-                                       self_ty, expr_id)?;
+                                       self_ty, expr_id, ProbeScope::TraitsInScope)?;
 
         if let Some(import_id) = pick.import_id {
             let import_def_id = self.tcx.hir.local_def_id(import_id);
diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs
index dfc5cd00b6e..3195b10404d 100644
--- a/src/librustc_typeck/check/method/probe.rs
+++ b/src/librustc_typeck/check/method/probe.rs
@@ -106,7 +106,7 @@ enum CandidateKind<'tcx> {
                          ty::PolyTraitRef<'tcx>),
 }
 
-#[derive(Debug)]
+#[derive(Debug, PartialEq, Eq, Clone)]
 pub struct Pick<'tcx> {
     pub item: ty::AssociatedItem,
     pub kind: PickKind<'tcx>,
@@ -130,7 +130,7 @@ pub struct Pick<'tcx> {
     pub unsize: Option<Ty<'tcx>>,
 }
 
-#[derive(Clone,Debug)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub enum PickKind<'tcx> {
     InherentImplPick,
     ExtensionImplPick(// Impl
@@ -155,6 +155,15 @@ pub enum Mode {
     Path,
 }
 
+#[derive(PartialEq, Eq, Copy, Clone, Debug)]
+pub enum ProbeScope {
+    // Assemble candidates coming only from traits in scope.
+    TraitsInScope,
+
+    // Assemble candidates coming from all traits.
+    AllTraits,
+}
+
 impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
     /// This is used to offer suggestions to users. It returns methods
     /// that could have been called which have the desired return
@@ -175,14 +184,14 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                scope_expr_id);
         let method_names =
             self.probe_op(span, mode, LookingFor::ReturnType(return_type), IsSuggestion(true),
-                          self_ty, scope_expr_id,
+                          self_ty, scope_expr_id, ProbeScope::TraitsInScope,
                           |probe_cx| Ok(probe_cx.candidate_method_names()))
                 .unwrap_or(vec![]);
         method_names
             .iter()
             .flat_map(|&method_name| {
                 match self.probe_for_name(span, mode, method_name, IsSuggestion(true), self_ty,
-                                          scope_expr_id) {
+                                          scope_expr_id, ProbeScope::TraitsInScope) {
                     Ok(pick) => Some(pick.item),
                     Err(_) => None,
                 }
@@ -196,7 +205,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                           item_name: ast::Name,
                           is_suggestion: IsSuggestion,
                           self_ty: Ty<'tcx>,
-                          scope_expr_id: ast::NodeId)
+                          scope_expr_id: ast::NodeId,
+                          scope: ProbeScope)
                           -> PickResult<'tcx> {
         debug!("probe(self_ty={:?}, item_name={}, scope_expr_id={})",
                self_ty,
@@ -208,6 +218,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                       is_suggestion,
                       self_ty,
                       scope_expr_id,
+                      scope,
                       |probe_cx| probe_cx.pick())
     }
 
@@ -218,6 +229,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                       is_suggestion: IsSuggestion,
                       self_ty: Ty<'tcx>,
                       scope_expr_id: ast::NodeId,
+                      scope: ProbeScope,
                       op: OP)
                       -> Result<R, MethodError<'tcx>>
         where OP: FnOnce(ProbeContext<'a, 'gcx, 'tcx>) -> Result<R, MethodError<'tcx>>
@@ -275,8 +287,14 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
             let mut probe_cx =
                 ProbeContext::new(self, span, mode, looking_for,
                                   steps, opt_simplified_steps);
+
             probe_cx.assemble_inherent_candidates();
-            probe_cx.assemble_extension_candidates_for_traits_in_scope(scope_expr_id)?;
+            match scope {
+                ProbeScope::TraitsInScope =>
+                    probe_cx.assemble_extension_candidates_for_traits_in_scope(scope_expr_id)?,
+                ProbeScope::AllTraits =>
+                    probe_cx.assemble_extension_candidates_for_all_traits()?,
+            };
             op(probe_cx)
         })
     }
diff --git a/src/librustc_typeck/check/method/suggest.rs b/src/librustc_typeck/check/method/suggest.rs
index 4faf71e0cc9..c480febdec6 100644
--- a/src/librustc_typeck/check/method/suggest.rs
+++ b/src/librustc_typeck/check/method/suggest.rs
@@ -315,7 +315,42 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                 let msg = format!("{} `{}` is private", def.kind_name(), item_name);
                 self.tcx.sess.span_err(span, &msg);
             }
+
+            MethodError::IllegalSizedBound(candidates) => {
+                let msg = format!("the `{}` method cannot be invoked on a trait object", item_name);
+                let mut err = self.sess().struct_span_err(span, &msg);
+                if !candidates.is_empty() {
+                    let help = format!("{an}other candidate{s} {were} found in the following \
+                                        trait{s}, perhaps add a `use` for {one_of_them}:",
+                                    an = if candidates.len() == 1 {"an" } else { "" },
+                                    s = if candidates.len() == 1 { "" } else { "s" },
+                                    were = if candidates.len() == 1 { "was" } else { "were" },
+                                    one_of_them = if candidates.len() == 1 {
+                                        "it"
+                                    } else {
+                                        "one_of_them"
+                                    });
+                    self.suggest_use_candidates(&mut err, help, candidates);
+                }
+                err.emit();
+            }
+        }
+    }
+
+    fn suggest_use_candidates(&self,
+                              err: &mut DiagnosticBuilder,
+                              mut msg: String,
+                              candidates: Vec<DefId>) {
+        let limit = if candidates.len() == 5 { 5 } else { 4 };
+        for (i, trait_did) in candidates.iter().take(limit).enumerate() {
+            msg.push_str(&format!("\ncandidate #{}: `use {};`",
+                                    i + 1,
+                                    self.tcx.item_path_str(*trait_did)));
+        }
+        if candidates.len() > limit {
+            msg.push_str(&format!("\nand {} others", candidates.len() - limit));
         }
+        err.note(&msg[..]);
     }
 
     fn suggest_traits_to_import(&self,
@@ -330,30 +365,20 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
             candidates.sort();
             candidates.dedup();
             err.help("items from traits can only be used if the trait is in scope");
-            let mut msg = format!("the following {traits_are} implemented but not in scope, \
-                                   perhaps add a `use` for {one_of_them}:",
-                              traits_are = if candidates.len() == 1 {
-                                  "trait is"
-                              } else {
-                                  "traits are"
-                              },
-                              one_of_them = if candidates.len() == 1 {
-                                  "it"
-                              } else {
-                                  "one of them"
-                              });
-
-            let limit = if candidates.len() == 5 { 5 } else { 4 };
-            for (i, trait_did) in candidates.iter().take(limit).enumerate() {
-                msg.push_str(&format!("\ncandidate #{}: `use {};`",
-                                      i + 1,
-                                      self.tcx.item_path_str(*trait_did)));
-            }
-            if candidates.len() > limit {
-                msg.push_str(&format!("\nand {} others", candidates.len() - limit));
-            }
-            err.note(&msg[..]);
-
+            let msg = format!("the following {traits_are} implemented but not in scope, \
+                               perhaps add a `use` for {one_of_them}:",
+                            traits_are = if candidates.len() == 1 {
+                                "trait is"
+                            } else {
+                                "traits are"
+                            },
+                            one_of_them = if candidates.len() == 1 {
+                                "it"
+                            } else {
+                                "one of them"
+                            });
+
+            self.suggest_use_candidates(err, msg, candidates);
             return;
         }
 
diff --git a/src/test/ui/issue-35976.rs b/src/test/ui/issue-35976.rs
new file mode 100644
index 00000000000..169d7b55916
--- /dev/null
+++ b/src/test/ui/issue-35976.rs
@@ -0,0 +1,31 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+mod private {
+    pub trait Future {
+        fn wait(&self) where Self: Sized;
+    }
+
+    impl Future for Box<Future> {
+        fn wait(&self) { }
+    }
+}
+
+//use private::Future;
+
+fn bar(arg: Box<private::Future>) {
+    arg.wait();
+    //~^ ERROR the `wait` method cannot be invoked on a trait object
+    //~| another candidate was found in the following trait, perhaps add a `use` for it:
+}
+
+fn main() {
+
+}
diff --git a/src/test/ui/issue-35976.stderr b/src/test/ui/issue-35976.stderr
new file mode 100644
index 00000000000..9fb67449734
--- /dev/null
+++ b/src/test/ui/issue-35976.stderr
@@ -0,0 +1,11 @@
+error: the `wait` method cannot be invoked on a trait object
+  --> $DIR/issue-35976.rs:24:9
+   |
+24 |     arg.wait();
+   |         ^^^^
+   |
+   = note: another candidate was found in the following trait, perhaps add a `use` for it:
+           candidate #1: `use private::Future;`
+
+error: aborting due to previous error
+