From 64b3e4af2031fb2faf3330ab01c27ae084412079 Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Sat, 20 Aug 2022 02:51:20 +0900 Subject: [PATCH 1/2] suggest adding a reference to a trait assoc item --- .../src/traits/error_reporting/suggestions.rs | 186 ++++++++++-------- ...adding-reference-to-trait-assoc-item.fixed | 15 ++ ...st-adding-reference-to-trait-assoc-item.rs | 15 ++ ...dding-reference-to-trait-assoc-item.stderr | 25 +++ 4 files changed, 156 insertions(+), 85 deletions(-) create mode 100644 src/test/ui/suggestions/suggest-adding-reference-to-trait-assoc-item.fixed create mode 100644 src/test/ui/suggestions/suggest-adding-reference-to-trait-assoc-item.rs create mode 100644 src/test/ui/suggestions/suggest-adding-reference-to-trait-assoc-item.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 219413121d812..28b3d886b50b5 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -882,6 +882,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { obligation.cause.code() { &parent_code + } else if let ObligationCauseCode::ItemObligation(_) = obligation.cause.code() { + obligation.cause.code() } else if let ExpnKind::Desugaring(DesugaringKind::ForLoop) = span.ctxt().outer_expn_data().kind { @@ -906,102 +908,116 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { let param_env = obligation.param_env; // Try to apply the original trait binding obligation by borrowing. - let mut try_borrowing = - |old_pred: ty::PolyTraitPredicate<'tcx>, blacklist: &[DefId]| -> bool { - if blacklist.contains(&old_pred.def_id()) { - return false; - } - // We map bounds to `&T` and `&mut T` - let trait_pred_and_imm_ref = old_pred.map_bound(|trait_pred| { - ( - trait_pred, - self.tcx.mk_imm_ref(self.tcx.lifetimes.re_static, trait_pred.self_ty()), - ) - }); - let trait_pred_and_mut_ref = old_pred.map_bound(|trait_pred| { - ( - trait_pred, - self.tcx.mk_mut_ref(self.tcx.lifetimes.re_static, trait_pred.self_ty()), - ) - }); + let mut try_borrowing = |old_pred: ty::PolyTraitPredicate<'tcx>, + blacklist: &[DefId]| + -> bool { + if blacklist.contains(&old_pred.def_id()) { + return false; + } + // We map bounds to `&T` and `&mut T` + let trait_pred_and_imm_ref = old_pred.map_bound(|trait_pred| { + ( + trait_pred, + self.tcx.mk_imm_ref(self.tcx.lifetimes.re_static, trait_pred.self_ty()), + ) + }); + let trait_pred_and_mut_ref = old_pred.map_bound(|trait_pred| { + ( + trait_pred, + self.tcx.mk_mut_ref(self.tcx.lifetimes.re_static, trait_pred.self_ty()), + ) + }); - let mk_result = |trait_pred_and_new_ty| { - let obligation = - self.mk_trait_obligation_with_new_self_ty(param_env, trait_pred_and_new_ty); - self.predicate_must_hold_modulo_regions(&obligation) + let mk_result = |trait_pred_and_new_ty| { + let obligation = + self.mk_trait_obligation_with_new_self_ty(param_env, trait_pred_and_new_ty); + self.predicate_must_hold_modulo_regions(&obligation) + }; + let imm_result = mk_result(trait_pred_and_imm_ref); + let mut_result = mk_result(trait_pred_and_mut_ref); + + let ref_inner_ty_result = + if let ObligationCauseCode::ItemObligation(_) = obligation.cause.code() + && let ty::Ref(_, ty, mutability) = old_pred.self_ty().skip_binder().kind() + { + Some((mk_result(old_pred.map_bound(|trait_pred| (trait_pred, *ty))), mutability)) + } else { + None }; - let imm_result = mk_result(trait_pred_and_imm_ref); - let mut_result = mk_result(trait_pred_and_mut_ref); - - if imm_result || mut_result { - 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. - - let msg = format!("the trait bound `{}` is not satisfied", old_pred); - if has_custom_message { - err.note(&msg); - } else { - err.message = - vec![(rustc_errors::DiagnosticMessage::Str(msg), 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_label( - span, - &format!( - "expected an implementor of trait `{}`", - old_pred.print_modifiers_and_trait_path(), - ), - ); - // This if is to prevent a special edge-case - if matches!( - span.ctxt().outer_expn_data().kind, - ExpnKind::Root | ExpnKind::Desugaring(DesugaringKind::ForLoop) - ) { - // We don't want a borrowing suggestion on the fields in structs, - // ``` - // struct Foo { - // the_foos: Vec - // } - // ``` + if imm_result || mut_result || ref_inner_ty_result.map_or(false, |(result, _)| result) { + 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. + + let msg = format!("the trait bound `{}` is not satisfied", old_pred); + if has_custom_message { + err.note(&msg); + } else { + err.message = + vec![(rustc_errors::DiagnosticMessage::Str(msg), 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_label( + span, + &format!( + "expected an implementor of trait `{}`", + old_pred.print_modifiers_and_trait_path(), + ), + ); - if imm_result && mut_result { - err.span_suggestions( - span.shrink_to_lo(), - "consider borrowing here", - ["&".to_string(), "&mut ".to_string()].into_iter(), - Applicability::MaybeIncorrect, - ); - } else { - err.span_suggestion_verbose( - span.shrink_to_lo(), - &format!( - "consider{} borrowing here", - if mut_result { " mutably" } else { "" } - ), - format!("&{}", if mut_result { "mut " } else { "" }), - Applicability::MaybeIncorrect, - ); - } + // This if is to prevent a special edge-case + if matches!( + span.ctxt().outer_expn_data().kind, + ExpnKind::Root | ExpnKind::Desugaring(DesugaringKind::ForLoop) + ) { + // We don't want a borrowing suggestion on the fields in structs, + // ``` + // struct Foo { + // the_foos: Vec + // } + // ``` + + if imm_result && mut_result { + err.span_suggestions( + span.shrink_to_lo(), + "consider borrowing here", + ["&".to_string(), "&mut ".to_string()].into_iter(), + Applicability::MaybeIncorrect, + ); + } else { + let is_mut = mut_result + || ref_inner_ty_result.map_or(false, |(_, mutabl)| { + matches!(mutabl, hir::Mutability::Mut) + }); + err.span_suggestion_verbose( + span.shrink_to_lo(), + &format!( + "consider{} borrowing here", + if is_mut { " mutably" } else { "" } + ), + format!("&{}", if is_mut { "mut " } else { "" }), + Applicability::MaybeIncorrect, + ); } - return true; } + return true; } - return false; - }; + } + return false; + }; if let ObligationCauseCode::ImplDerivedObligation(cause) = &*code { try_borrowing(cause.derived.parent_trait_pred, &[]) } else if let ObligationCauseCode::BindingObligation(_, _) - | ObligationCauseCode::ItemObligation(_) = code + | ObligationCauseCode::ItemObligation(..) = code { try_borrowing(poly_trait_pred, &never_suggest_borrow) } else { diff --git a/src/test/ui/suggestions/suggest-adding-reference-to-trait-assoc-item.fixed b/src/test/ui/suggestions/suggest-adding-reference-to-trait-assoc-item.fixed new file mode 100644 index 0000000000000..e9b8a9caa484a --- /dev/null +++ b/src/test/ui/suggestions/suggest-adding-reference-to-trait-assoc-item.fixed @@ -0,0 +1,15 @@ +// run-rustfix +#![allow(unused_variables)] + +fn foo(foo: &mut usize) { + todo!() +} + +fn bar(bar: &usize) { + todo!() +} + +fn main() { + foo(&mut Default::default()); //~ the trait bound `&mut usize: Default` is not satisfied + bar(&Default::default()); //~ the trait bound `&usize: Default` is not satisfied +} diff --git a/src/test/ui/suggestions/suggest-adding-reference-to-trait-assoc-item.rs b/src/test/ui/suggestions/suggest-adding-reference-to-trait-assoc-item.rs new file mode 100644 index 0000000000000..5fae21cccef23 --- /dev/null +++ b/src/test/ui/suggestions/suggest-adding-reference-to-trait-assoc-item.rs @@ -0,0 +1,15 @@ +// run-rustfix +#![allow(unused_variables)] + +fn foo(foo: &mut usize) { + todo!() +} + +fn bar(bar: &usize) { + todo!() +} + +fn main() { + foo(Default::default()); //~ the trait bound `&mut usize: Default` is not satisfied + bar(Default::default()); //~ the trait bound `&usize: Default` is not satisfied +} diff --git a/src/test/ui/suggestions/suggest-adding-reference-to-trait-assoc-item.stderr b/src/test/ui/suggestions/suggest-adding-reference-to-trait-assoc-item.stderr new file mode 100644 index 0000000000000..b930d22a3915e --- /dev/null +++ b/src/test/ui/suggestions/suggest-adding-reference-to-trait-assoc-item.stderr @@ -0,0 +1,25 @@ +error[E0277]: the trait bound `&mut usize: Default` is not satisfied + --> $DIR/suggest-adding-reference-to-trait-assoc-item.rs:13:9 + | +LL | foo(Default::default()); + | ^^^^^^^^^^^^^^^^ expected an implementor of trait `Default` + | +help: consider mutably borrowing here + | +LL | foo(&mut Default::default()); + | ++++ + +error[E0277]: the trait bound `&usize: Default` is not satisfied + --> $DIR/suggest-adding-reference-to-trait-assoc-item.rs:14:9 + | +LL | bar(Default::default()); + | ^^^^^^^^^^^^^^^^ expected an implementor of trait `Default` + | +help: consider borrowing here + | +LL | bar(&Default::default()); + | + + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0277`. From a311b8a4c5ed3dfbf80a94c48768fda2e6785e52 Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Sat, 20 Aug 2022 19:35:17 +0900 Subject: [PATCH 2/2] use more descriptive names --- .../src/traits/error_reporting/suggestions.rs | 188 +++++++++--------- 1 file changed, 95 insertions(+), 93 deletions(-) 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 28b3d886b50b5..ffc4f8cf3d3b3 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -908,111 +908,113 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { let param_env = obligation.param_env; // Try to apply the original trait binding obligation by borrowing. - let mut try_borrowing = |old_pred: ty::PolyTraitPredicate<'tcx>, - blacklist: &[DefId]| - -> bool { - if blacklist.contains(&old_pred.def_id()) { - return false; - } - // We map bounds to `&T` and `&mut T` - let trait_pred_and_imm_ref = old_pred.map_bound(|trait_pred| { - ( - trait_pred, - self.tcx.mk_imm_ref(self.tcx.lifetimes.re_static, trait_pred.self_ty()), - ) - }); - let trait_pred_and_mut_ref = old_pred.map_bound(|trait_pred| { - ( - trait_pred, - self.tcx.mk_mut_ref(self.tcx.lifetimes.re_static, trait_pred.self_ty()), - ) - }); + let mut try_borrowing = + |old_pred: ty::PolyTraitPredicate<'tcx>, blacklist: &[DefId]| -> bool { + if blacklist.contains(&old_pred.def_id()) { + return false; + } + // We map bounds to `&T` and `&mut T` + let trait_pred_and_imm_ref = old_pred.map_bound(|trait_pred| { + ( + trait_pred, + self.tcx.mk_imm_ref(self.tcx.lifetimes.re_static, trait_pred.self_ty()), + ) + }); + let trait_pred_and_mut_ref = old_pred.map_bound(|trait_pred| { + ( + trait_pred, + self.tcx.mk_mut_ref(self.tcx.lifetimes.re_static, trait_pred.self_ty()), + ) + }); - let mk_result = |trait_pred_and_new_ty| { - let obligation = - self.mk_trait_obligation_with_new_self_ty(param_env, trait_pred_and_new_ty); - self.predicate_must_hold_modulo_regions(&obligation) - }; - let imm_result = mk_result(trait_pred_and_imm_ref); - let mut_result = mk_result(trait_pred_and_mut_ref); + let mk_result = |trait_pred_and_new_ty| { + let obligation = + self.mk_trait_obligation_with_new_self_ty(param_env, trait_pred_and_new_ty); + self.predicate_must_hold_modulo_regions(&obligation) + }; + let imm_ref_self_ty_satisfies_pred = mk_result(trait_pred_and_imm_ref); + let mut_ref_self_ty_satisfies_pred = mk_result(trait_pred_and_mut_ref); - let ref_inner_ty_result = + let (ref_inner_ty_satisfies_pred, ref_inner_ty_mut) = if let ObligationCauseCode::ItemObligation(_) = obligation.cause.code() && let ty::Ref(_, ty, mutability) = old_pred.self_ty().skip_binder().kind() { - Some((mk_result(old_pred.map_bound(|trait_pred| (trait_pred, *ty))), mutability)) + ( + mk_result(old_pred.map_bound(|trait_pred| (trait_pred, *ty))), + matches!(mutability, hir::Mutability::Mut), + ) } else { - None + (false, false) }; - if imm_result || mut_result || ref_inner_ty_result.map_or(false, |(result, _)| result) { - 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. - - let msg = format!("the trait bound `{}` is not satisfied", old_pred); - if has_custom_message { - err.note(&msg); - } else { - err.message = - vec![(rustc_errors::DiagnosticMessage::Str(msg), 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_label( - span, - &format!( - "expected an implementor of trait `{}`", - old_pred.print_modifiers_and_trait_path(), - ), - ); - - // This if is to prevent a special edge-case - if matches!( - span.ctxt().outer_expn_data().kind, - ExpnKind::Root | ExpnKind::Desugaring(DesugaringKind::ForLoop) - ) { - // We don't want a borrowing suggestion on the fields in structs, - // ``` - // struct Foo { - // the_foos: Vec - // } - // ``` - - if imm_result && mut_result { - err.span_suggestions( - span.shrink_to_lo(), - "consider borrowing here", - ["&".to_string(), "&mut ".to_string()].into_iter(), - Applicability::MaybeIncorrect, - ); + if imm_ref_self_ty_satisfies_pred + || mut_ref_self_ty_satisfies_pred + || ref_inner_ty_satisfies_pred + { + 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. + + let msg = format!("the trait bound `{}` is not satisfied", old_pred); + if has_custom_message { + err.note(&msg); } else { - let is_mut = mut_result - || ref_inner_ty_result.map_or(false, |(_, mutabl)| { - matches!(mutabl, hir::Mutability::Mut) - }); - err.span_suggestion_verbose( - span.shrink_to_lo(), - &format!( - "consider{} borrowing here", - if is_mut { " mutably" } else { "" } - ), - format!("&{}", if is_mut { "mut " } else { "" }), - Applicability::MaybeIncorrect, - ); + err.message = + vec![(rustc_errors::DiagnosticMessage::Str(msg), 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_label( + span, + &format!( + "expected an implementor of trait `{}`", + old_pred.print_modifiers_and_trait_path(), + ), + ); + + // This if is to prevent a special edge-case + if matches!( + span.ctxt().outer_expn_data().kind, + ExpnKind::Root | ExpnKind::Desugaring(DesugaringKind::ForLoop) + ) { + // We don't want a borrowing suggestion on the fields in structs, + // ``` + // struct Foo { + // the_foos: Vec + // } + // ``` + + if imm_ref_self_ty_satisfies_pred && mut_ref_self_ty_satisfies_pred { + err.span_suggestions( + span.shrink_to_lo(), + "consider borrowing here", + ["&".to_string(), "&mut ".to_string()].into_iter(), + Applicability::MaybeIncorrect, + ); + } else { + let is_mut = mut_ref_self_ty_satisfies_pred || ref_inner_ty_mut; + err.span_suggestion_verbose( + span.shrink_to_lo(), + &format!( + "consider{} borrowing here", + if is_mut { " mutably" } else { "" } + ), + format!("&{}", if is_mut { "mut " } else { "" }), + Applicability::MaybeIncorrect, + ); + } + } + return true; } - return true; } - } - return false; - }; + return false; + }; if let ObligationCauseCode::ImplDerivedObligation(cause) = &*code { try_borrowing(cause.derived.parent_trait_pred, &[])