Rollup merge of rust-lang#78659 - ayrtonm:fn-ref-lint-fix, r=oli-obk

Corrected suggestion for generic parameters in `function_item_references` lint

This commit handles functions with generic type parameters like you pointed out as well as const generics. Also this is probably a minor thing, but the type alias you used in the example doesn't show up so the suggestion right now would be `size_of::<[u8; 16]> as fn() ->`. This is because the lint checker works with MIR instead of HIR. I don't think we can get the alias at that point, but let me know if I'm wrong and there's a way to fix this. Also I put you as the reviewer, but I'm not sure if you want to review it or if it makes more sense to ask one of the original reviewers of this lint.
closes rust-lang#78571

Author: m-ou-se

compiler/rustc_mir/src/transform/function_item_references.rs

@@ -51,10 +51,11 @@ impl<'a, 'tcx> Visitor<'tcx> for FunctionItemRefChecker<'a, 'tcx> {
                         let arg_ty = args[0].ty(self.body, self.tcx);
                         for generic_inner_ty in arg_ty.walk() {
                             if let GenericArgKind::Type(inner_ty) = generic_inner_ty.unpack() {
-                                if let Some(fn_id) = FunctionItemRefChecker::is_fn_ref(inner_ty) {
-                                    let ident = self.tcx.item_name(fn_id).to_ident_string();
+                                if let Some((fn_id, fn_substs)) =
+                                    FunctionItemRefChecker::is_fn_ref(inner_ty)
+                                {
                                     let span = self.nth_arg_span(&args, 0);
-                                    self.emit_lint(ident, fn_id, source_info, span);
+                                    self.emit_lint(fn_id, fn_substs, source_info, span);
                                 }
                             }
                         }
@@ -66,6 +67,7 @@ impl<'a, 'tcx> Visitor<'tcx> for FunctionItemRefChecker<'a, 'tcx> {
         }
         self.super_terminator(terminator, location);
     }
+
     /// Emits a lint for function references formatted with `fmt::Pointer::fmt` by macros. These
     /// cases are handled as operands instead of call terminators to avoid any dependence on
     /// unstable, internal formatting details like whether `fmt` is called directly or not.
@@ -76,13 +78,12 @@ impl<'a, 'tcx> Visitor<'tcx> for FunctionItemRefChecker<'a, 'tcx> {
             if let ty::FnDef(def_id, substs_ref) = *op_ty.kind() {
                 if self.tcx.is_diagnostic_item(sym::pointer_trait_fmt, def_id) {
                     let param_ty = substs_ref.type_at(0);
-                    if let Some(fn_id) = FunctionItemRefChecker::is_fn_ref(param_ty) {
+                    if let Some((fn_id, fn_substs)) = FunctionItemRefChecker::is_fn_ref(param_ty) {
                         // The operand's ctxt wouldn't display the lint since it's inside a macro so
                         // we have to use the callsite's ctxt.
                         let callsite_ctxt = source_info.span.source_callsite().ctxt();
                         let span = source_info.span.with_ctxt(callsite_ctxt);
-                        let ident = self.tcx.item_name(fn_id).to_ident_string();
-                        self.emit_lint(ident, fn_id, source_info, span);
+                        self.emit_lint(fn_id, fn_substs, source_info, span);
                     }
                 }
             }
@@ -115,10 +116,11 @@ impl<'a, 'tcx> FunctionItemRefChecker<'a, 'tcx> {
                             if TyS::same_type(inner_ty, bound_ty) {
                                 // Do a substitution using the parameters from the callsite
                                 let subst_ty = inner_ty.subst(self.tcx, substs_ref);
-                                if let Some(fn_id) = FunctionItemRefChecker::is_fn_ref(subst_ty) {
-                                    let ident = self.tcx.item_name(fn_id).to_ident_string();
+                                if let Some((fn_id, fn_substs)) =
+                                    FunctionItemRefChecker::is_fn_ref(subst_ty)
+                                {
                                     let span = self.nth_arg_span(args, arg_num);
-                                    self.emit_lint(ident, fn_id, source_info, span);
+                                    self.emit_lint(fn_id, fn_substs, source_info, span);
                                 }
                             }
                         }
@@ -127,6 +129,7 @@ impl<'a, 'tcx> FunctionItemRefChecker<'a, 'tcx> {
             }
         }
     }
+
     /// If the given predicate is the trait `fmt::Pointer`, returns the bound parameter type.
     fn is_pointer_trait(&self, bound: &PredicateAtom<'tcx>) -> Option<Ty<'tcx>> {
         if let ty::PredicateAtom::Trait(predicate, _) = bound {
@@ -139,22 +142,26 @@ impl<'a, 'tcx> FunctionItemRefChecker<'a, 'tcx> {
             None
         }
     }
+
     /// If a type is a reference or raw pointer to the anonymous type of a function definition,
-    /// returns that function's `DefId`.
-    fn is_fn_ref(ty: Ty<'tcx>) -> Option<DefId> {
+    /// returns that function's `DefId` and `SubstsRef`.
+    fn is_fn_ref(ty: Ty<'tcx>) -> Option<(DefId, SubstsRef<'tcx>)> {
         let referent_ty = match ty.kind() {
             ty::Ref(_, referent_ty, _) => Some(referent_ty),
             ty::RawPtr(ty_and_mut) => Some(&ty_and_mut.ty),
             _ => None,
         };
         referent_ty
-            .map(
-                |ref_ty| {
-                    if let ty::FnDef(def_id, _) = *ref_ty.kind() { Some(def_id) } else { None }
-                },
-            )
+            .map(|ref_ty| {
+                if let ty::FnDef(def_id, substs_ref) = *ref_ty.kind() {
+                    Some((def_id, substs_ref))
+                } else {
+                    None
+                }
+            })
             .unwrap_or(None)
     }
+
     fn nth_arg_span(&self, args: &Vec<Operand<'tcx>>, n: usize) -> Span {
         match &args[n] {
             Operand::Copy(place) | Operand::Move(place) => {
@@ -163,7 +170,14 @@ impl<'a, 'tcx> FunctionItemRefChecker<'a, 'tcx> {
             Operand::Constant(constant) => constant.span,
         }
     }
-    fn emit_lint(&self, ident: String, fn_id: DefId, source_info: SourceInfo, span: Span) {
+
+    fn emit_lint(
+        &self,
+        fn_id: DefId,
+        fn_substs: SubstsRef<'tcx>,
+        source_info: SourceInfo,
+        span: Span,
+    ) {
         let lint_root = self.body.source_scopes[source_info.scope]
             .local_data
             .as_ref()
@@ -180,6 +194,10 @@ impl<'a, 'tcx> FunctionItemRefChecker<'a, 'tcx> {
                 s
             }
         };
+        let ident = self.tcx.item_name(fn_id).to_ident_string();
+        let ty_params = fn_substs.types().map(|ty| format!("{}", ty));
+        let const_params = fn_substs.consts().map(|c| format!("{}", c));
+        let params = ty_params.chain(const_params).collect::<Vec<String>>().join(", ");
         let num_args = fn_sig.inputs().map_bound(|inputs| inputs.len()).skip_binder();
         let variadic = if fn_sig.c_variadic() { ", ..." } else { "" };
         let ret = if fn_sig.output().skip_binder().is_unit() { "" } else { " -> _" };
@@ -190,7 +208,7 @@ impl<'a, 'tcx> FunctionItemRefChecker<'a, 'tcx> {
                     &format!("cast `{}` to obtain a function pointer", ident),
                     format!(
                         "{} as {}{}fn({}{}){}",
-                        ident,
+                        if params.is_empty() { ident } else { format!("{}::<{}>", ident, params) },
                         unsafety,
                         abi,
                         vec!["_"; num_args].join(", "),

src/test/ui/lint/function-item-references.rs

@@ -1,5 +1,5 @@
 // check-pass
-#![feature(c_variadic)]
+#![feature(c_variadic, min_const_generics)]
 #![warn(function_item_references)]
 use std::fmt::Pointer;
 use std::fmt::Formatter;
@@ -12,6 +12,10 @@ unsafe fn unsafe_fn() { }
 extern "C" fn c_fn() { }
 unsafe extern "C" fn unsafe_c_fn() { }
 unsafe extern fn variadic(_x: u32, _args: ...) { }
+fn take_generic_ref<'a, T>(_x: &'a T) { }
+fn take_generic_array<T, const N: usize>(_x: [T; N]) { }
+fn multiple_generic<T, U>(_x: T, _y: U) { }
+fn multiple_generic_arrays<T, U, const N: usize, const M: usize>(_x: [T; N], _y: [U; M]) { }
 
 //function references passed to these functions should never lint
 fn call_fn(f: &dyn Fn(u32) -> u32, x: u32) { f(x); }
@@ -109,6 +113,14 @@ fn main() {
     //~^ WARNING taking a reference to a function item does not give a function pointer
     println!("{:p}", &variadic);
     //~^ WARNING taking a reference to a function item does not give a function pointer
+    println!("{:p}", &take_generic_ref::<u32>);
+    //~^ WARNING taking a reference to a function item does not give a function pointer
+    println!("{:p}", &take_generic_array::<u32, 4>);
+    //~^ WARNING taking a reference to a function item does not give a function pointer
+    println!("{:p}", &multiple_generic::<u32, f32>);
+    //~^ WARNING taking a reference to a function item does not give a function pointer
+    println!("{:p}", &multiple_generic_arrays::<u32, f32, 4, 8>);
+    //~^ WARNING taking a reference to a function item does not give a function pointer
     println!("{:p}", &std::env::var::<String>);
     //~^ WARNING taking a reference to a function item does not give a function pointer
 
@@ -132,6 +144,8 @@ fn main() {
         std::mem::transmute::<_, (usize, usize)>((&foo, &bar));
         //~^ WARNING taking a reference to a function item does not give a function pointer
         //~^^ WARNING taking a reference to a function item does not give a function pointer
+        std::mem::transmute::<_, usize>(&take_generic_ref::<u32>);
+        //~^ WARNING taking a reference to a function item does not give a function pointer
 
         //the correct way to transmute function pointers
         std::mem::transmute::<_, usize>(foo as fn() -> u32);