From fb9d98c8fc60789085de49b4a683337fd1cffd3c Mon Sep 17 00:00:00 2001 From: akida31 Date: Sat, 8 Oct 2022 15:48:28 +0200 Subject: [PATCH 1/6] Improve diagnostic when passing arg to closure and missing borrow. This checks the number of references for the given and expected type and shows hints to the user if the numbers don't match. --- .../src/traits/error_reporting/mod.rs | 2 + .../src/traits/error_reporting/suggestions.rs | 71 +++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs index 4e8baa2dfab6c..78acbd44ee20b 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -1203,6 +1203,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { _ => None, }; + let found_node = found_did.and_then(|did| self.tcx.hir().get_if_local(did)); let found_span = found_did.and_then(|did| self.tcx.hir().span_if_local(did)); if self.reported_closure_mismatch.borrow().contains(&(span, found_span)) { @@ -1256,6 +1257,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { found_trait_ref, expected_trait_ref, obligation.cause.code(), + found_node, ) } else { let (closure_span, found) = found_did 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 fda6a2236b195..46e543c371c97 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -255,6 +255,7 @@ pub trait TypeErrCtxtExt<'tcx> { found: ty::PolyTraitRef<'tcx>, expected: ty::PolyTraitRef<'tcx>, cause: &ObligationCauseCode<'tcx>, + found_node: Option>, ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed>; fn note_conflicting_closure_bounds( @@ -1592,6 +1593,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { found: ty::PolyTraitRef<'tcx>, expected: ty::PolyTraitRef<'tcx>, cause: &ObligationCauseCode<'tcx>, + found_node: Option>, ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { pub(crate) fn build_fn_sig_ty<'tcx>( infcx: &InferCtxt<'tcx>, @@ -1655,6 +1657,75 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { self.note_conflicting_closure_bounds(cause, &mut err); + let found_args = match found.kind() { + ty::FnPtr(f) => f.inputs().skip_binder().iter(), + kind => { + span_bug!(span, "found was converted to a FnPtr above but is now {:?}", kind) + } + }; + let expected_args = match expected.kind() { + ty::FnPtr(f) => f.inputs().skip_binder().iter(), + kind => { + span_bug!(span, "expected was converted to a FnPtr above but is now {:?}", kind) + } + }; + + if let Some(found_node) = found_node { + let fn_decl = match found_node { + Node::Expr(expr) => match &expr.kind { + hir::ExprKind::Closure(hir::Closure { fn_decl, .. }) => fn_decl, + kind => { + span_bug!(found_span, "expression must be a closure but is {:?}", kind) + } + }, + Node::Item(item) => match &item.kind { + hir::ItemKind::Fn(signature, _generics, _body) => signature.decl, + kind => { + span_bug!(found_span, "item must be a function but is {:?}", kind) + } + }, + node => { + span_bug!(found_span, "node must be a expr or item but is {:?}", node) + } + }; + + let arg_spans = fn_decl.inputs.iter().map(|ty| ty.span); + + fn get_deref_type_and_refs(mut ty: Ty<'_>) -> (Ty<'_>, usize) { + let mut refs = 0; + + while let ty::Ref(_, new_ty, _) = ty.kind() { + ty = *new_ty; + refs += 1; + } + + (ty, refs) + } + + for ((found_arg, expected_arg), arg_span) in + found_args.zip(expected_args).zip(arg_spans) + { + let (found_ty, found_refs) = get_deref_type_and_refs(*found_arg); + let (expected_ty, expected_refs) = get_deref_type_and_refs(*expected_arg); + + if found_ty == expected_ty { + let hint = if found_refs < expected_refs { + "hint: consider borrowing here:" + } else if found_refs == expected_refs { + continue; + } else { + "hint: consider removing the borrow:" + }; + err.span_suggestion_verbose( + arg_span, + hint, + expected_arg.to_string(), + Applicability::MaybeIncorrect, + ); + } + } + } + err } From 4992e8cabb39d8d3cf816ec18f4b4a72407ef409 Mon Sep 17 00:00:00 2001 From: akida31 Date: Sat, 8 Oct 2022 15:50:35 +0200 Subject: [PATCH 2/6] Fix stderr of tests which have improved diagnostics --- .../anonymous-higher-ranked-lifetime.stderr | 72 +++++++++++++++++++ .../closure-arg-type-mismatch.stderr | 4 ++ .../ui/mismatched_types/issue-36053-2.stderr | 4 ++ 3 files changed, 80 insertions(+) diff --git a/src/test/ui/anonymous-higher-ranked-lifetime.stderr b/src/test/ui/anonymous-higher-ranked-lifetime.stderr index bf5f642ca823d..af6ae1273041a 100644 --- a/src/test/ui/anonymous-higher-ranked-lifetime.stderr +++ b/src/test/ui/anonymous-higher-ranked-lifetime.stderr @@ -13,6 +13,14 @@ note: required by a bound in `f1` | LL | fn f1(_: F) where F: Fn(&(), &()) {} | ^^^^^^^^^^^^ required by this bound in `f1` +help: hint: consider borrowing here: + | +LL | f1(|_: &(), _: ()| {}); + | ~~~ +help: hint: consider borrowing here: + | +LL | f1(|_: (), _: &()| {}); + | ~~~ error[E0631]: type mismatch in closure arguments --> $DIR/anonymous-higher-ranked-lifetime.rs:3:5 @@ -29,6 +37,14 @@ note: required by a bound in `f2` | LL | fn f2(_: F) where F: for<'a> Fn(&'a (), &()) {} | ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `f2` +help: hint: consider borrowing here: + | +LL | f2(|_: &'a (), _: ()| {}); + | ~~~~~~ +help: hint: consider borrowing here: + | +LL | f2(|_: (), _: &()| {}); + | ~~~ error[E0631]: type mismatch in closure arguments --> $DIR/anonymous-higher-ranked-lifetime.rs:4:5 @@ -45,6 +61,14 @@ note: required by a bound in `f3` | LL | fn f3<'a, F>(_: F) where F: Fn(&'a (), &()) {} | ^^^^^^^^^^^^^^^ required by this bound in `f3` +help: hint: consider borrowing here: + | +LL | f3(|_: &(), _: ()| {}); + | ~~~ +help: hint: consider borrowing here: + | +LL | f3(|_: (), _: &()| {}); + | ~~~ error[E0631]: type mismatch in closure arguments --> $DIR/anonymous-higher-ranked-lifetime.rs:5:5 @@ -61,6 +85,14 @@ note: required by a bound in `f4` | LL | fn f4(_: F) where F: for<'r> Fn(&(), &'r ()) {} | ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `f4` +help: hint: consider borrowing here: + | +LL | f4(|_: &(), _: ()| {}); + | ~~~ +help: hint: consider borrowing here: + | +LL | f4(|_: (), _: &'r ()| {}); + | ~~~~~~ error[E0631]: type mismatch in closure arguments --> $DIR/anonymous-higher-ranked-lifetime.rs:6:5 @@ -77,6 +109,14 @@ note: required by a bound in `f5` | LL | fn f5(_: F) where F: for<'r> Fn(&'r (), &'r ()) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `f5` +help: hint: consider borrowing here: + | +LL | f5(|_: &'r (), _: ()| {}); + | ~~~~~~ +help: hint: consider borrowing here: + | +LL | f5(|_: (), _: &'r ()| {}); + | ~~~~~~ error[E0631]: type mismatch in closure arguments --> $DIR/anonymous-higher-ranked-lifetime.rs:7:5 @@ -93,6 +133,10 @@ note: required by a bound in `g1` | LL | fn g1(_: F) where F: Fn(&(), Box) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `g1` +help: hint: consider borrowing here: + | +LL | g1(|_: &(), _: ()| {}); + | ~~~ error[E0631]: type mismatch in closure arguments --> $DIR/anonymous-higher-ranked-lifetime.rs:8:5 @@ -109,6 +153,10 @@ note: required by a bound in `g2` | LL | fn g2(_: F) where F: Fn(&(), fn(&())) {} | ^^^^^^^^^^^^^^^^ required by this bound in `g2` +help: hint: consider borrowing here: + | +LL | g2(|_: &(), _: ()| {}); + | ~~~ error[E0631]: type mismatch in closure arguments --> $DIR/anonymous-higher-ranked-lifetime.rs:9:5 @@ -125,6 +173,10 @@ note: required by a bound in `g3` | LL | fn g3(_: F) where F: for<'s> Fn(&'s (), Box) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `g3` +help: hint: consider borrowing here: + | +LL | g3(|_: &'s (), _: ()| {}); + | ~~~~~~ error[E0631]: type mismatch in closure arguments --> $DIR/anonymous-higher-ranked-lifetime.rs:10:5 @@ -141,6 +193,10 @@ note: required by a bound in `g4` | LL | fn g4(_: F) where F: Fn(&(), for<'r> fn(&'r ())) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `g4` +help: hint: consider borrowing here: + | +LL | g4(|_: &(), _: ()| {}); + | ~~~ error[E0631]: type mismatch in closure arguments --> $DIR/anonymous-higher-ranked-lifetime.rs:11:5 @@ -157,6 +213,14 @@ note: required by a bound in `h1` | LL | fn h1(_: F) where F: Fn(&(), Box, &(), fn(&(), &())) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `h1` +help: hint: consider borrowing here: + | +LL | h1(|_: &(), _: (), _: (), _: ()| {}); + | ~~~ +help: hint: consider borrowing here: + | +LL | h1(|_: (), _: (), _: &(), _: ()| {}); + | ~~~ error[E0631]: type mismatch in closure arguments --> $DIR/anonymous-higher-ranked-lifetime.rs:12:5 @@ -173,6 +237,14 @@ note: required by a bound in `h2` | LL | fn h2(_: F) where F: for<'t0> Fn(&(), Box, &'t0 (), fn(&(), &())) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `h2` +help: hint: consider borrowing here: + | +LL | h2(|_: &(), _: (), _: (), _: ()| {}); + | ~~~ +help: hint: consider borrowing here: + | +LL | h2(|_: (), _: (), _: &'t0 (), _: ()| {}); + | ~~~~~~~ error: aborting due to 11 previous errors diff --git a/src/test/ui/mismatched_types/closure-arg-type-mismatch.stderr b/src/test/ui/mismatched_types/closure-arg-type-mismatch.stderr index 92d545b7366e3..92502c5b48f07 100644 --- a/src/test/ui/mismatched_types/closure-arg-type-mismatch.stderr +++ b/src/test/ui/mismatched_types/closure-arg-type-mismatch.stderr @@ -13,6 +13,10 @@ note: required by a bound in `map` | LL | F: FnMut(Self::Item) -> B, | ^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `map` +help: hint: consider borrowing here: + | +LL | a.iter().map(|_: &(u32, u32)| 45); + | ~~~~~~~~~~~ error[E0631]: type mismatch in closure arguments --> $DIR/closure-arg-type-mismatch.rs:4:14 diff --git a/src/test/ui/mismatched_types/issue-36053-2.stderr b/src/test/ui/mismatched_types/issue-36053-2.stderr index 906001ca1e09e..f778bab1e8ecc 100644 --- a/src/test/ui/mismatched_types/issue-36053-2.stderr +++ b/src/test/ui/mismatched_types/issue-36053-2.stderr @@ -13,6 +13,10 @@ note: required by a bound in `filter` | LL | P: FnMut(&Self::Item) -> bool, | ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `filter` +help: hint: consider borrowing here: + | +LL | once::<&str>("str").fuse().filter(|a: &&str| true).count(); + | ~~~~~ error[E0599]: the method `count` exists for struct `Filter>, [closure@$DIR/issue-36053-2.rs:7:39: 7:48]>`, but its trait bounds were not satisfied --> $DIR/issue-36053-2.rs:7:55 From e8cd512a38fd5db93cc5b4881604cefefbf9ed2f Mon Sep 17 00:00:00 2001 From: akida31 Date: Sun, 9 Oct 2022 10:07:47 +0200 Subject: [PATCH 3/6] Remove `hint` from help message --- .../src/traits/error_reporting/suggestions.rs | 4 +-- .../anonymous-higher-ranked-lifetime.stderr | 36 +++++++++---------- .../closure-arg-type-mismatch.stderr | 2 +- .../ui/mismatched_types/issue-36053-2.stderr | 2 +- 4 files changed, 22 insertions(+), 22 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 46e543c371c97..ee8fba6cc4b52 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -1710,11 +1710,11 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { if found_ty == expected_ty { let hint = if found_refs < expected_refs { - "hint: consider borrowing here:" + "consider borrowing here:" } else if found_refs == expected_refs { continue; } else { - "hint: consider removing the borrow:" + "consider removing the borrow:" }; err.span_suggestion_verbose( arg_span, diff --git a/src/test/ui/anonymous-higher-ranked-lifetime.stderr b/src/test/ui/anonymous-higher-ranked-lifetime.stderr index af6ae1273041a..499ee1f9b5b46 100644 --- a/src/test/ui/anonymous-higher-ranked-lifetime.stderr +++ b/src/test/ui/anonymous-higher-ranked-lifetime.stderr @@ -13,11 +13,11 @@ note: required by a bound in `f1` | LL | fn f1(_: F) where F: Fn(&(), &()) {} | ^^^^^^^^^^^^ required by this bound in `f1` -help: hint: consider borrowing here: +help: consider borrowing here: | LL | f1(|_: &(), _: ()| {}); | ~~~ -help: hint: consider borrowing here: +help: consider borrowing here: | LL | f1(|_: (), _: &()| {}); | ~~~ @@ -37,11 +37,11 @@ note: required by a bound in `f2` | LL | fn f2(_: F) where F: for<'a> Fn(&'a (), &()) {} | ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `f2` -help: hint: consider borrowing here: +help: consider borrowing here: | LL | f2(|_: &'a (), _: ()| {}); | ~~~~~~ -help: hint: consider borrowing here: +help: consider borrowing here: | LL | f2(|_: (), _: &()| {}); | ~~~ @@ -61,11 +61,11 @@ note: required by a bound in `f3` | LL | fn f3<'a, F>(_: F) where F: Fn(&'a (), &()) {} | ^^^^^^^^^^^^^^^ required by this bound in `f3` -help: hint: consider borrowing here: +help: consider borrowing here: | LL | f3(|_: &(), _: ()| {}); | ~~~ -help: hint: consider borrowing here: +help: consider borrowing here: | LL | f3(|_: (), _: &()| {}); | ~~~ @@ -85,11 +85,11 @@ note: required by a bound in `f4` | LL | fn f4(_: F) where F: for<'r> Fn(&(), &'r ()) {} | ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `f4` -help: hint: consider borrowing here: +help: consider borrowing here: | LL | f4(|_: &(), _: ()| {}); | ~~~ -help: hint: consider borrowing here: +help: consider borrowing here: | LL | f4(|_: (), _: &'r ()| {}); | ~~~~~~ @@ -109,11 +109,11 @@ note: required by a bound in `f5` | LL | fn f5(_: F) where F: for<'r> Fn(&'r (), &'r ()) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `f5` -help: hint: consider borrowing here: +help: consider borrowing here: | LL | f5(|_: &'r (), _: ()| {}); | ~~~~~~ -help: hint: consider borrowing here: +help: consider borrowing here: | LL | f5(|_: (), _: &'r ()| {}); | ~~~~~~ @@ -133,7 +133,7 @@ note: required by a bound in `g1` | LL | fn g1(_: F) where F: Fn(&(), Box) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `g1` -help: hint: consider borrowing here: +help: consider borrowing here: | LL | g1(|_: &(), _: ()| {}); | ~~~ @@ -153,7 +153,7 @@ note: required by a bound in `g2` | LL | fn g2(_: F) where F: Fn(&(), fn(&())) {} | ^^^^^^^^^^^^^^^^ required by this bound in `g2` -help: hint: consider borrowing here: +help: consider borrowing here: | LL | g2(|_: &(), _: ()| {}); | ~~~ @@ -173,7 +173,7 @@ note: required by a bound in `g3` | LL | fn g3(_: F) where F: for<'s> Fn(&'s (), Box) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `g3` -help: hint: consider borrowing here: +help: consider borrowing here: | LL | g3(|_: &'s (), _: ()| {}); | ~~~~~~ @@ -193,7 +193,7 @@ note: required by a bound in `g4` | LL | fn g4(_: F) where F: Fn(&(), for<'r> fn(&'r ())) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `g4` -help: hint: consider borrowing here: +help: consider borrowing here: | LL | g4(|_: &(), _: ()| {}); | ~~~ @@ -213,11 +213,11 @@ note: required by a bound in `h1` | LL | fn h1(_: F) where F: Fn(&(), Box, &(), fn(&(), &())) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `h1` -help: hint: consider borrowing here: +help: consider borrowing here: | LL | h1(|_: &(), _: (), _: (), _: ()| {}); | ~~~ -help: hint: consider borrowing here: +help: consider borrowing here: | LL | h1(|_: (), _: (), _: &(), _: ()| {}); | ~~~ @@ -237,11 +237,11 @@ note: required by a bound in `h2` | LL | fn h2(_: F) where F: for<'t0> Fn(&(), Box, &'t0 (), fn(&(), &())) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `h2` -help: hint: consider borrowing here: +help: consider borrowing here: | LL | h2(|_: &(), _: (), _: (), _: ()| {}); | ~~~ -help: hint: consider borrowing here: +help: consider borrowing here: | LL | h2(|_: (), _: (), _: &'t0 (), _: ()| {}); | ~~~~~~~ diff --git a/src/test/ui/mismatched_types/closure-arg-type-mismatch.stderr b/src/test/ui/mismatched_types/closure-arg-type-mismatch.stderr index 92502c5b48f07..321b7544f288b 100644 --- a/src/test/ui/mismatched_types/closure-arg-type-mismatch.stderr +++ b/src/test/ui/mismatched_types/closure-arg-type-mismatch.stderr @@ -13,7 +13,7 @@ note: required by a bound in `map` | LL | F: FnMut(Self::Item) -> B, | ^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `map` -help: hint: consider borrowing here: +help: consider borrowing here: | LL | a.iter().map(|_: &(u32, u32)| 45); | ~~~~~~~~~~~ diff --git a/src/test/ui/mismatched_types/issue-36053-2.stderr b/src/test/ui/mismatched_types/issue-36053-2.stderr index f778bab1e8ecc..5aa0eb48725d0 100644 --- a/src/test/ui/mismatched_types/issue-36053-2.stderr +++ b/src/test/ui/mismatched_types/issue-36053-2.stderr @@ -13,7 +13,7 @@ note: required by a bound in `filter` | LL | P: FnMut(&Self::Item) -> bool, | ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `filter` -help: hint: consider borrowing here: +help: consider borrowing here: | LL | once::<&str>("str").fuse().filter(|a: &&str| true).count(); | ~~~~~ From 23c99a46d8494106c89c5fe33e8f8a46a0490d28 Mon Sep 17 00:00:00 2001 From: akida31 Date: Tue, 11 Oct 2022 16:20:52 +0200 Subject: [PATCH 4/6] move changes to an extra function --- .../src/traits/error_reporting/suggestions.rs | 142 ++++++++++-------- 1 file changed, 76 insertions(+), 66 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 ee8fba6cc4b52..9d44b081848e9 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -1657,73 +1657,8 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { self.note_conflicting_closure_bounds(cause, &mut err); - let found_args = match found.kind() { - ty::FnPtr(f) => f.inputs().skip_binder().iter(), - kind => { - span_bug!(span, "found was converted to a FnPtr above but is now {:?}", kind) - } - }; - let expected_args = match expected.kind() { - ty::FnPtr(f) => f.inputs().skip_binder().iter(), - kind => { - span_bug!(span, "expected was converted to a FnPtr above but is now {:?}", kind) - } - }; - if let Some(found_node) = found_node { - let fn_decl = match found_node { - Node::Expr(expr) => match &expr.kind { - hir::ExprKind::Closure(hir::Closure { fn_decl, .. }) => fn_decl, - kind => { - span_bug!(found_span, "expression must be a closure but is {:?}", kind) - } - }, - Node::Item(item) => match &item.kind { - hir::ItemKind::Fn(signature, _generics, _body) => signature.decl, - kind => { - span_bug!(found_span, "item must be a function but is {:?}", kind) - } - }, - node => { - span_bug!(found_span, "node must be a expr or item but is {:?}", node) - } - }; - - let arg_spans = fn_decl.inputs.iter().map(|ty| ty.span); - - fn get_deref_type_and_refs(mut ty: Ty<'_>) -> (Ty<'_>, usize) { - let mut refs = 0; - - while let ty::Ref(_, new_ty, _) = ty.kind() { - ty = *new_ty; - refs += 1; - } - - (ty, refs) - } - - for ((found_arg, expected_arg), arg_span) in - found_args.zip(expected_args).zip(arg_spans) - { - let (found_ty, found_refs) = get_deref_type_and_refs(*found_arg); - let (expected_ty, expected_refs) = get_deref_type_and_refs(*expected_arg); - - if found_ty == expected_ty { - let hint = if found_refs < expected_refs { - "consider borrowing here:" - } else if found_refs == expected_refs { - continue; - } else { - "consider removing the borrow:" - }; - err.span_suggestion_verbose( - arg_span, - hint, - expected_arg.to_string(), - Applicability::MaybeIncorrect, - ); - } - } + hint_missing_borrow(span, found_span, found, expected, found_node, &mut err); } err @@ -3108,6 +3043,81 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { } } +/// Add a hint to add a missing borrow or remove an unnecessary one. +fn hint_missing_borrow<'tcx>( + span: Span, + found_span: Span, + found: Ty<'tcx>, + expected: Ty<'tcx>, + found_node: Node<'_>, + err: &mut Diagnostic, +) { + let found_args = match found.kind() { + ty::FnPtr(f) => f.inputs().skip_binder().iter(), + kind => { + span_bug!(span, "found was converted to a FnPtr above but is now {:?}", kind) + } + }; + let expected_args = match expected.kind() { + ty::FnPtr(f) => f.inputs().skip_binder().iter(), + kind => { + span_bug!(span, "expected was converted to a FnPtr above but is now {:?}", kind) + } + }; + + let fn_decl = match found_node { + Node::Expr(expr) => match &expr.kind { + hir::ExprKind::Closure(hir::Closure { fn_decl, .. }) => fn_decl, + kind => { + span_bug!(found_span, "expression must be a closure but is {:?}", kind) + } + }, + Node::Item(item) => match &item.kind { + hir::ItemKind::Fn(signature, _generics, _body) => signature.decl, + kind => { + span_bug!(found_span, "item must be a function but is {:?}", kind) + } + }, + node => { + span_bug!(found_span, "node must be a expr or item but is {:?}", node) + } + }; + + let arg_spans = fn_decl.inputs.iter().map(|ty| ty.span); + + fn get_deref_type_and_refs<'tcx>(mut ty: Ty<'tcx>) -> (Ty<'tcx>, usize) { + let mut refs = 0; + + while let ty::Ref(_, new_ty, _) = ty.kind() { + ty = *new_ty; + refs += 1; + } + + (ty, refs) + } + + for ((found_arg, expected_arg), arg_span) in found_args.zip(expected_args).zip(arg_spans) { + let (found_ty, found_refs) = get_deref_type_and_refs(*found_arg); + let (expected_ty, expected_refs) = get_deref_type_and_refs(*expected_arg); + + if found_ty == expected_ty { + let hint = if found_refs < expected_refs { + "consider borrowing here:" + } else if found_refs == expected_refs { + continue; + } else { + "consider removing the borrow:" + }; + err.span_suggestion_verbose( + arg_span, + hint, + expected_arg.to_string(), + Applicability::MaybeIncorrect, + ); + } + } +} + /// Collect all the returned expressions within the input expression. /// Used to point at the return spans when we want to suggest some change to them. #[derive(Default)] From 121b43159ff21575aaecaca998d1a66325c29842 Mon Sep 17 00:00:00 2001 From: akida31 Date: Tue, 11 Oct 2022 17:02:56 +0200 Subject: [PATCH 5/6] change error message --- .../src/traits/error_reporting/suggestions.rs | 4 +-- .../anonymous-higher-ranked-lifetime.stderr | 36 +++++++++---------- .../ui/closures/multiple-fn-bounds.stderr | 4 +++ .../closure-arg-type-mismatch.stderr | 2 +- .../ui/mismatched_types/issue-36053-2.stderr | 2 +- 5 files changed, 26 insertions(+), 22 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 9d44b081848e9..8bb9aad020017 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -3102,11 +3102,11 @@ fn hint_missing_borrow<'tcx>( if found_ty == expected_ty { let hint = if found_refs < expected_refs { - "consider borrowing here:" + "consider borrowing the argument" } else if found_refs == expected_refs { continue; } else { - "consider removing the borrow:" + "do not borrow the argument" }; err.span_suggestion_verbose( arg_span, diff --git a/src/test/ui/anonymous-higher-ranked-lifetime.stderr b/src/test/ui/anonymous-higher-ranked-lifetime.stderr index 499ee1f9b5b46..2e5f7a51b6e2d 100644 --- a/src/test/ui/anonymous-higher-ranked-lifetime.stderr +++ b/src/test/ui/anonymous-higher-ranked-lifetime.stderr @@ -13,11 +13,11 @@ note: required by a bound in `f1` | LL | fn f1(_: F) where F: Fn(&(), &()) {} | ^^^^^^^^^^^^ required by this bound in `f1` -help: consider borrowing here: +help: consider borrowing the argument | LL | f1(|_: &(), _: ()| {}); | ~~~ -help: consider borrowing here: +help: consider borrowing the argument | LL | f1(|_: (), _: &()| {}); | ~~~ @@ -37,11 +37,11 @@ note: required by a bound in `f2` | LL | fn f2(_: F) where F: for<'a> Fn(&'a (), &()) {} | ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `f2` -help: consider borrowing here: +help: consider borrowing the argument | LL | f2(|_: &'a (), _: ()| {}); | ~~~~~~ -help: consider borrowing here: +help: consider borrowing the argument | LL | f2(|_: (), _: &()| {}); | ~~~ @@ -61,11 +61,11 @@ note: required by a bound in `f3` | LL | fn f3<'a, F>(_: F) where F: Fn(&'a (), &()) {} | ^^^^^^^^^^^^^^^ required by this bound in `f3` -help: consider borrowing here: +help: consider borrowing the argument | LL | f3(|_: &(), _: ()| {}); | ~~~ -help: consider borrowing here: +help: consider borrowing the argument | LL | f3(|_: (), _: &()| {}); | ~~~ @@ -85,11 +85,11 @@ note: required by a bound in `f4` | LL | fn f4(_: F) where F: for<'r> Fn(&(), &'r ()) {} | ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `f4` -help: consider borrowing here: +help: consider borrowing the argument | LL | f4(|_: &(), _: ()| {}); | ~~~ -help: consider borrowing here: +help: consider borrowing the argument | LL | f4(|_: (), _: &'r ()| {}); | ~~~~~~ @@ -109,11 +109,11 @@ note: required by a bound in `f5` | LL | fn f5(_: F) where F: for<'r> Fn(&'r (), &'r ()) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `f5` -help: consider borrowing here: +help: consider borrowing the argument | LL | f5(|_: &'r (), _: ()| {}); | ~~~~~~ -help: consider borrowing here: +help: consider borrowing the argument | LL | f5(|_: (), _: &'r ()| {}); | ~~~~~~ @@ -133,7 +133,7 @@ note: required by a bound in `g1` | LL | fn g1(_: F) where F: Fn(&(), Box) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `g1` -help: consider borrowing here: +help: consider borrowing the argument | LL | g1(|_: &(), _: ()| {}); | ~~~ @@ -153,7 +153,7 @@ note: required by a bound in `g2` | LL | fn g2(_: F) where F: Fn(&(), fn(&())) {} | ^^^^^^^^^^^^^^^^ required by this bound in `g2` -help: consider borrowing here: +help: consider borrowing the argument | LL | g2(|_: &(), _: ()| {}); | ~~~ @@ -173,7 +173,7 @@ note: required by a bound in `g3` | LL | fn g3(_: F) where F: for<'s> Fn(&'s (), Box) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `g3` -help: consider borrowing here: +help: consider borrowing the argument | LL | g3(|_: &'s (), _: ()| {}); | ~~~~~~ @@ -193,7 +193,7 @@ note: required by a bound in `g4` | LL | fn g4(_: F) where F: Fn(&(), for<'r> fn(&'r ())) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `g4` -help: consider borrowing here: +help: consider borrowing the argument | LL | g4(|_: &(), _: ()| {}); | ~~~ @@ -213,11 +213,11 @@ note: required by a bound in `h1` | LL | fn h1(_: F) where F: Fn(&(), Box, &(), fn(&(), &())) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `h1` -help: consider borrowing here: +help: consider borrowing the argument | LL | h1(|_: &(), _: (), _: (), _: ()| {}); | ~~~ -help: consider borrowing here: +help: consider borrowing the argument | LL | h1(|_: (), _: (), _: &(), _: ()| {}); | ~~~ @@ -237,11 +237,11 @@ note: required by a bound in `h2` | LL | fn h2(_: F) where F: for<'t0> Fn(&(), Box, &'t0 (), fn(&(), &())) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `h2` -help: consider borrowing here: +help: consider borrowing the argument | LL | h2(|_: &(), _: (), _: (), _: ()| {}); | ~~~ -help: consider borrowing here: +help: consider borrowing the argument | LL | h2(|_: (), _: (), _: &'t0 (), _: ()| {}); | ~~~~~~~ diff --git a/src/test/ui/closures/multiple-fn-bounds.stderr b/src/test/ui/closures/multiple-fn-bounds.stderr index eefc123fed7ac..32a1edb0024c0 100644 --- a/src/test/ui/closures/multiple-fn-bounds.stderr +++ b/src/test/ui/closures/multiple-fn-bounds.stderr @@ -18,6 +18,10 @@ note: required by a bound in `foo` | LL | fn foo bool + Fn(char) -> bool>(f: F) { | ^^^^^^^^^^^^^^^^ required by this bound in `foo` +help: do not borrow the argument + | +LL | foo(move |char| v); + | ~~~~ error: aborting due to previous error diff --git a/src/test/ui/mismatched_types/closure-arg-type-mismatch.stderr b/src/test/ui/mismatched_types/closure-arg-type-mismatch.stderr index 321b7544f288b..73290aa494738 100644 --- a/src/test/ui/mismatched_types/closure-arg-type-mismatch.stderr +++ b/src/test/ui/mismatched_types/closure-arg-type-mismatch.stderr @@ -13,7 +13,7 @@ note: required by a bound in `map` | LL | F: FnMut(Self::Item) -> B, | ^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `map` -help: consider borrowing here: +help: consider borrowing the argument | LL | a.iter().map(|_: &(u32, u32)| 45); | ~~~~~~~~~~~ diff --git a/src/test/ui/mismatched_types/issue-36053-2.stderr b/src/test/ui/mismatched_types/issue-36053-2.stderr index 5aa0eb48725d0..4340a4b89e058 100644 --- a/src/test/ui/mismatched_types/issue-36053-2.stderr +++ b/src/test/ui/mismatched_types/issue-36053-2.stderr @@ -13,7 +13,7 @@ note: required by a bound in `filter` | LL | P: FnMut(&Self::Item) -> bool, | ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `filter` -help: consider borrowing here: +help: consider borrowing the argument | LL | once::<&str>("str").fuse().filter(|a: &&str| true).count(); | ~~~~~ From 0f5409bd1c1e443a51558e10a2cb8bf5ff8f9f6e Mon Sep 17 00:00:00 2001 From: akida31 Date: Tue, 11 Oct 2022 17:48:46 +0200 Subject: [PATCH 6/6] remove manual `fn_decl` extraction --- .../src/traits/error_reporting/suggestions.rs | 20 +++---------------- 1 file changed, 3 insertions(+), 17 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 8bb9aad020017..01be926307e1e 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -3065,23 +3065,9 @@ fn hint_missing_borrow<'tcx>( } }; - let fn_decl = match found_node { - Node::Expr(expr) => match &expr.kind { - hir::ExprKind::Closure(hir::Closure { fn_decl, .. }) => fn_decl, - kind => { - span_bug!(found_span, "expression must be a closure but is {:?}", kind) - } - }, - Node::Item(item) => match &item.kind { - hir::ItemKind::Fn(signature, _generics, _body) => signature.decl, - kind => { - span_bug!(found_span, "item must be a function but is {:?}", kind) - } - }, - node => { - span_bug!(found_span, "node must be a expr or item but is {:?}", node) - } - }; + let fn_decl = found_node + .fn_decl() + .unwrap_or_else(|| span_bug!(found_span, "found node must be a function")); let arg_spans = fn_decl.inputs.iter().map(|ty| ty.span);