Rollup merge of #126558 - jieyouxu:caller-chooses-ty, r=fmease

hir_typeck: be more conservative in making "note caller chooses ty param" note

In https://github.com/rust-lang/rust/pull/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.

#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 https://github.com/rust-lang/rust/issues/126547

4 files changed, 49 insertions, 16 deletions

diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
index 337a92c0d01..9dd82868adc 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 { 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
diff --git a/tests/ui/return/return-ty-mismatch-note.rs b/tests/ui/return/return-ty-mismatch-note.rs
index 352bc2a1637..36d590519fb 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>() -> (T,) {
     (0,) //~ ERROR mismatched types
@@ -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::<(), ()>;
diff --git a/tests/ui/return/return-ty-mismatch-note.stderr b/tests/ui/return/return-ty-mismatch-note.stderr
index 135903da5c2..47ef6863063 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>() -> (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>() -> (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>() -> (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) -> 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 2c4be26a82b..afbb9c32d51 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
    |