Skip to content

Commit bc5c988

Browse files
estebankestebank
committed
Suggest borrowing when it would satisfy an unmet trait bound
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.
1 parent c217711 commit bc5c988

10 files changed

+107
-24
lines changed

‎src/libcore/ops/function.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -137,10 +137,6 @@ pub trait Fn<Args> : FnMut<Args> {
137137
#[rustc_paren_sugar]
138138
#[rustc_on_unimplemented(
139139
on(Args="()", note="wrap the `{Self}` in a closure with no arguments: `|| {{ /* code */ }}"),
140-
on(
141-
all(Args="(char,)", _Self="std::string::String"),
142-
note="borrowing the `{Self}` might fix the problem"
143-
),
144140
message="expected a `{FnMut}<{Args}>` closure, found `{Self}`",
145141
label="expected an `FnMut<{Args}>` closure, found `{Self}`",
146142
)]

‎src/librustc/traits/error_reporting.rs

Lines changed: 72 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ use crate::ty::subst::Subst;
3333
use crate::ty::SubtypePredicate;
3434
use crate::util::nodemap::{FxHashMap, FxHashSet};
3535

36-
use errors::{Applicability, DiagnosticBuilder, pluralize};
36+
use errors::{Applicability, DiagnosticBuilder, pluralise, Style};
3737
use std::fmt;
3838
use syntax::ast;
3939
use syntax::symbol::{sym, kw};
@@ -721,7 +721,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
721721
post_message,
722722
pre_message,
723723
) = self.get_parent_trait_ref(&obligation.cause.code)
724-
.map(|t| (format!(" in `{}`", t), format!("within `{}`, ", t)))
724+
.map(|t| (format!(" in `{}`", t), format!("within `{}`, ", t)))
725725
.unwrap_or_default();
726726

727727
let OnUnimplementedNote {
@@ -785,6 +785,16 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
785785
self.suggest_borrow_on_unsized_slice(&obligation.cause.code, &mut err);
786786
self.suggest_fn_call(&obligation, &mut err, &trait_ref, points_at_arg);
787787
self.suggest_remove_reference(&obligation, &mut err, &trait_ref);
788+
if self.suggest_add_reference_to_arg(
789+
&obligation,
790+
&mut err,
791+
&trait_ref,
792+
points_at_arg,
793+
) {
794+
self.note_obligation_cause(&mut err, obligation);
795+
err.emit();
796+
return;
797+
}
788798
self.suggest_semicolon_removal(&obligation, &mut err, span, &trait_ref);
789799

790800
// Try to report a help message
@@ -1300,6 +1310,66 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
13001310
}
13011311
}
13021312

1313+
fn suggest_add_reference_to_arg(
1314+
&self,
1315+
obligation: &PredicateObligation<'tcx>,
1316+
err: &mut DiagnosticBuilder<'tcx>,
1317+
trait_ref: &ty::Binder<ty::TraitRef<'tcx>>,
1318+
points_at_arg: bool,
1319+
) -> bool {
1320+
if !points_at_arg {
1321+
return false;
1322+
}
1323+
1324+
let span = obligation.cause.span;
1325+
let param_env = obligation.param_env;
1326+
let trait_ref = trait_ref.skip_binder();
1327+
1328+
if let ObligationCauseCode::ImplDerivedObligation(obligation) = &obligation.cause.code {
1329+
// Try to apply the original trait binding obligation by borrowing.
1330+
let self_ty = trait_ref.self_ty();
1331+
let found = self_ty.to_string();
1332+
let new_self_ty = self.tcx.mk_imm_ref(self.tcx.lifetimes.re_static, self_ty);
1333+
let substs = self.tcx.mk_substs_trait(new_self_ty, &[]);
1334+
let new_trait_ref = ty::TraitRef::new(obligation.parent_trait_ref.def_id(), substs);
1335+
let new_obligation = Obligation::new(
1336+
ObligationCause::dummy(),
1337+
param_env,
1338+
new_trait_ref.to_predicate(),
1339+
);
1340+
if self.predicate_may_hold(&new_obligation) {
1341+
if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) {
1342+
// We have a very specific type of error, where just borrowing this argument
1343+
// might solve the problem. In cases like this, the important part is the
1344+
// original type obligation, not the last one that failed, which is arbitrary.
1345+
// Because of this, we modify the error to refer to the original obligation and
1346+
// return early in the caller.
1347+
err.message = vec![(
1348+
format!(
1349+
"the trait bound `{}: {}` is not satisfied",
1350+
found,
1351+
obligation.parent_trait_ref.skip_binder(),
1352+
),
1353+
Style::NoStyle,
1354+
)];
1355+
if snippet.starts_with('&') {
1356+
// This is already a literal borrow and the obligation is failing
1357+
// somewhere else in the obligation chain. Do not suggest non-sense.
1358+
return false;
1359+
}
1360+
err.span_suggestion(
1361+
span,
1362+
"consider borrowing here",
1363+
format!("&{}", snippet),
1364+
Applicability::MachineApplicable,
1365+
);
1366+
return true;
1367+
}
1368+
}
1369+
}
1370+
false
1371+
}
1372+
13031373
/// Whenever references are used by mistake, like `for (i, e) in &vec.iter().enumerate()`,
13041374
/// suggest removing these references until we reach a type that implements the trait.
13051375
fn suggest_remove_reference(

‎src/test/ui/derives/deriving-copyclone.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@ fn main() {
2828
is_clone(B { a: 1, b: 2 });
2929

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

3434
// B<D> can be cloned but not copied
35-
is_copy(B { a: 1, b: D }); //~ERROR Copy
35+
is_copy(B { a: 1, b: D }); //~ ERROR Copy
3636
is_clone(B { a: 1, b: D });
3737
}

‎src/test/ui/derives/deriving-copyclone.stderr

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@ LL | fn is_copy<T: Copy>(_: T) {}
55
| ------- ---- required by this bound in `is_copy`
66
...
77
LL | is_copy(B { a: 1, b: C });
8-
| ^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `C`
8+
| ^^^^^^^^^^^^^^^^
9+
| |
10+
| the trait `std::marker::Copy` is not implemented for `C`
11+
| help: consider borrowing here: `&B { a: 1, b: C }`
912
|
1013
= note: required because of the requirements on the impl of `std::marker::Copy` for `B<C>`
1114

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

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

‎src/test/ui/kindck/kindck-impl-type-params-2.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,5 @@ fn take_param<T:Foo>(foo: &T) { }
1111
fn main() {
1212
let x: Box<_> = box 3;
1313
take_param(&x);
14-
//~^ ERROR `std::boxed::Box<{integer}>: std::marker::Copy` is not satisfied
14+
//~^ ERROR the trait bound `std::boxed::Box<{integer}>: Foo` is not satisfied
1515
}

‎src/test/ui/kindck/kindck-impl-type-params-2.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
error[E0277]: the trait bound `std::boxed::Box<{integer}>: std::marker::Copy` is not satisfied
1+
error[E0277]: the trait bound `std::boxed::Box<{integer}>: Foo` is not satisfied
22
--> $DIR/kindck-impl-type-params-2.rs:13:16
33
|
44
LL | fn take_param<T:Foo>(foo: &T) { }

‎src/test/ui/kindck/kindck-inherited-copy-bound.curr.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
error[E0277]: the trait bound `std::boxed::Box<{integer}>: std::marker::Copy` is not satisfied
1+
error[E0277]: the trait bound `std::boxed::Box<{integer}>: Foo` is not satisfied
22
--> $DIR/kindck-inherited-copy-bound.rs:21:16
33
|
44
LL | fn take_param<T:Foo>(foo: &T) { }

‎src/test/ui/suggestions/issue-62843.stderr

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1-
error[E0277]: expected a `std::ops::FnMut<(char,)>` closure, found `std::string::String`
1+
error[E0277]: the trait bound `std::string::String: std::str::pattern::Pattern<'_>` is not satisfied
22
--> $DIR/issue-62843.rs:4:32
33
|
44
LL | println!("{:?}", line.find(pattern));
5-
| ^^^^^^^ expected an `FnMut<(char,)>` closure, found `std::string::String`
5+
| ^^^^^^^
6+
| |
7+
| expected an `FnMut<(char,)>` closure, found `std::string::String`
8+
| help: consider borrowing here: `&pattern`
69
|
710
= help: the trait `std::ops::FnMut<(char,)>` is not implemented for `std::string::String`
8-
= note: borrowing the `std::string::String` might fix the problem
911
= note: required because of the requirements on the impl of `std::str::pattern::Pattern<'_>` for `std::string::String`
1012

1113
error: aborting due to previous error

‎src/test/ui/traits/traits-negative-impls.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ fn dummy2() {
4646
impl !Send for TestType {}
4747

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

5252
fn dummy3() {
@@ -64,5 +64,5 @@ fn main() {
6464
// This will complain about a missing Send impl because `Sync` is implement *just*
6565
// for T that are `Send`. Look at #20366 and #19950
6666
is_sync(Outer2(TestType));
67-
//~^ ERROR `main::TestType` cannot be sent between threads safely
67+
//~^ ERROR the trait bound `main::TestType: std::marker::Sync` is not satisfied
6868
}

‎src/test/ui/traits/traits-negative-impls.stderr

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,17 @@ LL | is_send((8, TestType));
4343
= help: within `({integer}, dummy1c::TestType)`, the trait `std::marker::Send` is not implemented for `dummy1c::TestType`
4444
= note: required because it appears within the type `({integer}, dummy1c::TestType)`
4545

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

73-
error[E0277]: `main::TestType` cannot be sent between threads safely
76+
error[E0277]: the trait bound `main::TestType: std::marker::Sync` is not satisfied
7477
--> $DIR/traits-negative-impls.rs:66:13
7578
|
7679
LL | fn is_sync<T: Sync>(_: T) {}
7780
| ------- ---- required by this bound in `is_sync`
7881
...
7982
LL | is_sync(Outer2(TestType));
80-
| ^^^^^^^^^^^^^^^^ `main::TestType` cannot be sent between threads safely
83+
| ^^^^^^^^^^^^^^^^
84+
| |
85+
| `main::TestType` cannot be sent between threads safely
86+
| help: consider borrowing here: `&Outer2(TestType)`
8187
|
8288
= help: the trait `std::marker::Send` is not implemented for `main::TestType`
8389
= note: required because of the requirements on the impl of `std::marker::Sync` for `Outer2<main::TestType>`

0 commit comments

Comments
 (0)