From 48d07d13266d321a2be15a18d335cf642dbd685c Mon Sep 17 00:00:00 2001 From: Fabian Wolff Date: Sun, 16 May 2021 16:20:35 +0200 Subject: [PATCH] Suggest borrowing if a trait implementation is found for &/&mut --- .../src/traits/error_reporting/suggestions.rs | 87 ++++++++++++++++--- .../imm-ref-trait-object-literal.stderr | 8 +- src/test/ui/suggestions/issue-84973-2.rs | 13 +++ src/test/ui/suggestions/issue-84973-2.stderr | 15 ++++ src/test/ui/suggestions/issue-84973.rs | 33 +++++++ src/test/ui/suggestions/issue-84973.stderr | 15 ++++ 6 files changed, 154 insertions(+), 17 deletions(-) create mode 100644 src/test/ui/suggestions/issue-84973-2.rs create mode 100644 src/test/ui/suggestions/issue-84973-2.stderr create mode 100644 src/test/ui/suggestions/issue-84973.rs create mode 100644 src/test/ui/suggestions/issue-84973.stderr diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index 6a4d41ffc1a..d0901204061 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -686,17 +686,42 @@ 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::Pin, + 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 +738,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 +755,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 +770,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()`, diff --git a/src/test/ui/suggestions/imm-ref-trait-object-literal.stderr b/src/test/ui/suggestions/imm-ref-trait-object-literal.stderr index df483b3912d..3cf74410569 100644 --- a/src/test/ui/suggestions/imm-ref-trait-object-literal.stderr +++ b/src/test/ui/suggestions/imm-ref-trait-object-literal.stderr @@ -21,10 +21,10 @@ LL | fn foo(_: X) {} | ----- required by this bound in `foo` ... LL | foo(s); - | ^ the trait `Trait` is not implemented for `S` - | - = help: the following implementations were found: - <&'a mut S as Trait> + | ^ + | | + | expected an implementor of trait `Trait` + | help: consider borrowing mutably here: `&mut s` error: aborting due to 2 previous errors diff --git a/src/test/ui/suggestions/issue-84973-2.rs b/src/test/ui/suggestions/issue-84973-2.rs new file mode 100644 index 00000000000..050cf8c64b3 --- /dev/null +++ b/src/test/ui/suggestions/issue-84973-2.rs @@ -0,0 +1,13 @@ +// A slight variation of issue-84973.rs. Here, a mutable borrow is +// required (and the obligation kind is different). + +trait Tr {} +impl Tr for &mut i32 {} + +fn foo(i: T) {} + +fn main() { + let a: i32 = 32; + foo(a); + //~^ ERROR: the trait bound `i32: Tr` is not satisfied [E0277] +} diff --git a/src/test/ui/suggestions/issue-84973-2.stderr b/src/test/ui/suggestions/issue-84973-2.stderr new file mode 100644 index 00000000000..6394e58a2a9 --- /dev/null +++ b/src/test/ui/suggestions/issue-84973-2.stderr @@ -0,0 +1,15 @@ +error[E0277]: the trait bound `i32: Tr` is not satisfied + --> $DIR/issue-84973-2.rs:11:9 + | +LL | fn foo(i: T) {} + | -- required by this bound in `foo` +... +LL | foo(a); + | ^ + | | + | expected an implementor of trait `Tr` + | help: consider borrowing mutably here: `&mut a` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`. diff --git a/src/test/ui/suggestions/issue-84973.rs b/src/test/ui/suggestions/issue-84973.rs new file mode 100644 index 00000000000..42468478ed9 --- /dev/null +++ b/src/test/ui/suggestions/issue-84973.rs @@ -0,0 +1,33 @@ +// Checks whether borrowing is suggested when a trait bound is not satisfied +// for found type `T`, but is for `&/&mut T`. + +fn main() { + let f = Fancy{}; + let o = Other::new(f); + //~^ ERROR: the trait bound `Fancy: SomeTrait` is not satisfied [E0277] +} + +struct Fancy {} + +impl <'a> SomeTrait for &'a Fancy { +} + +trait SomeTrait {} + +struct Other<'a, G> { + a: &'a str, + g: G, +} + +// Broadly copied from https://docs.rs/petgraph/0.5.1/src/petgraph/dot.rs.html#70 +impl<'a, G> Other<'a, G> +where + G: SomeTrait, +{ + pub fn new(g: G) -> Self { + Other { + a: "hi", + g: g, + } + } +} diff --git a/src/test/ui/suggestions/issue-84973.stderr b/src/test/ui/suggestions/issue-84973.stderr new file mode 100644 index 00000000000..49fa94da859 --- /dev/null +++ b/src/test/ui/suggestions/issue-84973.stderr @@ -0,0 +1,15 @@ +error[E0277]: the trait bound `Fancy: SomeTrait` is not satisfied + --> $DIR/issue-84973.rs:6:24 + | +LL | let o = Other::new(f); + | ^ + | | + | expected an implementor of trait `SomeTrait` + | help: consider borrowing here: `&f` +... +LL | pub fn new(g: G) -> Self { + | ------------------------ required by `Other::<'a, G>::new` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`.