Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tweak "borrow closure argument" suggestion #106891

Merged
merged 1 commit into from
Jan 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1350,6 +1350,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
expected_trait_ref,
obligation.cause.code(),
found_node,
obligation.param_env,
)
} else {
let (closure_span, closure_arg_span, found) = found_did
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ pub trait TypeErrCtxtExt<'tcx> {
expected: ty::PolyTraitRef<'tcx>,
cause: &ObligationCauseCode<'tcx>,
found_node: Option<Node<'_>>,
param_env: ty::ParamEnv<'tcx>,
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed>;

fn note_conflicting_closure_bounds(
Expand Down Expand Up @@ -1978,6 +1979,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
expected: ty::PolyTraitRef<'tcx>,
cause: &ObligationCauseCode<'tcx>,
found_node: Option<Node<'_>>,
param_env: ty::ParamEnv<'tcx>,
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
pub(crate) fn build_fn_sig_ty<'tcx>(
infcx: &InferCtxt<'tcx>,
Expand Down Expand Up @@ -2040,7 +2042,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
self.note_conflicting_closure_bounds(cause, &mut err);

if let Some(found_node) = found_node {
hint_missing_borrow(span, found, expected, found_node, &mut err);
hint_missing_borrow(self, param_env, span, found, expected, found_node, &mut err);
}

err
Expand Down Expand Up @@ -3747,6 +3749,8 @@ 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>(
infcx: &InferCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
span: Span,
found: Ty<'tcx>,
expected: Ty<'tcx>,
Expand All @@ -3769,7 +3773,7 @@ fn hint_missing_borrow<'tcx>(
// This could be a variant constructor, for example.
let Some(fn_decl) = found_node.fn_decl() else { return; };

let arg_spans = fn_decl.inputs.iter().map(|ty| ty.span);
let args = fn_decl.inputs.iter().map(|ty| ty);

fn get_deref_type_and_refs(mut ty: Ty<'_>) -> (Ty<'_>, usize) {
let mut refs = 0;
Expand All @@ -3785,29 +3789,42 @@ fn hint_missing_borrow<'tcx>(
let mut to_borrow = Vec::new();
let mut remove_borrow = Vec::new();

for ((found_arg, expected_arg), arg_span) in found_args.zip(expected_args).zip(arg_spans) {
for ((found_arg, expected_arg), arg) in found_args.zip(expected_args).zip(args) {
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 {
if infcx.can_eq(param_env, found_ty, expected_ty).is_ok() {
if found_refs < expected_refs {
to_borrow.push((arg_span, expected_arg.to_string()));
to_borrow.push((arg.span.shrink_to_lo(), "&".repeat(expected_refs - found_refs)));
} else if found_refs > expected_refs {
remove_borrow.push((arg_span, expected_arg.to_string()));
let mut span = arg.span.shrink_to_lo();
let mut left = found_refs - expected_refs;
let mut ty = arg;
while let hir::TyKind::Ref(_, mut_ty) = &ty.kind && left > 0 {
span = span.with_hi(mut_ty.ty.span.lo());
ty = mut_ty.ty;
left -= 1;
}
let sugg = if left == 0 {
(span, String::new())
} else {
(arg.span, expected_arg.to_string())
};
remove_borrow.push(sugg);
}
}
}

if !to_borrow.is_empty() {
err.multipart_suggestion(
err.multipart_suggestion_verbose(
"consider borrowing the argument",
to_borrow,
Applicability::MaybeIncorrect,
);
}

if !remove_borrow.is_empty() {
err.multipart_suggestion(
err.multipart_suggestion_verbose(
"do not borrow the argument",
remove_borrow,
Applicability::MaybeIncorrect,
Expand Down
62 changes: 35 additions & 27 deletions tests/ui/anonymous-higher-ranked-lifetime.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ LL | fn f1<F>(_: F) where F: Fn(&(), &()) {}
help: consider borrowing the argument
|
LL | f1(|_: &(), _: &()| {});
| ~~~ ~~~
| + +

error[E0631]: type mismatch in closure arguments
--> $DIR/anonymous-higher-ranked-lifetime.rs:3:5
Expand All @@ -35,8 +35,8 @@ LL | fn f2<F>(_: F) where F: for<'a> Fn(&'a (), &()) {}
| ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `f2`
help: consider borrowing the argument
|
LL | f2(|_: &'a (), _: &()| {});
| ~~~~~~ ~~~
LL | f2(|_: &(), _: &()| {});
| + +

error[E0631]: type mismatch in closure arguments
--> $DIR/anonymous-higher-ranked-lifetime.rs:4:5
Expand All @@ -56,7 +56,7 @@ LL | fn f3<'a, F>(_: F) where F: Fn(&'a (), &()) {}
help: consider borrowing the argument
|
LL | f3(|_: &(), _: &()| {});
| ~~~ ~~~
| + +

error[E0631]: type mismatch in closure arguments
--> $DIR/anonymous-higher-ranked-lifetime.rs:5:5
Expand All @@ -75,8 +75,8 @@ LL | fn f4<F>(_: F) where F: for<'r> Fn(&(), &'r ()) {}
| ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `f4`
help: consider borrowing the argument
|
LL | f4(|_: &(), _: &'r ()| {});
| ~~~ ~~~~~~
LL | f4(|_: &(), _: &()| {});
| + +

error[E0631]: type mismatch in closure arguments
--> $DIR/anonymous-higher-ranked-lifetime.rs:6:5
Expand All @@ -95,17 +95,15 @@ LL | fn f5<F>(_: F) where F: for<'r> Fn(&'r (), &'r ()) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `f5`
help: consider borrowing the argument
|
LL | f5(|_: &'r (), _: &'r ()| {});
| ~~~~~~ ~~~~~~
LL | f5(|_: &(), _: &()| {});
| + +

error[E0631]: type mismatch in closure arguments
--> $DIR/anonymous-higher-ranked-lifetime.rs:7:5
|
LL | g1(|_: (), _: ()| {});
| ^^ --------------
| | | |
| | | help: consider borrowing the argument: `&()`
| | found signature defined here
| ^^ -------------- found signature defined here
| |
| expected due to this
|
= note: expected closure signature `for<'a> fn(&'a (), Box<(dyn for<'a> Fn(&'a ()) + 'static)>) -> _`
Expand All @@ -115,15 +113,17 @@ note: required by a bound in `g1`
|
LL | fn g1<F>(_: F) where F: Fn(&(), Box<dyn Fn(&())>) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `g1`
help: consider borrowing the argument
|
LL | g1(|_: &(), _: ()| {});
| +

error[E0631]: type mismatch in closure arguments
--> $DIR/anonymous-higher-ranked-lifetime.rs:8:5
|
LL | g2(|_: (), _: ()| {});
| ^^ --------------
| | | |
| | | help: consider borrowing the argument: `&()`
| | found signature defined here
| ^^ -------------- found signature defined here
| |
| expected due to this
|
= note: expected closure signature `for<'a> fn(&'a (), for<'a> fn(&'a ())) -> _`
Expand All @@ -133,15 +133,17 @@ note: required by a bound in `g2`
|
LL | fn g2<F>(_: F) where F: Fn(&(), fn(&())) {}
| ^^^^^^^^^^^^^^^^ required by this bound in `g2`
help: consider borrowing the argument
|
LL | g2(|_: &(), _: ()| {});
| +

error[E0631]: type mismatch in closure arguments
--> $DIR/anonymous-higher-ranked-lifetime.rs:9:5
|
LL | g3(|_: (), _: ()| {});
| ^^ --------------
| | | |
| | | help: consider borrowing the argument: `&'s ()`
| | found signature defined here
| ^^ -------------- found signature defined here
| |
| expected due to this
|
= note: expected closure signature `for<'s> fn(&'s (), Box<(dyn for<'a> Fn(&'a ()) + 'static)>) -> _`
Expand All @@ -151,15 +153,17 @@ note: required by a bound in `g3`
|
LL | fn g3<F>(_: F) where F: for<'s> Fn(&'s (), Box<dyn Fn(&())>) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `g3`
help: consider borrowing the argument
|
LL | g3(|_: &(), _: ()| {});
| +

error[E0631]: type mismatch in closure arguments
--> $DIR/anonymous-higher-ranked-lifetime.rs:10:5
|
LL | g4(|_: (), _: ()| {});
| ^^ --------------
| | | |
| | | help: consider borrowing the argument: `&()`
| | found signature defined here
| ^^ -------------- found signature defined here
| |
| expected due to this
|
= note: expected closure signature `for<'a> fn(&'a (), for<'r> fn(&'r ())) -> _`
Expand All @@ -169,6 +173,10 @@ note: required by a bound in `g4`
|
LL | fn g4<F>(_: F) where F: Fn(&(), for<'r> fn(&'r ())) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `g4`
help: consider borrowing the argument
|
LL | g4(|_: &(), _: ()| {});
| +

error[E0631]: type mismatch in closure arguments
--> $DIR/anonymous-higher-ranked-lifetime.rs:11:5
Expand All @@ -188,7 +196,7 @@ LL | fn h1<F>(_: F) where F: Fn(&(), Box<dyn Fn(&())>, &(), fn(&(), &())) {}
help: consider borrowing the argument
|
LL | h1(|_: &(), _: (), _: &(), _: ()| {});
| ~~~ ~~~
| + +

error[E0631]: type mismatch in closure arguments
--> $DIR/anonymous-higher-ranked-lifetime.rs:12:5
Expand All @@ -207,8 +215,8 @@ LL | fn h2<F>(_: F) where F: for<'t0> Fn(&(), Box<dyn Fn(&())>, &'t0 (), fn(&(),
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `h2`
help: consider borrowing the argument
|
LL | h2(|_: &(), _: (), _: &'t0 (), _: ()| {});
| ~~~ ~~~~~~~
LL | h2(|_: &(), _: (), _: &(), _: ()| {});
| + +

error: aborting due to 11 previous errors

Expand Down
10 changes: 6 additions & 4 deletions tests/ui/closures/multiple-fn-bounds.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@ error[E0631]: type mismatch in closure arguments
--> $DIR/multiple-fn-bounds.rs:10:5
|
LL | foo(move |x| v);
| ^^^ --------
| | | |
| | | help: do not borrow the argument: `char`
| | found signature defined here
| ^^^ -------- found signature defined here
| |
| expected due to this
|
= note: expected closure signature `fn(char) -> _`
Expand All @@ -20,6 +18,10 @@ note: required by a bound in `foo`
|
LL | fn foo<F: Fn(&char) -> bool + Fn(char) -> bool>(f: F) {
| ^^^^^^^^^^^^^^^^ required by this bound in `foo`
help: do not borrow the argument
|
LL | foo(move |char| v);
| ~~~~
Comment on lines +21 to +24
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not adding this incorrect suggestion, just changing its presentation, but we need to fix this in a follow up.


error: aborting due to previous error

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// run-rustfix
fn main() {
let _ = (-10..=10).find(|x: &i32| x.signum() == 0); //~ ERROR type mismatch in closure arguments
let _ = (-10..=10).find(|x: &i32| x.signum() == 0); //~ ERROR type mismatch in closure arguments
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// run-rustfix
fn main() {
let _ = (-10..=10).find(|x: i32| x.signum() == 0); //~ ERROR type mismatch in closure arguments
let _ = (-10..=10).find(|x: &&&i32| x.signum() == 0); //~ ERROR type mismatch in closure arguments
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
error[E0631]: type mismatch in closure arguments
--> $DIR/closure-arg-type-mismatch-issue-45727.rs:3:24
|
LL | let _ = (-10..=10).find(|x: i32| x.signum() == 0);
| ^^^^ -------- found signature defined here
| |
| expected due to this
|
= note: expected closure signature `for<'a> fn(&'a {integer}) -> _`
found closure signature `fn(i32) -> _`
note: required by a bound in `find`
--> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
help: consider borrowing the argument
|
LL | let _ = (-10..=10).find(|x: &i32| x.signum() == 0);
| +

error[E0631]: type mismatch in closure arguments
--> $DIR/closure-arg-type-mismatch-issue-45727.rs:4:24
|
LL | let _ = (-10..=10).find(|x: &&&i32| x.signum() == 0);
| ^^^^ ----------- found signature defined here
| |
| expected due to this
|
= note: expected closure signature `for<'a> fn(&'a {integer}) -> _`
found closure signature `for<'a, 'b, 'c> fn(&'a &'b &'c i32) -> _`
note: required by a bound in `find`
--> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
help: do not borrow the argument
|
LL - let _ = (-10..=10).find(|x: &&&i32| x.signum() == 0);
LL + let _ = (-10..=10).find(|x: &i32| x.signum() == 0);
|

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0631`.
10 changes: 6 additions & 4 deletions tests/ui/mismatched_types/closure-arg-type-mismatch.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,18 @@ error[E0631]: type mismatch in closure arguments
--> $DIR/closure-arg-type-mismatch.rs:3:14
|
LL | a.iter().map(|_: (u32, u32)| 45);
| ^^^ ---------------
| | | |
| | | help: consider borrowing the argument: `&(u32, u32)`
| | found signature defined here
| ^^^ --------------- found signature defined here
| |
| expected due to this
|
= note: expected closure signature `fn(&(u32, u32)) -> _`
found closure signature `fn((u32, u32)) -> _`
note: required by a bound in `map`
--> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
help: consider borrowing the argument
|
LL | a.iter().map(|_: &(u32, u32)| 45);
| +

error[E0631]: type mismatch in closure arguments
--> $DIR/closure-arg-type-mismatch.rs:4:14
Expand Down
10 changes: 6 additions & 4 deletions tests/ui/mismatched_types/issue-36053-2.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,18 @@ error[E0631]: type mismatch in closure arguments
--> $DIR/issue-36053-2.rs:7:32
|
LL | once::<&str>("str").fuse().filter(|a: &str| true).count();
| ^^^^^^ ---------
| | | |
| | | help: consider borrowing the argument: `&&str`
| | found signature defined here
| ^^^^^^ --------- found signature defined here
| |
| expected due to this
|
= note: expected closure signature `for<'a> fn(&'a &str) -> _`
found closure signature `for<'a> fn(&'a str) -> _`
note: required by a bound in `filter`
--> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
help: consider borrowing the argument
|
LL | once::<&str>("str").fuse().filter(|a: &&str| true).count();
| +

error[E0599]: the method `count` exists for struct `Filter<Fuse<Once<&str>>, [closure@issue-36053-2.rs:7:39]>`, but its trait bounds were not satisfied
--> $DIR/issue-36053-2.rs:7:55
Expand Down