Skip to content

Commit

Permalink
Rollup merge of rust-lang#126558 - jieyouxu:caller-chooses-ty, r=fmease
Browse files Browse the repository at this point in the history
hir_typeck: be more conservative in making "note caller chooses ty param" note

In rust-lang#122195 I added a "caller chooses ty for type param" note for when the return expression type a.k.a. found type does not match the expected return type.

rust-lang#126547 found that this note was confusing when the found return type *contains* the expected type, e.g.

```rs
fn f<T>(t: &T) -> T {
    t
}
```

because the found return type `&T` will *always* be different from the expected return type `T`, so the note was needlessly redundant and confusing.

This PR addresses that by not making the note if the found return type contains the expected return type.

r? `@fmease` (since you reviewed the original PR)

Fixes rust-lang#126547
  • Loading branch information
jieyouxu authored Jun 19, 2024
2 parents b32d9ef + 939026c commit 5ec5dda
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 16 deletions.
32 changes: 22 additions & 10 deletions compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -859,10 +859,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
} else {
// Only point to return type if the expected type is the return type, as if they
// are not, the expectation must have been caused by something else.
debug!("return type {:?}", hir_ty);
debug!(?hir_ty, "return type");
let ty = self.lowerer().lower_ty(hir_ty);
debug!("return type {:?}", ty);
debug!("expected type {:?}", expected);
debug!(?ty, "return type (lowered)");
debug!(?expected, "expected type");
let bound_vars = self.tcx.late_bound_vars(hir_ty.hir_id.owner.into());
let ty = Binder::bind_with_vars(ty, bound_vars);
let ty = self.normalize(hir_ty.span, ty);
Expand All @@ -873,7 +873,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
expected,
});
self.try_suggest_return_impl_trait(err, expected, found, fn_id);
self.note_caller_chooses_ty_for_ty_param(err, expected, found);
self.try_note_caller_chooses_ty_for_ty_param(err, expected, found);
return true;
}
}
Expand All @@ -883,18 +883,30 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
false
}

fn note_caller_chooses_ty_for_ty_param(
fn try_note_caller_chooses_ty_for_ty_param(
&self,
diag: &mut Diag<'_>,
expected: Ty<'tcx>,
found: Ty<'tcx>,
) {
if let ty::Param(expected_ty_as_param) = expected.kind() {
diag.subdiagnostic(errors::NoteCallerChoosesTyForTyParam {
ty_param_name: expected_ty_as_param.name,
found_ty: found,
});
// Only show the note if:
// 1. `expected` ty is a type parameter;
// 2. The `expected` type parameter does *not* occur in the return expression type. This can
// happen for e.g. `fn foo<T>(t: &T) -> T { t }`, where `expected` is `T` but `found` is
// `&T`. Saying "the caller chooses a type for `T` which can be different from `&T`" is
// "well duh" and is only confusing and not helpful.
let ty::Param(expected_ty_as_param) = expected.kind() else {
return;
};

if found.contains(expected) {
return;
}

diag.subdiagnostic(errors::NoteCallerChoosesTyForTyParam {
ty_param_name: expected_ty_as_param.name,
found_ty: found,
});
}

/// check whether the return type is a generic type with a trait bound
Expand Down
11 changes: 10 additions & 1 deletion tests/ui/return/return-ty-mismatch-note.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// Checks existence of a note for "a caller chooses ty for ty param" upon return ty mismatch.
// Checks existence or absence of a note for "a caller chooses ty for ty param" upon return ty
// mismatch.

fn f<T>() -> (T,) {
(0,) //~ ERROR mismatched types
Expand All @@ -14,6 +15,14 @@ fn h() -> u8 {
0u8
}

// This case was reported in <https://github.com/rust-lang/rust/issues/126547> where it doesn't
// make sense to make the "note caller chooses ty for ty param" note if the found type contains
// the ty param...
fn k<T>(_t: &T) -> T {
_t
//~^ ERROR mismatched types
}

fn main() {
f::<()>();
g::<(), ()>;
Expand Down
21 changes: 17 additions & 4 deletions tests/ui/return/return-ty-mismatch-note.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0308]: mismatched types
--> $DIR/return-ty-mismatch-note.rs:4:6
--> $DIR/return-ty-mismatch-note.rs:5:6
|
LL | fn f<T>() -> (T,) {
| - expected this type parameter
Expand All @@ -10,7 +10,7 @@ LL | (0,)
found type `{integer}`

error[E0308]: mismatched types
--> $DIR/return-ty-mismatch-note.rs:8:6
--> $DIR/return-ty-mismatch-note.rs:9:6
|
LL | fn g<U, V>() -> (U, V) {
| - expected this type parameter
Expand All @@ -21,7 +21,7 @@ LL | (0, "foo")
found type `{integer}`

error[E0308]: mismatched types
--> $DIR/return-ty-mismatch-note.rs:8:9
--> $DIR/return-ty-mismatch-note.rs:9:9
|
LL | fn g<U, V>() -> (U, V) {
| - expected this type parameter
Expand All @@ -31,6 +31,19 @@ LL | (0, "foo")
= note: expected type parameter `V`
found reference `&'static str`

error: aborting due to 3 previous errors
error[E0308]: mismatched types
--> $DIR/return-ty-mismatch-note.rs:22:5
|
LL | fn k<T>(_t: &T) -> T {
| - - expected `T` because of return type
| |
| expected this type parameter
LL | _t
| ^^ expected type parameter `T`, found `&T`
|
= note: expected type parameter `_`
found reference `&_`

error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0308`.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ LL | t.clone()
|
= note: expected type parameter `_`
found reference `&_`
= note: the caller chooses a type for `T` which can be different from `&T`
note: `T` does not implement `Clone`, so `&T` was cloned instead
--> $DIR/clone-on-unconstrained-borrowed-type-param.rs:3:5
|
Expand Down

0 comments on commit 5ec5dda

Please sign in to comment.