Rollup merge of rust-lang#95395 - compiler-errors:infer-on-impl-for-trait, r=oli-obk

Better error message for `_` in function signature in `impl Trait for Ty`

Provides a replacement suggestion for when `_` is present in the function signature for `impl Trait for Ty`, using the substitutions from the trait to compute the exact type.

Fixes rust-lang#95097

Author: matthiaskrgr

compiler/rustc_typeck/src/astconv/mod.rs

@@ -2563,40 +2563,77 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
 
         // We proactively collect all the inferred type params to emit a single error per fn def.
         let mut visitor = HirPlaceholderCollector::default();
-        for ty in decl.inputs {
-            visitor.visit_ty(ty);
-        }
+        let mut infer_replacements = vec![];
+
         walk_generics(&mut visitor, generics);
 
-        let input_tys = decl.inputs.iter().map(|a| self.ty_of_arg(a, None));
+        let input_tys: Vec<_> = decl
+            .inputs
+            .iter()
+            .enumerate()
+            .map(|(i, a)| {
+                if let hir::TyKind::Infer = a.kind && !self.allow_ty_infer() {
+                    if let Some(suggested_ty) =
+                        self.suggest_trait_fn_ty_for_impl_fn_infer(hir_id, Some(i))
+                    {
+                        infer_replacements.push((a.span, suggested_ty.to_string()));
+                        return suggested_ty;
+                    }
+                }
+
+                // Only visit the type looking for `_` if we didn't fix the type above
+                visitor.visit_ty(a);
+                self.ty_of_arg(a, None)
+            })
+            .collect();
+
         let output_ty = match decl.output {
             hir::FnRetTy::Return(output) => {
-                visitor.visit_ty(output);
-                self.ast_ty_to_ty(output)
+                if let hir::TyKind::Infer = output.kind
+                    && !self.allow_ty_infer()
+                    && let Some(suggested_ty) =
+                        self.suggest_trait_fn_ty_for_impl_fn_infer(hir_id, None)
+                {
+                    infer_replacements.push((output.span, suggested_ty.to_string()));
+                    suggested_ty
+                } else {
+                    visitor.visit_ty(output);
+                    self.ast_ty_to_ty(output)
+                }
             }
             hir::FnRetTy::DefaultReturn(..) => tcx.mk_unit(),
         };
 
         debug!("ty_of_fn: output_ty={:?}", output_ty);
 
-        let fn_ty = tcx.mk_fn_sig(input_tys, output_ty, decl.c_variadic, unsafety, abi);
+        let fn_ty = tcx.mk_fn_sig(input_tys.into_iter(), output_ty, decl.c_variadic, unsafety, abi);
         let bare_fn_ty = ty::Binder::bind_with_vars(fn_ty, bound_vars);
 
-        if !self.allow_ty_infer() {
+        if !self.allow_ty_infer() && !(visitor.0.is_empty() && infer_replacements.is_empty()) {
             // We always collect the spans for placeholder types when evaluating `fn`s, but we
             // only want to emit an error complaining about them if infer types (`_`) are not
             // allowed. `allow_ty_infer` gates this behavior. We check for the presence of
             // `ident_span` to not emit an error twice when we have `fn foo(_: fn() -> _)`.
 
-            crate::collect::placeholder_type_error(
+            let mut diag = crate::collect::placeholder_type_error_diag(
                 tcx,
                 ident_span.map(|sp| sp.shrink_to_hi()),
                 generics.params,
                 visitor.0,
+                infer_replacements.iter().map(|(s, _)| *s).collect(),
                 true,
                 hir_ty,
                 "function",
             );
+
+            if !infer_replacements.is_empty() {
+                diag.multipart_suggestion(&format!(
+                    "try replacing `_` with the type{} in the corresponding trait method signature",
+                    rustc_errors::pluralize!(infer_replacements.len()),
+                ), infer_replacements, Applicability::MachineApplicable);
+            }
+
+            diag.emit();
         }
 
         // Find any late-bound regions declared in return type that do
@@ -2624,6 +2661,43 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
         bare_fn_ty
     }
 
+    /// Given a fn_hir_id for a impl function, suggest the type that is found on the
+    /// corresponding function in the trait that the impl implements, if it exists.
+    /// If arg_idx is Some, then it corresponds to an input type index, otherwise it
+    /// corresponds to the return type.
+    fn suggest_trait_fn_ty_for_impl_fn_infer(
+        &self,
+        fn_hir_id: hir::HirId,
+        arg_idx: Option<usize>,
+    ) -> Option<Ty<'tcx>> {
+        let tcx = self.tcx();
+        let hir = tcx.hir();
+
+        let hir::Node::ImplItem(hir::ImplItem { kind: hir::ImplItemKind::Fn(..), ident, .. }) =
+            hir.get(fn_hir_id) else { return None };
+        let hir::Node::Item(hir::Item { kind: hir::ItemKind::Impl(i), .. }) =
+                hir.get(hir.get_parent_node(fn_hir_id)) else { bug!("ImplItem should have Impl parent") };
+
+        let trait_ref =
+            self.instantiate_mono_trait_ref(i.of_trait.as_ref()?, self.ast_ty_to_ty(i.self_ty));
+
+        let x: &ty::AssocItem = tcx.associated_items(trait_ref.def_id).find_by_name_and_kind(
+            tcx,
+            *ident,
+            ty::AssocKind::Fn,
+            trait_ref.def_id,
+        )?;
+
+        let fn_sig = tcx.fn_sig(x.def_id).subst(
+            tcx,
+            trait_ref.substs.extend_to(tcx, x.def_id, |param, _| tcx.mk_param_from_def(param)),
+        );
+
+        let ty = if let Some(arg_idx) = arg_idx { fn_sig.input(arg_idx) } else { fn_sig.output() };
+
+        Some(tcx.erase_late_bound_regions(ty))
+    }
+
     fn validate_late_bound_regions(
         &self,
         constrained_regions: FxHashSet<ty::BoundRegionKind>,

compiler/rustc_typeck/src/collect.rs

@@ -160,6 +160,33 @@ crate fn placeholder_type_error<'tcx>(
         return;
     }
 
+    placeholder_type_error_diag(
+        tcx,
+        span,
+        generics,
+        placeholder_types,
+        vec![],
+        suggest,
+        hir_ty,
+        kind,
+    )
+    .emit();
+}
+
+crate fn placeholder_type_error_diag<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    span: Option<Span>,
+    generics: &[hir::GenericParam<'_>],
+    placeholder_types: Vec<Span>,
+    additional_spans: Vec<Span>,
+    suggest: bool,
+    hir_ty: Option<&hir::Ty<'_>>,
+    kind: &'static str,
+) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
+    if placeholder_types.is_empty() {
+        return bad_placeholder(tcx, additional_spans, kind);
+    }
+
     let type_name = generics.next_type_param_name(None);
     let mut sugg: Vec<_> =
         placeholder_types.iter().map(|sp| (*sp, (*type_name).to_string())).collect();
@@ -182,7 +209,8 @@ crate fn placeholder_type_error<'tcx>(
         sugg.push((span, format!(", {}", type_name)));
     }
 
-    let mut err = bad_placeholder(tcx, placeholder_types, kind);
+    let mut err =
+        bad_placeholder(tcx, placeholder_types.into_iter().chain(additional_spans).collect(), kind);
 
     // Suggest, but only if it is not a function in const or static
     if suggest {
@@ -218,7 +246,8 @@ crate fn placeholder_type_error<'tcx>(
             );
         }
     }
-    err.emit();
+
+    err
 }
 
 fn reject_placeholder_type_signatures_in_item<'tcx>(
@@ -1868,50 +1897,17 @@ fn fn_sig(tcx: TyCtxt<'_>, def_id: DefId) -> ty::PolyFnSig<'_> {
             generics,
             ..
         })
-        | ImplItem(hir::ImplItem { kind: ImplItemKind::Fn(sig, _), ident, generics, .. })
         | Item(hir::Item { kind: ItemKind::Fn(sig, generics, _), ident, .. }) => {
-            match get_infer_ret_ty(&sig.decl.output) {
-                Some(ty) => {
-                    let fn_sig = tcx.typeck(def_id).liberated_fn_sigs()[hir_id];
-                    // Typeck doesn't expect erased regions to be returned from `type_of`.
-                    let fn_sig = tcx.fold_regions(fn_sig, &mut false, |r, _| match *r {
-                        ty::ReErased => tcx.lifetimes.re_static,
-                        _ => r,
-                    });
-                    let fn_sig = ty::Binder::dummy(fn_sig);
-
-                    let mut visitor = HirPlaceholderCollector::default();
-                    visitor.visit_ty(ty);
-                    let mut diag = bad_placeholder(tcx, visitor.0, "return type");
-                    let ret_ty = fn_sig.skip_binder().output();
-                    if !ret_ty.references_error() {
-                        if !ret_ty.is_closure() {
-                            let ret_ty_str = match ret_ty.kind() {
-                                // Suggest a function pointer return type instead of a unique function definition
-                                // (e.g. `fn() -> i32` instead of `fn() -> i32 { f }`, the latter of which is invalid
-                                // syntax)
-                                ty::FnDef(..) => ret_ty.fn_sig(tcx).to_string(),
-                                _ => ret_ty.to_string(),
-                            };
-                            diag.span_suggestion(
-                                ty.span,
-                                "replace with the correct return type",
-                                ret_ty_str,
-                                Applicability::MaybeIncorrect,
-                            );
-                        } else {
-                            // We're dealing with a closure, so we should suggest using `impl Fn` or trait bounds
-                            // to prevent the user from getting a papercut while trying to use the unique closure
-                            // syntax (e.g. `[closure@src/lib.rs:2:5: 2:9]`).
-                            diag.help("consider using an `Fn`, `FnMut`, or `FnOnce` trait bound");
-                            diag.note("for more information on `Fn` traits and closure types, see https://doc.rust-lang.org/book/ch13-01-closures.html");
-                        }
-                    }
-                    diag.emit();
+            infer_return_ty_for_fn_sig(tcx, sig, *ident, generics, def_id, &icx)
+        }
 
-                    fn_sig
-                }
-                None => <dyn AstConv<'_>>::ty_of_fn(
+        ImplItem(hir::ImplItem { kind: ImplItemKind::Fn(sig, _), ident, generics, .. }) => {
+            // Do not try to inference the return type for a impl method coming from a trait
+            if let Item(hir::Item { kind: ItemKind::Impl(i), .. }) =
+                tcx.hir().get(tcx.hir().get_parent_node(hir_id))
+                && i.of_trait.is_some()
+            {
+                <dyn AstConv<'_>>::ty_of_fn(
                     &icx,
                     hir_id,
                     sig.header.unsafety,
@@ -1920,7 +1916,9 @@ fn fn_sig(tcx: TyCtxt<'_>, def_id: DefId) -> ty::PolyFnSig<'_> {
                     generics,
                     Some(ident.span),
                     None,
-                ),
+                )
+            } else {
+                infer_return_ty_for_fn_sig(tcx, sig, *ident, generics, def_id, &icx)
             }
         }
 
@@ -1982,6 +1980,70 @@ fn fn_sig(tcx: TyCtxt<'_>, def_id: DefId) -> ty::PolyFnSig<'_> {
     }
 }
 
+fn infer_return_ty_for_fn_sig<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    sig: &hir::FnSig<'_>,
+    ident: Ident,
+    generics: &hir::Generics<'_>,
+    def_id: LocalDefId,
+    icx: &ItemCtxt<'tcx>,
+) -> ty::PolyFnSig<'tcx> {
+    let hir_id = tcx.hir().local_def_id_to_hir_id(def_id);
+
+    match get_infer_ret_ty(&sig.decl.output) {
+        Some(ty) => {
+            let fn_sig = tcx.typeck(def_id).liberated_fn_sigs()[hir_id];
+            // Typeck doesn't expect erased regions to be returned from `type_of`.
+            let fn_sig = tcx.fold_regions(fn_sig, &mut false, |r, _| match *r {
+                ty::ReErased => tcx.lifetimes.re_static,
+                _ => r,
+            });
+            let fn_sig = ty::Binder::dummy(fn_sig);
+
+            let mut visitor = HirPlaceholderCollector::default();
+            visitor.visit_ty(ty);
+            let mut diag = bad_placeholder(tcx, visitor.0, "return type");
+            let ret_ty = fn_sig.skip_binder().output();
+            if !ret_ty.references_error() {
+                if !ret_ty.is_closure() {
+                    let ret_ty_str = match ret_ty.kind() {
+                        // Suggest a function pointer return type instead of a unique function definition
+                        // (e.g. `fn() -> i32` instead of `fn() -> i32 { f }`, the latter of which is invalid
+                        // syntax)
+                        ty::FnDef(..) => ret_ty.fn_sig(tcx).to_string(),
+                        _ => ret_ty.to_string(),
+                    };
+                    diag.span_suggestion(
+                        ty.span,
+                        "replace with the correct return type",
+                        ret_ty_str,
+                        Applicability::MaybeIncorrect,
+                    );
+                } else {
+                    // We're dealing with a closure, so we should suggest using `impl Fn` or trait bounds
+                    // to prevent the user from getting a papercut while trying to use the unique closure
+                    // syntax (e.g. `[closure@src/lib.rs:2:5: 2:9]`).
+                    diag.help("consider using an `Fn`, `FnMut`, or `FnOnce` trait bound");
+                    diag.note("for more information on `Fn` traits and closure types, see https://doc.rust-lang.org/book/ch13-01-closures.html");
+                }
+            }
+            diag.emit();
+
+            fn_sig
+        }
+        None => <dyn AstConv<'_>>::ty_of_fn(
+            icx,
+            hir_id,
+            sig.header.unsafety,
+            sig.header.abi,
+            sig.decl,
+            generics,
+            Some(ident.span),
+            None,
+        ),
+    }
+}
+
 fn impl_trait_ref(tcx: TyCtxt<'_>, def_id: DefId) -> Option<ty::TraitRef<'_>> {
     let icx = ItemCtxt::new(tcx, def_id);
     match tcx.hir().expect_item(def_id.expect_local()).kind {

src/test/ui/did_you_mean/replace-impl-infer-ty-from-trait.fixed

@@ -0,0 +1,15 @@
+// run-rustfix
+#![allow(unused)]
+
+trait Foo<T>: Sized {
+    fn bar(i: i32, t: T, s: &Self) -> (T, i32);
+}
+
+impl Foo<usize> for () {
+    fn bar(i: i32, t: usize, s: &()) -> (usize, i32) {
+        //~^ ERROR the placeholder `_` is not allowed within types on item signatures for functions
+        (1, 2)
+    }
+}
+
+fn main() {}

src/test/ui/did_you_mean/replace-impl-infer-ty-from-trait.rs

@@ -0,0 +1,15 @@
+// run-rustfix
+#![allow(unused)]
+
+trait Foo<T>: Sized {
+    fn bar(i: i32, t: T, s: &Self) -> (T, i32);
+}
+
+impl Foo<usize> for () {
+    fn bar(i: _, t: _, s: _) -> _ {
+        //~^ ERROR the placeholder `_` is not allowed within types on item signatures for functions
+        (1, 2)
+    }
+}
+
+fn main() {}

src/test/ui/did_you_mean/replace-impl-infer-ty-from-trait.stderr

@@ -0,0 +1,18 @@
+error[E0121]: the placeholder `_` is not allowed within types on item signatures for functions
+  --> $DIR/replace-impl-infer-ty-from-trait.rs:9:15
+   |
+LL |     fn bar(i: _, t: _, s: _) -> _ {
+   |               ^     ^     ^     ^ not allowed in type signatures
+   |               |     |     |
+   |               |     |     not allowed in type signatures
+   |               |     not allowed in type signatures
+   |               not allowed in type signatures
+   |
+help: try replacing `_` with the types in the corresponding trait method signature
+   |
+LL |     fn bar(i: i32, t: usize, s: &()) -> (usize, i32) {
+   |               ~~~     ~~~~~     ~~~     ~~~~~~~~~~~~
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0121`.

src/test/ui/typeck/typeck_type_placeholder_item.rs

@@ -57,7 +57,7 @@ unsafe fn test12(x: *const usize) -> *const *const _ {
 
 impl Clone for Test9 {
     fn clone(&self) -> _ { Test9 }
-    //~^ ERROR the placeholder `_` is not allowed within types on item signatures for return types
+    //~^ ERROR the placeholder `_` is not allowed within types on item signatures for functions
 
     fn clone_from(&mut self, other: _) { *self = Test9; }
     //~^ ERROR the placeholder `_` is not allowed within types on item signatures for functions
@@ -113,7 +113,7 @@ pub fn main() {
 
     impl Clone for FnTest9 {
         fn clone(&self) -> _ { FnTest9 }
-        //~^ ERROR the placeholder `_` is not allowed within types on item signatures for return types
+        //~^ ERROR the placeholder `_` is not allowed within types on item signatures for functions
 
         fn clone_from(&mut self, other: _) { *self = FnTest9; }
         //~^ ERROR the placeholder `_` is not allowed within types on item signatures for functions