From bfe6a26af6669485b4661877583ebe3d22cd0317 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Tue, 18 Jun 2024 21:05:25 +0000 Subject: [PATCH 1/2] hir_typeck: be more conservative in making "note caller chooses ty param" note - Avoid "caller chooses ty for type param" note if the found type a.k.a. the return expression type *contains* the type parameter, because e.g. `&T` will always be different from `T` (i.e. "well duh"). - Rename `note_caller_chooses_ty_for_ty_param` to `try_note_caller_chooses_ty_for_ty_param` because the note is not always made. Issue: https://github.com/rust-lang/rust/issues/126547 --- .../src/fn_ctxt/suggestions.rs | 32 +++++++++++++------ 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs index 337a92c0d0120..9dd82868adc54 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs @@ -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); @@ -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; } } @@ -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 }`, 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 From 939026c8fba0d6b9edc2e2cdfbbf6dd6fb575662 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Sun, 16 Jun 2024 17:57:01 +0000 Subject: [PATCH 2/2] tests: update tests for more conservative return ty mismatch note --- tests/ui/return/return-ty-mismatch-note.rs | 11 +++++++++- .../ui/return/return-ty-mismatch-note.stderr | 21 +++++++++++++++---- ...n-unconstrained-borrowed-type-param.stderr | 1 - 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/tests/ui/return/return-ty-mismatch-note.rs b/tests/ui/return/return-ty-mismatch-note.rs index 352bc2a163763..36d590519fba2 100644 --- a/tests/ui/return/return-ty-mismatch-note.rs +++ b/tests/ui/return/return-ty-mismatch-note.rs @@ -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,) { (0,) //~ ERROR mismatched types @@ -14,6 +15,14 @@ fn h() -> u8 { 0u8 } +// This case was reported in 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 + //~^ ERROR mismatched types +} + fn main() { f::<()>(); g::<(), ()>; diff --git a/tests/ui/return/return-ty-mismatch-note.stderr b/tests/ui/return/return-ty-mismatch-note.stderr index 135903da5c263..47ef6863063c7 100644 --- a/tests/ui/return/return-ty-mismatch-note.stderr +++ b/tests/ui/return/return-ty-mismatch-note.stderr @@ -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,) { | - expected this type parameter @@ -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) { | - expected this type parameter @@ -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) { | - expected this type parameter @@ -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 { + | - - 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`. diff --git a/tests/ui/suggestions/clone-on-unconstrained-borrowed-type-param.stderr b/tests/ui/suggestions/clone-on-unconstrained-borrowed-type-param.stderr index 2c4be26a82b7b..afbb9c32d516e 100644 --- a/tests/ui/suggestions/clone-on-unconstrained-borrowed-type-param.stderr +++ b/tests/ui/suggestions/clone-on-unconstrained-borrowed-type-param.stderr @@ -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 |