modified lint to work with MIR

Working with MIR let's us exclude expressions like `&fn_name as &dyn Something`
and `(&fn_name)()`. Also added ABI, unsafety and whether a function is variadic
in the lint suggestion, included the `&` in the span of the lint and updated the
test.

Author: ayrtonm

compiler/rustc_lint/src/builtin.rs

@@ -2942,37 +2942,3 @@ impl<'tcx> LateLintPass<'tcx> for ClashingExternDeclarations {
         }
     }
 }
-
-declare_lint! {
-    FUNCTION_REFERENCES,
-    Warn,
-    "suggest casting functions to pointers when attempting to take references"
-}
-
-declare_lint_pass!(FunctionReferences => [FUNCTION_REFERENCES]);
-
-impl<'tcx> LateLintPass<'tcx> for FunctionReferences {
-    fn check_expr(&mut self, cx: &LateContext<'_>, e: &hir::Expr<'_>) {
-        if let hir::ExprKind::AddrOf(hir::BorrowKind::Ref, _, referent) = e.kind {
-            if let hir::ExprKind::Path(qpath) = &referent.kind {
-                if let Some(def_id) = cx.qpath_res(qpath, referent.hir_id).opt_def_id() {
-                    cx.tcx.hir().get_if_local(def_id).map(|node| {
-                        node.fn_decl().map(|decl| {
-                            if let Some(ident) = node.ident() {
-                                cx.struct_span_lint(FUNCTION_REFERENCES, referent.span, |lint| {
-                                    let num_args = decl.inputs.len();
-                                    lint.build(&format!(
-                                        "cast `{}` with `as *const fn({}) -> _` to use it as a pointer",
-                                        ident.to_string(),
-                                        vec!["_"; num_args].join(", ")
-                                    ))
-                                    .emit()
-                                });
-                            }
-                        });
-                    });
-                }
-            }
-        }
-    }
-}

compiler/rustc_lint/src/lib.rs

@@ -194,7 +194,6 @@ macro_rules! late_lint_mod_passes {
                 UnreachablePub: UnreachablePub,
                 ExplicitOutlivesRequirements: ExplicitOutlivesRequirements,
                 InvalidValue: InvalidValue,
-                FunctionReferences: FunctionReferences,
             ]
         );
     };

compiler/rustc_mir/src/transform/function_references.rs

@@ -0,0 +1,158 @@
+use rustc_hir::def_id::DefId;
+use rustc_middle::mir::visit::Visitor;
+use rustc_middle::mir::*;
+use rustc_middle::ty::{self, Ty, TyCtxt};
+use rustc_session::lint::builtin::FUNCTION_REFERENCES;
+use rustc_span::Span;
+use rustc_target::spec::abi::Abi;
+
+use crate::transform::{MirPass, MirSource};
+
+pub struct FunctionReferences;
+
+impl<'tcx> MirPass<'tcx> for FunctionReferences {
+    fn run_pass(&self, tcx: TyCtxt<'tcx>, _src: MirSource<'tcx>, body: &mut Body<'tcx>) {
+        let source_info = SourceInfo::outermost(body.span);
+        let mut checker = FunctionRefChecker {
+            tcx,
+            body,
+            potential_lints: Vec::new(),
+            casts: Vec::new(),
+            calls: Vec::new(),
+            source_info,
+        };
+        checker.visit_body(&body);
+    }
+}
+
+struct FunctionRefChecker<'a, 'tcx> {
+    tcx: TyCtxt<'tcx>,
+    body: &'a Body<'tcx>,
+    potential_lints: Vec<FunctionRefLint>,
+    casts: Vec<Span>,
+    calls: Vec<Span>,
+    source_info: SourceInfo,
+}
+
+impl<'a, 'tcx> Visitor<'tcx> for FunctionRefChecker<'a, 'tcx> {
+    fn visit_basic_block_data(&mut self, block: BasicBlock, data: &BasicBlockData<'tcx>) {
+        self.super_basic_block_data(block, data);
+        for cast_span in self.casts.drain(..) {
+            self.potential_lints.retain(|lint| lint.source_info.span != cast_span);
+        }
+        for call_span in self.calls.drain(..) {
+            self.potential_lints.retain(|lint| lint.source_info.span != call_span);
+        }
+        for lint in self.potential_lints.drain(..) {
+            lint.emit(self.tcx, self.body);
+        }
+    }
+    fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) {
+        self.source_info = statement.source_info;
+        self.super_statement(statement, location);
+    }
+    fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) {
+        self.source_info = terminator.source_info;
+        if let TerminatorKind::Call {
+            func,
+            args: _,
+            destination: _,
+            cleanup: _,
+            from_hir_call: _,
+            fn_span: _,
+        } = &terminator.kind
+        {
+            let span = match func {
+                Operand::Copy(place) | Operand::Move(place) => {
+                    self.body.local_decls[place.local].source_info.span
+                }
+                Operand::Constant(constant) => constant.span,
+            };
+            self.calls.push(span);
+        };
+        self.super_terminator(terminator, location);
+    }
+    fn visit_rvalue(&mut self, rvalue: &Rvalue<'tcx>, location: Location) {
+        match rvalue {
+            Rvalue::Ref(_, _, place) | Rvalue::AddressOf(_, place) => {
+                let decl = &self.body.local_decls[place.local];
+                if let ty::FnDef(def_id, _) = decl.ty.kind {
+                    let ident = self
+                        .body
+                        .var_debug_info
+                        .iter()
+                        .find(|info| info.source_info.span == decl.source_info.span)
+                        .map(|info| info.name.to_ident_string())
+                        .unwrap_or(self.tcx.def_path_str(def_id));
+                    let lint = FunctionRefLint { ident, def_id, source_info: self.source_info };
+                    self.potential_lints.push(lint);
+                }
+            }
+            Rvalue::Cast(_, op, _) => {
+                let op_ty = op.ty(self.body, self.tcx);
+                if self.is_fn_ref(op_ty) {
+                    self.casts.push(self.source_info.span);
+                }
+            }
+            _ => {}
+        }
+        self.super_rvalue(rvalue, location);
+    }
+}
+
+impl<'a, 'tcx> FunctionRefChecker<'a, 'tcx> {
+    fn is_fn_ref(&self, ty: Ty<'tcx>) -> bool {
+        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(..) = ref_ty.kind { true } else { false })
+            .unwrap_or(false)
+    }
+}
+
+struct FunctionRefLint {
+    ident: String,
+    def_id: DefId,
+    source_info: SourceInfo,
+}
+
+impl<'tcx> FunctionRefLint {
+    fn emit(&self, tcx: TyCtxt<'tcx>, body: &Body<'tcx>) {
+        let def_id = self.def_id;
+        let source_info = self.source_info;
+        let lint_root = body.source_scopes[source_info.scope]
+            .local_data
+            .as_ref()
+            .assert_crate_local()
+            .lint_root;
+        let fn_sig = tcx.fn_sig(def_id);
+        let unsafety = fn_sig.unsafety().prefix_str();
+        let abi = match fn_sig.abi() {
+            Abi::Rust => String::from(""),
+            other_abi => {
+                let mut s = String::from("extern \"");
+                s.push_str(other_abi.name());
+                s.push_str("\" ");
+                s
+            }
+        };
+        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 { " -> _" };
+        tcx.struct_span_lint_hir(FUNCTION_REFERENCES, lint_root, source_info.span, |lint| {
+            lint.build(&format!(
+                "cast `{}` with `as {}{}fn({}{}){}` to use it as a pointer",
+                self.ident,
+                unsafety,
+                abi,
+                vec!["_"; num_args].join(", "),
+                variadic,
+                ret,
+            ))
+            .emit()
+        });
+    }
+}

compiler/rustc_mir/src/transform/mod.rs

@@ -27,6 +27,7 @@ pub mod dest_prop;
 pub mod dump_mir;
 pub mod early_otherwise_branch;
 pub mod elaborate_drops;
+pub mod function_references;
 pub mod generator;
 pub mod inline;
 pub mod instcombine;
@@ -266,6 +267,7 @@ fn mir_const<'tcx>(
             // MIR-level lints.
             &check_packed_ref::CheckPackedRef,
             &check_const_item_mutation::CheckConstItemMutation,
+            &function_references::FunctionReferences,
             // What we need to do constant evaluation.
             &simplify::SimplifyCfg::new("initial"),
             &rustc_peek::SanityCheck,

compiler/rustc_session/src/lint/builtin.rs

@@ -2645,6 +2645,11 @@ declare_lint! {
         reference: "issue #76200 <https://github.com/rust-lang/rust/issues/76200>",
         edition: None,
     };
+
+declare_lint! {
+    pub FUNCTION_REFERENCES,
+    Warn,
+    "suggest casting functions to pointers when attempting to take references",
 }
 
 declare_lint! {
@@ -2762,6 +2767,7 @@ declare_lint_pass! {
         CONST_EVALUATABLE_UNCHECKED,
         INEFFECTIVE_UNSTABLE_TRAIT_IMPL,
         UNINHABITED_STATIC,
+        FUNCTION_REFERENCES,
     ]
 }
 

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

@@ -1,22 +1,71 @@
 // check-pass
-fn foo() -> usize { 42 }
-fn bar(x: usize) -> usize { x }
-fn baz(x: usize, y: usize) -> usize { x + y }
+#![feature(c_variadic)]
+#![allow(dead_code)]
+
+fn foo() -> u32 { 42 }
+fn bar(x: u32) -> u32 { x }
+fn baz(x: u32, y: u32) -> u32 { x + y }
+unsafe fn unsafe_fn() { }
+extern "C" fn c_fn() { }
+unsafe extern "C" fn unsafe_c_fn() { }
+unsafe extern fn variadic_fn(_x: u32, _args: ...) { }
+fn call_fn(f: &dyn Fn(u32) -> u32, x: u32) { f(x); }
+fn parameterized_call_fn<F: Fn(u32) -> u32>(f: &F, x: u32) { f(x); }
 
 fn main() {
+    let _zst_ref = &foo;
+    //~^ WARN cast `foo` with `as fn() -> _` to use it as a pointer
+    let fn_item = foo;
+    let _indirect_ref = &fn_item;
+    //~^ WARN cast `fn_item` with `as fn() -> _` to use it as a pointer
+    let _cast_zst_ptr = &foo as *const _;
+    //~^ WARN cast `foo` with `as fn() -> _` to use it as a pointer
+    let _coerced_zst_ptr: *const _ = &foo;
+    //~^ WARN cast `foo` with `as fn() -> _` to use it as a pointer
+
+    let _zst_ref = &mut foo;
+    //~^ WARN cast `foo` with `as fn() -> _` to use it as a pointer
+    let mut mut_fn_item = foo;
+    let _indirect_ref = &mut mut_fn_item;
+    //~^ WARN cast `fn_item` with `as fn() -> _` to use it as a pointer
+    let _cast_zst_ptr = &mut foo as *mut _;
+    //~^ WARN cast `foo` with `as fn() -> _` to use it as a pointer
+    let _coerced_zst_ptr: *mut _ = &mut foo;
+    //~^ WARN cast `foo` with `as fn() -> _` to use it as a pointer
+
+    let _cast_zst_ref = &foo as &dyn Fn() -> u32;
+    let _coerced_zst_ref: &dyn Fn() -> u32 = &foo;
+
+    let _cast_zst_ref = &mut foo as &mut dyn Fn() -> u32;
+    let _coerced_zst_ref: &mut dyn Fn() -> u32 = &mut foo;
+    let _fn_ptr = foo as fn() -> u32;
+
     println!("{:p}", &foo);
-    //~^ WARN cast `foo` with `as *const fn() -> _` to use it as a pointer
+    //~^ WARN cast `foo` with as fn() -> _` to use it as a pointer
     println!("{:p}", &bar);
-    //~^ WARN cast `bar` with `as *const fn(_) -> _` to use it as a pointer
+    //~^ WARN cast `bar` with as fn(_) -> _` to use it as a pointer
     println!("{:p}", &baz);
-    //~^ WARN cast `baz` with `as *const fn(_, _) -> _` to use it as a pointer
+    //~^ WARN cast `baz` with as fn(_, _) -> _` to use it as a pointer
+    println!("{:p}", &unsafe_fn);
+    //~^ WARN cast `baz` with as unsafe fn()` to use it as a pointer
+    println!("{:p}", &c_fn);
+    //~^ WARN cast `baz` with as extern "C" fn()` to use it as a pointer
+    println!("{:p}", &unsafe_c_fn);
+    //~^ WARN cast `baz` with as unsafe extern "C" fn()` to use it as a pointer
+    println!("{:p}", &variadic_fn);
+    //~^ WARN cast `baz` with as unsafe extern "C" fn(_, ...) -> _` to use it as a pointer
+    println!("{:p}", &std::env::var::<String>);
+    //~^ WARN cast `std::env::var` with as fn(_) -> _` to use it as a pointer
+
+    println!("{:p}", foo as fn() -> u32);
 
-    //should not produce any warnings
-    println!("{:p}", foo as *const fn() -> usize);
-    println!("{:p}", bar as *const fn(usize) -> usize);
-    println!("{:p}", baz as *const fn(usize, usize) -> usize);
+    unsafe {
+        std::mem::transmute::<_, usize>(&foo);
+        //~^ WARN cast `foo` with as fn() -> _` to use it as a pointer
+        std::mem::transmute::<_, usize>(foo as fn() -> u32);
+    }
 
-    //should not produce any warnings
-    let fn_thing = foo;
-    println!("{:p}", &fn_thing);
+    (&bar)(1);
+    call_fn(&bar, 1);
+    parameterized_call_fn(&bar, 1);
 }