Skip to content

Commit

Permalink
Suggest borrowing when it would satisfy an unmet trait bound
Browse files Browse the repository at this point in the history
When there are multiple implementors for the same trait that is present
in an unmet binding, modify the E0277 error to refer to the parent
obligation and verify whether borrowing the argument being passed in
would satisfy the unmet bound. If it would, suggest it.
  • Loading branch information
estebank committed Nov 16, 2019
1 parent 405a3dd commit f57413b
Show file tree
Hide file tree
Showing 10 changed files with 107 additions and 24 deletions.
4 changes: 0 additions & 4 deletions src/libcore/ops/function.rs
Expand Up @@ -137,10 +137,6 @@ pub trait Fn<Args> : FnMut<Args> {
#[rustc_paren_sugar]
#[rustc_on_unimplemented(
on(Args="()", note="wrap the `{Self}` in a closure with no arguments: `|| {{ /* code */ }}"),
on(
all(Args="(char,)", _Self="std::string::String"),
note="borrowing the `{Self}` might fix the problem"
),
message="expected a `{FnMut}<{Args}>` closure, found `{Self}`",
label="expected an `FnMut<{Args}>` closure, found `{Self}`",
)]
Expand Down
74 changes: 72 additions & 2 deletions src/librustc/traits/error_reporting.rs
Expand Up @@ -33,7 +33,7 @@ use crate::ty::subst::Subst;
use crate::ty::SubtypePredicate;
use crate::util::nodemap::{FxHashMap, FxHashSet};

use errors::{Applicability, DiagnosticBuilder, pluralize};
use errors::{Applicability, DiagnosticBuilder, pluralise, Style};
use std::fmt;
use syntax::ast;
use syntax::symbol::{sym, kw};
Expand Down Expand Up @@ -723,7 +723,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
post_message,
pre_message,
) = self.get_parent_trait_ref(&obligation.cause.code)
.map(|t| (format!(" in `{}`", t), format!("within `{}`, ", t)))
.map(|t| (format!(" in `{}`", t), format!("within `{}`, ", t)))
.unwrap_or_default();

let OnUnimplementedNote {
Expand Down Expand Up @@ -787,6 +787,16 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
self.suggest_borrow_on_unsized_slice(&obligation.cause.code, &mut err);
self.suggest_fn_call(&obligation, &mut err, &trait_ref, points_at_arg);
self.suggest_remove_reference(&obligation, &mut err, &trait_ref);
if self.suggest_add_reference_to_arg(
&obligation,
&mut err,
&trait_ref,
points_at_arg,
) {
self.note_obligation_cause(&mut err, obligation);
err.emit();
return;
}
self.suggest_semicolon_removal(&obligation, &mut err, span, &trait_ref);

// Try to report a help message
Expand Down Expand Up @@ -1302,6 +1312,66 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
}
}

fn suggest_add_reference_to_arg(
&self,
obligation: &PredicateObligation<'tcx>,
err: &mut DiagnosticBuilder<'tcx>,
trait_ref: &ty::Binder<ty::TraitRef<'tcx>>,
points_at_arg: bool,
) -> bool {
if !points_at_arg {
return false;
}

let span = obligation.cause.span;
let param_env = obligation.param_env;
let trait_ref = trait_ref.skip_binder();

if let ObligationCauseCode::ImplDerivedObligation(obligation) = &obligation.cause.code {
// Try to apply the original trait binding obligation by borrowing.
let self_ty = trait_ref.self_ty();
let found = self_ty.to_string();
let new_self_ty = self.tcx.mk_imm_ref(self.tcx.lifetimes.re_static, self_ty);
let substs = self.tcx.mk_substs_trait(new_self_ty, &[]);
let new_trait_ref = ty::TraitRef::new(obligation.parent_trait_ref.def_id(), substs);
let new_obligation = Obligation::new(
ObligationCause::dummy(),
param_env,
new_trait_ref.to_predicate(),
);
if self.predicate_may_hold(&new_obligation) {
if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) {
// We have a very specific type of error, where just borrowing this argument
// might solve the problem. In cases like this, the important part is the
// original type obligation, not the last one that failed, which is arbitrary.
// Because of this, we modify the error to refer to the original obligation and
// return early in the caller.
err.message = vec![(
format!(
"the trait bound `{}: {}` is not satisfied",
found,
obligation.parent_trait_ref.skip_binder(),
),
Style::NoStyle,
)];
if snippet.starts_with('&') {
// This is already a literal borrow and the obligation is failing
// somewhere else in the obligation chain. Do not suggest non-sense.
return false;
}
err.span_suggestion(
span,
"consider borrowing here",
format!("&{}", snippet),
Applicability::MachineApplicable,
);
return true;
}
}
}
false
}

/// Whenever references are used by mistake, like `for (i, e) in &vec.iter().enumerate()`,
/// suggest removing these references until we reach a type that implements the trait.
fn suggest_remove_reference(
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/derives/deriving-copyclone.rs
Expand Up @@ -28,10 +28,10 @@ fn main() {
is_clone(B { a: 1, b: 2 });

// B<C> cannot be copied or cloned
is_copy(B { a: 1, b: C }); //~ERROR Copy
is_clone(B { a: 1, b: C }); //~ERROR Clone
is_copy(B { a: 1, b: C }); //~ ERROR Copy
is_clone(B { a: 1, b: C }); //~ ERROR Clone

// B<D> can be cloned but not copied
is_copy(B { a: 1, b: D }); //~ERROR Copy
is_copy(B { a: 1, b: D }); //~ ERROR Copy
is_clone(B { a: 1, b: D });
}
15 changes: 12 additions & 3 deletions src/test/ui/derives/deriving-copyclone.stderr
Expand Up @@ -5,7 +5,10 @@ LL | fn is_copy<T: Copy>(_: T) {}
| ------- ---- required by this bound in `is_copy`
...
LL | is_copy(B { a: 1, b: C });
| ^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `C`
| ^^^^^^^^^^^^^^^^
| |
| the trait `std::marker::Copy` is not implemented for `C`
| help: consider borrowing here: `&B { a: 1, b: C }`
|
= note: required because of the requirements on the impl of `std::marker::Copy` for `B<C>`

Expand All @@ -16,7 +19,10 @@ LL | fn is_clone<T: Clone>(_: T) {}
| -------- ----- required by this bound in `is_clone`
...
LL | is_clone(B { a: 1, b: C });
| ^^^^^^^^^^^^^^^^ the trait `std::clone::Clone` is not implemented for `C`
| ^^^^^^^^^^^^^^^^
| |
| the trait `std::clone::Clone` is not implemented for `C`
| help: consider borrowing here: `&B { a: 1, b: C }`
|
= note: required because of the requirements on the impl of `std::clone::Clone` for `B<C>`

Expand All @@ -27,7 +33,10 @@ LL | fn is_copy<T: Copy>(_: T) {}
| ------- ---- required by this bound in `is_copy`
...
LL | is_copy(B { a: 1, b: D });
| ^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `D`
| ^^^^^^^^^^^^^^^^
| |
| the trait `std::marker::Copy` is not implemented for `D`
| help: consider borrowing here: `&B { a: 1, b: D }`
|
= note: required because of the requirements on the impl of `std::marker::Copy` for `B<D>`

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/kindck/kindck-impl-type-params-2.rs
Expand Up @@ -11,5 +11,5 @@ fn take_param<T:Foo>(foo: &T) { }
fn main() {
let x: Box<_> = box 3;
take_param(&x);
//~^ ERROR `std::boxed::Box<{integer}>: std::marker::Copy` is not satisfied
//~^ ERROR the trait bound `std::boxed::Box<{integer}>: Foo` is not satisfied
}
2 changes: 1 addition & 1 deletion src/test/ui/kindck/kindck-impl-type-params-2.stderr
@@ -1,4 +1,4 @@
error[E0277]: the trait bound `std::boxed::Box<{integer}>: std::marker::Copy` is not satisfied
error[E0277]: the trait bound `std::boxed::Box<{integer}>: Foo` is not satisfied
--> $DIR/kindck-impl-type-params-2.rs:13:16
|
LL | fn take_param<T:Foo>(foo: &T) { }
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/kindck/kindck-inherited-copy-bound.curr.stderr
@@ -1,4 +1,4 @@
error[E0277]: the trait bound `std::boxed::Box<{integer}>: std::marker::Copy` is not satisfied
error[E0277]: the trait bound `std::boxed::Box<{integer}>: Foo` is not satisfied
--> $DIR/kindck-inherited-copy-bound.rs:21:16
|
LL | fn take_param<T:Foo>(foo: &T) { }
Expand Down
8 changes: 5 additions & 3 deletions src/test/ui/suggestions/issue-62843.stderr
@@ -1,11 +1,13 @@
error[E0277]: expected a `std::ops::FnMut<(char,)>` closure, found `std::string::String`
error[E0277]: the trait bound `std::string::String: std::str::pattern::Pattern<'_>` is not satisfied
--> $DIR/issue-62843.rs:4:32
|
LL | println!("{:?}", line.find(pattern));
| ^^^^^^^ expected an `FnMut<(char,)>` closure, found `std::string::String`
| ^^^^^^^
| |
| expected an `FnMut<(char,)>` closure, found `std::string::String`
| help: consider borrowing here: `&pattern`
|
= help: the trait `std::ops::FnMut<(char,)>` is not implemented for `std::string::String`
= note: borrowing the `std::string::String` might fix the problem
= note: required because of the requirements on the impl of `std::str::pattern::Pattern<'_>` for `std::string::String`

error: aborting due to previous error
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/traits/traits-negative-impls.rs
Expand Up @@ -46,7 +46,7 @@ fn dummy2() {
impl !Send for TestType {}

is_send(Box::new(TestType));
//~^ ERROR `dummy2::TestType` cannot be sent between threads safely
//~^ ERROR the trait bound `dummy2::TestType: std::marker::Send` is not satisfied
}

fn dummy3() {
Expand All @@ -64,5 +64,5 @@ fn main() {
// This will complain about a missing Send impl because `Sync` is implement *just*
// for T that are `Send`. Look at #20366 and #19950
is_sync(Outer2(TestType));
//~^ ERROR `main::TestType` cannot be sent between threads safely
//~^ ERROR the trait bound `main::TestType: std::marker::Sync` is not satisfied
}
14 changes: 10 additions & 4 deletions src/test/ui/traits/traits-negative-impls.stderr
Expand Up @@ -43,14 +43,17 @@ LL | is_send((8, TestType));
= help: within `({integer}, dummy1c::TestType)`, the trait `std::marker::Send` is not implemented for `dummy1c::TestType`
= note: required because it appears within the type `({integer}, dummy1c::TestType)`

error[E0277]: `dummy2::TestType` cannot be sent between threads safely
error[E0277]: the trait bound `dummy2::TestType: std::marker::Send` is not satisfied
--> $DIR/traits-negative-impls.rs:48:13
|
LL | fn is_send<T: Send>(_: T) {}
| ------- ---- required by this bound in `is_send`
...
LL | is_send(Box::new(TestType));
| ^^^^^^^^^^^^^^^^^^ `dummy2::TestType` cannot be sent between threads safely
| ^^^^^^^^^^^^^^^^^^
| |
| `dummy2::TestType` cannot be sent between threads safely
| help: consider borrowing here: `&Box::new(TestType)`
|
= help: the trait `std::marker::Send` is not implemented for `dummy2::TestType`
= note: required because of the requirements on the impl of `std::marker::Send` for `std::ptr::Unique<dummy2::TestType>`
Expand All @@ -70,14 +73,17 @@ LL | is_send(Box::new(Outer2(TestType)));
= note: required because of the requirements on the impl of `std::marker::Send` for `std::ptr::Unique<Outer2<dummy3::TestType>>`
= note: required because it appears within the type `std::boxed::Box<Outer2<dummy3::TestType>>`

error[E0277]: `main::TestType` cannot be sent between threads safely
error[E0277]: the trait bound `main::TestType: std::marker::Sync` is not satisfied
--> $DIR/traits-negative-impls.rs:66:13
|
LL | fn is_sync<T: Sync>(_: T) {}
| ------- ---- required by this bound in `is_sync`
...
LL | is_sync(Outer2(TestType));
| ^^^^^^^^^^^^^^^^ `main::TestType` cannot be sent between threads safely
| ^^^^^^^^^^^^^^^^
| |
| `main::TestType` cannot be sent between threads safely
| help: consider borrowing here: `&Outer2(TestType)`
|
= help: the trait `std::marker::Send` is not implemented for `main::TestType`
= note: required because of the requirements on the impl of `std::marker::Sync` for `Outer2<main::TestType>`
Expand Down

0 comments on commit f57413b

Please sign in to comment.