bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Rollup merge of #85369 - FabianWolff:issue-84973, r=jackh726

Browse source 
Suggest borrowing if a trait implementation is found for &/&mut <type>

This pull request fixes #84973 by suggesting to borrow if a trait is not implemented for some type `T`, but it is for `&T` or `&mut T`. For instance:
```rust
trait Ti {}
impl<T> Ti for &T {}
fn foo<T: Ti>(_: T) {}

trait Tm {}
impl<T> Tm for &mut T {}
fn bar<T: Tm>(_: T) {}

fn main() {
    let a: i32 = 5;
    foo(a);

    let b: Box<i32> = Box::new(42);
    bar(b);
}
```
gives, on current nightly:
```
error[E0277]: the trait bound `i32: Ti` is not satisfied
  --> t2.rs:11:9
   |
3  | fn foo<T: Ti>(_: T) {}
   |           -- required by this bound in `foo`
...
11 |     foo(a);
   |         ^ the trait `Ti` is not implemented for `i32`

error[E0277]: the trait bound `Box<i32>: Tm` is not satisfied
  --> t2.rs:14:9
   |
7  | fn bar<T: Tm>(_: T) {}
   |           -- required by this bound in `bar`
...
14 |     bar(b);
   |         ^ the trait `Tm` is not implemented for `Box<i32>`

error: aborting due to 2 previous errors
```
whereas with my changes, I get:
```
error[E0277]: the trait bound `i32: Ti` is not satisfied
  --> t2.rs:11:9
   |
3  | fn foo<T: Ti>(_: T) {}
   |           -- required by this bound in `foo`
...
11 |     foo(a);
   |         ^
   |         |
   |         expected an implementor of trait `Ti`
   |         help: consider borrowing here: `&a`

error[E0277]: the trait bound `Box<i32>: Tm` is not satisfied
  --> t2.rs:14:9
   |
7  | fn bar<T: Tm>(_: T) {}
   |           -- required by this bound in `bar`
...
14 |     bar(b);
   |         ^
   |         |
   |         expected an implementor of trait `Tm`
   |         help: consider borrowing mutably here: `&mut b`

error: aborting due to 2 previous errors
```
In my implementation, I have added a "blacklist" to make these suggestions flexible. In particular, suggesting to borrow can interfere with other suggestions, such as to add another trait bound to a generic argument. I have tried to configure this blacklist to cause the least amount of test case failures, i.e. to model the current behavior as closely as possible (I only had to change one existing test case, and this change was quite clearly an improvement).
This commit is contained in:
Guillaume Gomez 2021-05-18 14:08:55 +02:00 • committed by GitHub
commit fad04ff60e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 277 additions and 17 deletions
Download patch file Download diff file Expand all files Collapse all files

81
compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
View file

@ -686,17 +686,36 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
return false;
}
// Blacklist traits for which it would be nonsensical to suggest borrowing.
// For instance, immutable references are always Copy, so suggesting to
// borrow would always succeed, but it's probably not what the user wanted.
let blacklist: Vec<_> =
[LangItem::Copy, LangItem::Clone, LangItem::Unpin, LangItem::Sized, LangItem::Send]
.iter()
.filter_map(|lang_item| self.tcx.lang_items().require(*lang_item).ok())
.collect();
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 found_ty = trait_ref.self_ty();
let found_ty_str = found_ty.to_string();
let imm_borrowed_found_ty = self.tcx.mk_imm_ref(self.tcx.lifetimes.re_static, found_ty);
let imm_substs = self.tcx.mk_substs_trait(imm_borrowed_found_ty, &[]);
let mut_borrowed_found_ty = self.tcx.mk_mut_ref(self.tcx.lifetimes.re_static, found_ty);
let mut_substs = self.tcx.mk_substs_trait(mut_borrowed_found_ty, &[]);
// Try to apply the original trait binding obligation by borrowing.
let mut try_borrowing = |new_trait_ref: ty::TraitRef<'tcx>,
expected_trait_ref: ty::TraitRef<'tcx>,
mtbl: bool,
blacklist: &[DefId]|
-> bool {
if blacklist.contains(&expected_trait_ref.def_id) {
return false;
}
let new_obligation = Obligation::new(
ObligationCause::dummy(),
param_env,
@ -713,8 +732,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
let msg = format!(
"the trait bound `{}: {}` is not satisfied",
found,
obligation.parent_trait_ref.skip_binder().print_only_trait_path(),
found_ty_str,
expected_trait_ref.print_only_trait_path(),
);
if has_custom_message {
err.note(&msg);
@ -730,7 +749,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
span,
&format!(
"expected an implementor of trait `{}`",
obligation.parent_trait_ref.skip_binder().print_only_trait_path(),
expected_trait_ref.print_only_trait_path(),
),
);
@ -745,16 +764,52 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
err.span_suggestion(
span,
"consider borrowing here",
format!("&{}", snippet),
&format!(
"consider{} borrowing here",
if mtbl { " mutably" } else { "" }
),
format!("&{}{}", if mtbl { "mut " } else { "" }, snippet),
Applicability::MaybeIncorrect,
);
}
return true;
}
}
return false;
};
if let ObligationCauseCode::ImplDerivedObligation(obligation) = &obligation.cause.code {
let expected_trait_ref = obligation.parent_trait_ref.skip_binder();
let new_imm_trait_ref =
ty::TraitRef::new(obligation.parent_trait_ref.def_id(), imm_substs);
let new_mut_trait_ref =
ty::TraitRef::new(obligation.parent_trait_ref.def_id(), mut_substs);
if try_borrowing(new_imm_trait_ref, expected_trait_ref, false, &[]) {
return true;
} else {
return try_borrowing(new_mut_trait_ref, expected_trait_ref, true, &[]);
}
} else if let ObligationCauseCode::BindingObligation(_, _)
| ObligationCauseCode::ItemObligation(_) = &obligation.cause.code
{
if try_borrowing(
ty::TraitRef::new(trait_ref.def_id, imm_substs),
trait_ref,
false,
&blacklist[..],
) {
return true;
} else {
return try_borrowing(
ty::TraitRef::new(trait_ref.def_id, mut_substs),
trait_ref,
true,
&blacklist[..],
);
}
} else {
false
}
false
}
/// Whenever references are used by mistake, like `for (i, e) in &vec.iter().enumerate()`,