Skip to content

Commit 0f529f0

Browse files
borsbors
committed
Auto merge of #102813 - Akida31:issue-64915/simpler_diagnostic_when_passing_arg_to_closure_and_missing_borrow, r=estebank
Simpler diagnostic when passing arg to closure and missing borrow fixes #64915 I followed roughly the instructions and the older PR #76362. The number of references for the expected and the found types will be compared and depending on which has more the diagnostic will be emitted. I'm not quite sure if my approach with the many `span_bug!`s is good, it could lead to some ICEs. Would it be better if those errors are ignored? As far as I know the following code works similarly but in a different context. Is this probably reusable since it looks like it would emit better diagnostics? https://github.com/rust-lang/rust/blob/a688a0305fad9219505a8f2576446510601bafe8/compiler/rustc_hir_analysis/src/check/demand.rs#L713-L1061 When running the tests locally, a codegen test failed. Is there something I can/ should do about that? If you have some improvements/ corrections please say so and I will happily include them. r? `@estebank` (as you added the mentoring instructions to the issue)
2 parents aa5b179 + 05bc251 commit 0f529f0

File tree

6 files changed

+137
-14
lines changed

6 files changed

+137
-14
lines changed

‎compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1234,6 +1234,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
12341234
_ => None,
12351235
};
12361236

1237+
let found_node = found_did.and_then(|did| self.tcx.hir().get_if_local(did));
12371238
let found_span = found_did.and_then(|did| self.tcx.hir().span_if_local(did));
12381239

12391240
if self.reported_closure_mismatch.borrow().contains(&(span, found_span)) {
@@ -1287,6 +1288,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
12871288
found_trait_ref,
12881289
expected_trait_ref,
12891290
obligation.cause.code(),
1291+
found_node,
12901292
)
12911293
} else {
12921294
let (closure_span, closure_arg_span, found) = found_did

‎compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// ignore-tidy-filelength
2+
23
use super::{DefIdOrName, Obligation, ObligationCause, ObligationCauseCode, PredicateObligation};
34

45
use crate::autoderef::Autoderef;
@@ -258,6 +259,7 @@ pub trait TypeErrCtxtExt<'tcx> {
258259
found: ty::PolyTraitRef<'tcx>,
259260
expected: ty::PolyTraitRef<'tcx>,
260261
cause: &ObligationCauseCode<'tcx>,
262+
found_node: Option<Node<'_>>,
261263
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed>;
262264

263265
fn note_conflicting_closure_bounds(
@@ -1695,6 +1697,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
16951697
found: ty::PolyTraitRef<'tcx>,
16961698
expected: ty::PolyTraitRef<'tcx>,
16971699
cause: &ObligationCauseCode<'tcx>,
1700+
found_node: Option<Node<'_>>,
16981701
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
16991702
pub(crate) fn build_fn_sig_ty<'tcx>(
17001703
infcx: &InferCtxt<'tcx>,
@@ -1756,6 +1759,10 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
17561759

17571760
self.note_conflicting_closure_bounds(cause, &mut err);
17581761

1762+
if let Some(found_node) = found_node {
1763+
hint_missing_borrow(span, found_span, found, expected, found_node, &mut err);
1764+
}
1765+
17591766
err
17601767
}
17611768

@@ -3384,6 +3391,78 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
33843391
}
33853392
}
33863393

3394+
/// Add a hint to add a missing borrow or remove an unnecessary one.
3395+
fn hint_missing_borrow<'tcx>(
3396+
span: Span,
3397+
found_span: Span,
3398+
found: Ty<'tcx>,
3399+
expected: Ty<'tcx>,
3400+
found_node: Node<'_>,
3401+
err: &mut Diagnostic,
3402+
) {
3403+
let found_args = match found.kind() {
3404+
ty::FnPtr(f) => f.inputs().skip_binder().iter(),
3405+
kind => {
3406+
span_bug!(span, "found was converted to a FnPtr above but is now {:?}", kind)
3407+
}
3408+
};
3409+
let expected_args = match expected.kind() {
3410+
ty::FnPtr(f) => f.inputs().skip_binder().iter(),
3411+
kind => {
3412+
span_bug!(span, "expected was converted to a FnPtr above but is now {:?}", kind)
3413+
}
3414+
};
3415+
3416+
let fn_decl = found_node
3417+
.fn_decl()
3418+
.unwrap_or_else(|| span_bug!(found_span, "found node must be a function"));
3419+
3420+
let arg_spans = fn_decl.inputs.iter().map(|ty| ty.span);
3421+
3422+
fn get_deref_type_and_refs<'tcx>(mut ty: Ty<'tcx>) -> (Ty<'tcx>, usize) {
3423+
let mut refs = 0;
3424+
3425+
while let ty::Ref(_, new_ty, _) = ty.kind() {
3426+
ty = *new_ty;
3427+
refs += 1;
3428+
}
3429+
3430+
(ty, refs)
3431+
}
3432+
3433+
let mut to_borrow = Vec::new();
3434+
let mut remove_borrow = Vec::new();
3435+
3436+
for ((found_arg, expected_arg), arg_span) in found_args.zip(expected_args).zip(arg_spans) {
3437+
let (found_ty, found_refs) = get_deref_type_and_refs(*found_arg);
3438+
let (expected_ty, expected_refs) = get_deref_type_and_refs(*expected_arg);
3439+
3440+
if found_ty == expected_ty {
3441+
if found_refs < expected_refs {
3442+
to_borrow.push((arg_span, expected_arg.to_string()));
3443+
} else if found_refs > expected_refs {
3444+
remove_borrow.push((arg_span, expected_arg.to_string()));
3445+
}
3446+
}
3447+
}
3448+
3449+
if !to_borrow.is_empty() {
3450+
err.multipart_suggestion(
3451+
"consider borrowing the argument",
3452+
to_borrow,
3453+
Applicability::MaybeIncorrect,
3454+
);
3455+
}
3456+
3457+
if !remove_borrow.is_empty() {
3458+
err.multipart_suggestion(
3459+
"do not borrow the argument",
3460+
remove_borrow,
3461+
Applicability::MaybeIncorrect,
3462+
);
3463+
}
3464+
}
3465+
33873466
/// Collect all the returned expressions within the input expression.
33883467
/// Used to point at the return spans when we want to suggest some change to them.
33893468
#[derive(Default)]

‎src/test/ui/anonymous-higher-ranked-lifetime.stderr‎

Lines changed: 44 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ note: required by a bound in `f1`
1313
|
1414
LL | fn f1<F>(_: F) where F: Fn(&(), &()) {}
1515
| ^^^^^^^^^^^^ required by this bound in `f1`
16+
help: consider borrowing the argument
17+
|
18+
LL | f1(|_: &(), _: &()| {});
19+
| ~~~ ~~~
1620

1721
error[E0631]: type mismatch in closure arguments
1822
--> $DIR/anonymous-higher-ranked-lifetime.rs:3:5
@@ -29,6 +33,10 @@ note: required by a bound in `f2`
2933
|
3034
LL | fn f2<F>(_: F) where F: for<'a> Fn(&'a (), &()) {}
3135
| ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `f2`
36+
help: consider borrowing the argument
37+
|
38+
LL | f2(|_: &'a (), _: &()| {});
39+
| ~~~~~~ ~~~
3240

3341
error[E0631]: type mismatch in closure arguments
3442
--> $DIR/anonymous-higher-ranked-lifetime.rs:4:5
@@ -45,6 +53,10 @@ note: required by a bound in `f3`
4553
|
4654
LL | fn f3<'a, F>(_: F) where F: Fn(&'a (), &()) {}
4755
| ^^^^^^^^^^^^^^^ required by this bound in `f3`
56+
help: consider borrowing the argument
57+
|
58+
LL | f3(|_: &(), _: &()| {});
59+
| ~~~ ~~~
4860

4961
error[E0631]: type mismatch in closure arguments
5062
--> $DIR/anonymous-higher-ranked-lifetime.rs:5:5
@@ -61,6 +73,10 @@ note: required by a bound in `f4`
6173
|
6274
LL | fn f4<F>(_: F) where F: for<'r> Fn(&(), &'r ()) {}
6375
| ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `f4`
76+
help: consider borrowing the argument
77+
|
78+
LL | f4(|_: &(), _: &'r ()| {});
79+
| ~~~ ~~~~~~
6480

6581
error[E0631]: type mismatch in closure arguments
6682
--> $DIR/anonymous-higher-ranked-lifetime.rs:6:5
@@ -77,13 +93,19 @@ note: required by a bound in `f5`
7793
|
7894
LL | fn f5<F>(_: F) where F: for<'r> Fn(&'r (), &'r ()) {}
7995
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `f5`
96+
help: consider borrowing the argument
97+
|
98+
LL | f5(|_: &'r (), _: &'r ()| {});
99+
| ~~~~~~ ~~~~~~
80100

81101
error[E0631]: type mismatch in closure arguments
82102
--> $DIR/anonymous-higher-ranked-lifetime.rs:7:5
83103
|
84104
LL | g1(|_: (), _: ()| {});
85-
| ^^ -------------- found signature defined here
86-
| |
105+
| ^^ --------------
106+
| | | |
107+
| | | help: consider borrowing the argument: `&()`
108+
| | found signature defined here
87109
| expected due to this
88110
|
89111
= note: expected closure signature `for<'a> fn(&'a (), Box<(dyn for<'a> Fn(&'a ()) + 'static)>) -> _`
@@ -98,8 +120,10 @@ error[E0631]: type mismatch in closure arguments
98120
--> $DIR/anonymous-higher-ranked-lifetime.rs:8:5
99121
|
100122
LL | g2(|_: (), _: ()| {});
101-
| ^^ -------------- found signature defined here
102-
| |
123+
| ^^ --------------
124+
| | | |
125+
| | | help: consider borrowing the argument: `&()`
126+
| | found signature defined here
103127
| expected due to this
104128
|
105129
= note: expected closure signature `for<'a> fn(&'a (), for<'a> fn(&'a ())) -> _`
@@ -114,8 +138,10 @@ error[E0631]: type mismatch in closure arguments
114138
--> $DIR/anonymous-higher-ranked-lifetime.rs:9:5
115139
|
116140
LL | g3(|_: (), _: ()| {});
117-
| ^^ -------------- found signature defined here
118-
| |
141+
| ^^ --------------
142+
| | | |
143+
| | | help: consider borrowing the argument: `&'s ()`
144+
| | found signature defined here
119145
| expected due to this
120146
|
121147
= note: expected closure signature `for<'s> fn(&'s (), Box<(dyn for<'a> Fn(&'a ()) + 'static)>) -> _`
@@ -130,8 +156,10 @@ error[E0631]: type mismatch in closure arguments
130156
--> $DIR/anonymous-higher-ranked-lifetime.rs:10:5
131157
|
132158
LL | g4(|_: (), _: ()| {});
133-
| ^^ -------------- found signature defined here
134-
| |
159+
| ^^ --------------
160+
| | | |
161+
| | | help: consider borrowing the argument: `&()`
162+
| | found signature defined here
135163
| expected due to this
136164
|
137165
= note: expected closure signature `for<'a> fn(&'a (), for<'r> fn(&'r ())) -> _`
@@ -157,6 +185,10 @@ note: required by a bound in `h1`
157185
|
158186
LL | fn h1<F>(_: F) where F: Fn(&(), Box<dyn Fn(&())>, &(), fn(&(), &())) {}
159187
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `h1`
188+
help: consider borrowing the argument
189+
|
190+
LL | h1(|_: &(), _: (), _: &(), _: ()| {});
191+
| ~~~ ~~~
160192

161193
error[E0631]: type mismatch in closure arguments
162194
--> $DIR/anonymous-higher-ranked-lifetime.rs:12:5
@@ -173,6 +205,10 @@ note: required by a bound in `h2`
173205
|
174206
LL | fn h2<F>(_: F) where F: for<'t0> Fn(&(), Box<dyn Fn(&())>, &'t0 (), fn(&(), &())) {}
175207
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `h2`
208+
help: consider borrowing the argument
209+
|
210+
LL | h2(|_: &(), _: (), _: &'t0 (), _: ()| {});
211+
| ~~~ ~~~~~~~
176212

177213
error: aborting due to 11 previous errors
178214

‎src/test/ui/closures/multiple-fn-bounds.stderr‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@ error[E0631]: type mismatch in closure arguments
22
--> $DIR/multiple-fn-bounds.rs:10:5
33
|
44
LL | foo(move |x| v);
5-
| ^^^ -------- found signature defined here
6-
| |
5+
| ^^^ --------
6+
| | | |
7+
| | | help: do not borrow the argument: `char`
8+
| | found signature defined here
79
| expected due to this
810
|
911
= note: expected closure signature `fn(char) -> _`

‎src/test/ui/mismatched_types/closure-arg-type-mismatch.stderr‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@ error[E0631]: type mismatch in closure arguments
22
--> $DIR/closure-arg-type-mismatch.rs:3:14
33
|
44
LL | a.iter().map(|_: (u32, u32)| 45);
5-
| ^^^ --------------- found signature defined here
6-
| |
5+
| ^^^ ---------------
6+
| | | |
7+
| | | help: consider borrowing the argument: `&(u32, u32)`
8+
| | found signature defined here
79
| expected due to this
810
|
911
= note: expected closure signature `fn(&(u32, u32)) -> _`

‎src/test/ui/mismatched_types/issue-36053-2.stderr‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@ error[E0631]: type mismatch in closure arguments
22
--> $DIR/issue-36053-2.rs:7:32
33
|
44
LL | once::<&str>("str").fuse().filter(|a: &str| true).count();
5-
| ^^^^^^ --------- found signature defined here
6-
| |
5+
| ^^^^^^ ---------
6+
| | | |
7+
| | | help: consider borrowing the argument: `&&str`
8+
| | found signature defined here
79
| expected due to this
810
|
911
= note: expected closure signature `for<'a> fn(&'a &str) -> _`

0 commit comments

Comments
 (0)