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
This commit is contained in:
bors 2017-08-04 15:03:00 +00:00
commit dae8864dbe
7 changed files with 228 additions and 48 deletions

View File

@ -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,

View File

@ -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 {

View File

@ -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);

View File

@ -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)
})
}

View File

@ -315,9 +315,44 @@ 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,
err: &mut DiagnosticBuilder,
span: Span,
@ -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;
}

View File

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

View File

@ -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