about summary refs log tree commit diff
diff options
context:
space:
mode:
authorAyrton <a.munoz3327@gmail.com>2020-10-21 17:19:21 -0400
committerAyrton <a.munoz3327@gmail.com>2020-10-27 11:04:04 -0400
commit935fc3642a61e1545e1b98bf4716acbb7bf12e91 (patch)
tree2b7c3e0fd656b9c28fdcac2baf5ce829624d3997
parentd6fa7e15d6679a1c01a6b4c32400f571d04c6a6c (diff)
downloadrust-935fc3642a61e1545e1b98bf4716acbb7bf12e91.tar.gz
rust-935fc3642a61e1545e1b98bf4716acbb7bf12e91.zip
Added documentation for `function_item_references` lint
Added documentation for `function_item_references` lint to the rustc book and
fixed comments in the lint checker itself.
-rw-r--r--compiler/rustc_mir/src/transform/function_item_references.rs (renamed from compiler/rustc_mir/src/transform/function_references.rs)92
-rw-r--r--compiler/rustc_mir/src/transform/mod.rs4
-rw-r--r--compiler/rustc_session/src/lint/builtin.rs24
-rw-r--r--src/test/ui/lint/function-item-references.rs (renamed from src/test/ui/lint/function-references.rs)0
-rw-r--r--src/test/ui/lint/function-item-references.stderr (renamed from src/test/ui/lint/function-references.stderr)58
5 files changed, 105 insertions, 73 deletions
diff --git a/compiler/rustc_mir/src/transform/function_references.rs b/compiler/rustc_mir/src/transform/function_item_references.rs
index 6e94fad5d0d..3a30b41d681 100644
--- a/compiler/rustc_mir/src/transform/function_references.rs
+++ b/compiler/rustc_mir/src/transform/function_item_references.rs
@@ -3,7 +3,7 @@ use rustc_middle::mir::visit::Visitor;
 use rustc_middle::mir::*;
 use rustc_middle::ty::{
     self,
-    subst::{GenericArgKind, Subst},
+    subst::{GenericArgKind, Subst, SubstsRef},
     PredicateAtom, Ty, TyCtxt, TyS,
 };
 use rustc_session::lint::builtin::FUNCTION_ITEM_REFERENCES;
@@ -27,6 +27,9 @@ struct FunctionItemRefChecker<'a, 'tcx> {
 }
 
 impl<'a, 'tcx> Visitor<'tcx> for FunctionItemRefChecker<'a, 'tcx> {
+    /// Emits a lint for function reference arguments bound by `fmt::Pointer` or passed to
+    /// `transmute`. This only handles arguments in calls outside macro expansions to avoid double
+    /// counting function references formatted as pointers by macros.
     fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) {
         if let TerminatorKind::Call {
             func,
@@ -38,11 +41,11 @@ impl<'a, 'tcx> Visitor<'tcx> for FunctionItemRefChecker<'a, 'tcx> {
         } = &terminator.kind
         {
             let source_info = *self.body.source_info(location);
-            //this handles all function calls outside macros
+            // Only handle function calls outside macros
             if !source_info.span.from_expansion() {
                 let func_ty = func.ty(self.body, self.tcx);
                 if let ty::FnDef(def_id, substs_ref) = *func_ty.kind() {
-                    //handle `std::mem::transmute`
+                    // Handle calls to `transmute`
                     if self.tcx.is_diagnostic_item(sym::transmute, def_id) {
                         let arg_ty = args[0].ty(self.body, self.tcx);
                         for generic_inner_ty in arg_ty.walk() {
@@ -55,48 +58,16 @@ impl<'a, 'tcx> Visitor<'tcx> for FunctionItemRefChecker<'a, 'tcx> {
                             }
                         }
                     } else {
-                        //handle any function call with `std::fmt::Pointer` as a bound trait
-                        //this includes calls to `std::fmt::Pointer::fmt` outside of macros
-                        let param_env = self.tcx.param_env(def_id);
-                        let bounds = param_env.caller_bounds();
-                        for bound in bounds {
-                            if let Some(bound_ty) = self.is_pointer_trait(&bound.skip_binders()) {
-                                //get the argument types as they appear in the function signature
-                                let arg_defs = self.tcx.fn_sig(def_id).skip_binder().inputs();
-                                for (arg_num, arg_def) in arg_defs.iter().enumerate() {
-                                    //for all types reachable from the argument type in the fn sig
-                                    for generic_inner_ty in arg_def.walk() {
-                                        if let GenericArgKind::Type(inner_ty) =
-                                            generic_inner_ty.unpack()
-                                        {
-                                            //if the inner type matches the type bound by `Pointer`
-                                            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();
-                                                    let span = self.nth_arg_span(&args, arg_num);
-                                                    self.emit_lint(ident, fn_id, source_info, span);
-                                                }
-                                            }
-                                        }
-                                    }
-                                }
-                            }
-                        }
+                        self.check_bound_args(def_id, substs_ref, &args, source_info);
                     }
                 }
             }
         }
         self.super_terminator(terminator, location);
     }
-    //This handles `std::fmt::Pointer::fmt` when it's used in the formatting macros.
-    //It's handled as an operand instead of a Call terminator so it won't depend on
-    //whether the formatting macros call `fmt` directly, transmute it first or other
-    //internal fmt details.
+    /// 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.
     fn visit_operand(&mut self, operand: &Operand<'tcx>, location: Location) {
         let source_info = *self.body.source_info(location);
         if source_info.span.from_expansion() {
@@ -105,8 +76,8 @@ impl<'a, 'tcx> Visitor<'tcx> for FunctionItemRefChecker<'a, 'tcx> {
                 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) {
-                        //the operand's ctxt wouldn't display the lint since it's inside a macro
-                        //so we have to use the callsite's ctxt
+                        // 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();
@@ -120,7 +91,42 @@ impl<'a, 'tcx> Visitor<'tcx> for FunctionItemRefChecker<'a, 'tcx> {
 }
 
 impl<'a, 'tcx> FunctionItemRefChecker<'a, 'tcx> {
-    //return the bound parameter type if the trait is `std::fmt::Pointer`
+    /// Emits a lint for function reference arguments bound by `fmt::Pointer` in calls to the
+    /// function defined by `def_id` with the substitutions `substs_ref`.
+    fn check_bound_args(
+        &self,
+        def_id: DefId,
+        substs_ref: SubstsRef<'tcx>,
+        args: &Vec<Operand<'tcx>>,
+        source_info: SourceInfo,
+    ) {
+        let param_env = self.tcx.param_env(def_id);
+        let bounds = param_env.caller_bounds();
+        for bound in bounds {
+            if let Some(bound_ty) = self.is_pointer_trait(&bound.skip_binders()) {
+                // Get the argument types as they appear in the function signature.
+                let arg_defs = self.tcx.fn_sig(def_id).skip_binder().inputs();
+                for (arg_num, arg_def) in arg_defs.iter().enumerate() {
+                    // For all types reachable from the argument type in the fn sig
+                    for generic_inner_ty in arg_def.walk() {
+                        if let GenericArgKind::Type(inner_ty) = generic_inner_ty.unpack() {
+                            // If the inner type matches the type bound by `Pointer`
+                            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();
+                                    let span = self.nth_arg_span(args, arg_num);
+                                    self.emit_lint(ident, fn_id, source_info, span);
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+        }
+    }
+    /// 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 {
             if self.tcx.is_diagnostic_item(sym::pointer_trait, predicate.def_id()) {
@@ -132,6 +138,8 @@ 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> {
         let referent_ty = match ty.kind() {
             ty::Ref(_, referent_ty, _) => Some(referent_ty),
diff --git a/compiler/rustc_mir/src/transform/mod.rs b/compiler/rustc_mir/src/transform/mod.rs
index e43a238c1ba..89db6bb13ca 100644
--- a/compiler/rustc_mir/src/transform/mod.rs
+++ b/compiler/rustc_mir/src/transform/mod.rs
@@ -27,7 +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 function_item_references;
 pub mod generator;
 pub mod inline;
 pub mod instcombine;
@@ -267,7 +267,7 @@ fn mir_const<'tcx>(
             // MIR-level lints.
             &check_packed_ref::CheckPackedRef,
             &check_const_item_mutation::CheckConstItemMutation,
-            &function_references::FunctionItemReferences,
+            &function_item_references::FunctionItemReferences,
             // What we need to do constant evaluation.
             &simplify::SimplifyCfg::new("initial"),
             &rustc_peek::SanityCheck,
diff --git a/compiler/rustc_session/src/lint/builtin.rs b/compiler/rustc_session/src/lint/builtin.rs
index 41abdacb54c..806f79af277 100644
--- a/compiler/rustc_session/src/lint/builtin.rs
+++ b/compiler/rustc_session/src/lint/builtin.rs
@@ -2648,6 +2648,30 @@ declare_lint! {
 }
 
 declare_lint! {
+    /// The `function_item_references` lint detects function references that are
+    /// formatted with [`fmt::Pointer`] or transmuted.
+    ///
+    /// [`fmt::Pointer`]: https://doc.rust-lang.org/std/fmt/trait.Pointer.html
+    ///
+    /// ### Example
+    ///
+    /// ```rust
+    /// fn foo() { }
+    ///
+    /// fn main() {
+    ///     println!("{:p}", &foo);
+    /// }
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// Taking a reference to a function may be mistaken as a way to obtain a
+    /// pointer to that function. This can give unexpected results when
+    /// formatting the reference as a pointer or transmuting it. This lint is
+    /// issued when function references are formatted as pointers, passed as
+    /// arguments bound by [`fmt::Pointer`] or transmuted.
     pub FUNCTION_ITEM_REFERENCES,
     Warn,
     "suggest casting functions to pointers when attempting to take references",
diff --git a/src/test/ui/lint/function-references.rs b/src/test/ui/lint/function-item-references.rs
index 28c66a31322..28c66a31322 100644
--- a/src/test/ui/lint/function-references.rs
+++ b/src/test/ui/lint/function-item-references.rs
diff --git a/src/test/ui/lint/function-references.stderr b/src/test/ui/lint/function-item-references.stderr
index 5e399c16305..a96b134ed58 100644
--- a/src/test/ui/lint/function-references.stderr
+++ b/src/test/ui/lint/function-item-references.stderr
@@ -1,173 +1,173 @@
 warning: cast `foo` with `as fn() -> _` to obtain a function pointer
-  --> $DIR/function-references.rs:40:18
+  --> $DIR/function-item-references.rs:40:18
    |
 LL |     Pointer::fmt(&zst_ref, f)
    |                  ^^^^^^^^
    |
 note: the lint level is defined here
-  --> $DIR/function-references.rs:3:9
+  --> $DIR/function-item-references.rs:3:9
    |
 LL | #![warn(function_item_references)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^
 
 warning: cast `foo` with `as fn() -> _` to obtain a function pointer
-  --> $DIR/function-references.rs:77:22
+  --> $DIR/function-item-references.rs:77:22
    |
 LL |     println!("{:p}", &foo);
    |                      ^^^^
 
 warning: cast `foo` with `as fn() -> _` to obtain a function pointer
-  --> $DIR/function-references.rs:79:20
+  --> $DIR/function-item-references.rs:79:20
    |
 LL |     print!("{:p}", &foo);
    |                    ^^^^
 
 warning: cast `foo` with `as fn() -> _` to obtain a function pointer
-  --> $DIR/function-references.rs:81:21
+  --> $DIR/function-item-references.rs:81:21
    |
 LL |     format!("{:p}", &foo);
    |                     ^^^^
 
 warning: cast `foo` with `as fn() -> _` to obtain a function pointer
-  --> $DIR/function-references.rs:84:22
+  --> $DIR/function-item-references.rs:84:22
    |
 LL |     println!("{:p}", &foo as *const _);
    |                      ^^^^^^^^^^^^^^^^
 
 warning: cast `foo` with `as fn() -> _` to obtain a function pointer
-  --> $DIR/function-references.rs:86:22
+  --> $DIR/function-item-references.rs:86:22
    |
 LL |     println!("{:p}", zst_ref);
    |                      ^^^^^^^
 
 warning: cast `foo` with `as fn() -> _` to obtain a function pointer
-  --> $DIR/function-references.rs:88:22
+  --> $DIR/function-item-references.rs:88:22
    |
 LL |     println!("{:p}", cast_zst_ptr);
    |                      ^^^^^^^^^^^^
 
 warning: cast `foo` with `as fn() -> _` to obtain a function pointer
-  --> $DIR/function-references.rs:90:22
+  --> $DIR/function-item-references.rs:90:22
    |
 LL |     println!("{:p}", coerced_zst_ptr);
    |                      ^^^^^^^^^^^^^^^
 
 warning: cast `foo` with `as fn() -> _` to obtain a function pointer
-  --> $DIR/function-references.rs:93:22
+  --> $DIR/function-item-references.rs:93:22
    |
 LL |     println!("{:p}", &fn_item);
    |                      ^^^^^^^^
 
 warning: cast `foo` with `as fn() -> _` to obtain a function pointer
-  --> $DIR/function-references.rs:95:22
+  --> $DIR/function-item-references.rs:95:22
    |
 LL |     println!("{:p}", indirect_ref);
    |                      ^^^^^^^^^^^^
 
 warning: cast `nop` with `as fn()` to obtain a function pointer
-  --> $DIR/function-references.rs:98:22
+  --> $DIR/function-item-references.rs:98:22
    |
 LL |     println!("{:p}", &nop);
    |                      ^^^^
 
 warning: cast `bar` with `as fn(_) -> _` to obtain a function pointer
-  --> $DIR/function-references.rs:100:22
+  --> $DIR/function-item-references.rs:100:22
    |
 LL |     println!("{:p}", &bar);
    |                      ^^^^
 
 warning: cast `baz` with `as fn(_, _) -> _` to obtain a function pointer
-  --> $DIR/function-references.rs:102:22
+  --> $DIR/function-item-references.rs:102:22
    |
 LL |     println!("{:p}", &baz);
    |                      ^^^^
 
 warning: cast `unsafe_fn` with `as unsafe fn()` to obtain a function pointer
-  --> $DIR/function-references.rs:104:22
+  --> $DIR/function-item-references.rs:104:22
    |
 LL |     println!("{:p}", &unsafe_fn);
    |                      ^^^^^^^^^^
 
 warning: cast `c_fn` with `as extern "C" fn()` to obtain a function pointer
-  --> $DIR/function-references.rs:106:22
+  --> $DIR/function-item-references.rs:106:22
    |
 LL |     println!("{:p}", &c_fn);
    |                      ^^^^^
 
 warning: cast `unsafe_c_fn` with `as unsafe extern "C" fn()` to obtain a function pointer
-  --> $DIR/function-references.rs:108:22
+  --> $DIR/function-item-references.rs:108:22
    |
 LL |     println!("{:p}", &unsafe_c_fn);
    |                      ^^^^^^^^^^^^
 
 warning: cast `variadic` with `as unsafe extern "C" fn(_, ...)` to obtain a function pointer
-  --> $DIR/function-references.rs:110:22
+  --> $DIR/function-item-references.rs:110:22
    |
 LL |     println!("{:p}", &variadic);
    |                      ^^^^^^^^^
 
 warning: cast `var` with `as fn(_) -> _` to obtain a function pointer
-  --> $DIR/function-references.rs:112:22
+  --> $DIR/function-item-references.rs:112:22
    |
 LL |     println!("{:p}", &std::env::var::<String>);
    |                      ^^^^^^^^^^^^^^^^^^^^^^^^
 
 warning: cast `nop` with `as fn()` to obtain a function pointer
-  --> $DIR/function-references.rs:115:32
+  --> $DIR/function-item-references.rs:115:32
    |
 LL |     println!("{:p} {:p} {:p}", &nop, &foo, &bar);
    |                                ^^^^
 
 warning: cast `foo` with `as fn() -> _` to obtain a function pointer
-  --> $DIR/function-references.rs:115:38
+  --> $DIR/function-item-references.rs:115:38
    |
 LL |     println!("{:p} {:p} {:p}", &nop, &foo, &bar);
    |                                      ^^^^
 
 warning: cast `bar` with `as fn(_) -> _` to obtain a function pointer
-  --> $DIR/function-references.rs:115:44
+  --> $DIR/function-item-references.rs:115:44
    |
 LL |     println!("{:p} {:p} {:p}", &nop, &foo, &bar);
    |                                            ^^^^
 
 warning: cast `foo` with `as fn() -> _` to obtain a function pointer
-  --> $DIR/function-references.rs:130:41
+  --> $DIR/function-item-references.rs:130:41
    |
 LL |         std::mem::transmute::<_, usize>(&foo);
    |                                         ^^^^
 
 warning: cast `foo` with `as fn() -> _` to obtain a function pointer
-  --> $DIR/function-references.rs:132:50
+  --> $DIR/function-item-references.rs:132:50
    |
 LL |         std::mem::transmute::<_, (usize, usize)>((&foo, &bar));
    |                                                  ^^^^^^^^^^^^
 
 warning: cast `bar` with `as fn(_) -> _` to obtain a function pointer
-  --> $DIR/function-references.rs:132:50
+  --> $DIR/function-item-references.rs:132:50
    |
 LL |         std::mem::transmute::<_, (usize, usize)>((&foo, &bar));
    |                                                  ^^^^^^^^^^^^
 
 warning: cast `bar` with `as fn(_) -> _` to obtain a function pointer
-  --> $DIR/function-references.rs:142:15
+  --> $DIR/function-item-references.rs:142:15
    |
 LL |     print_ptr(&bar);
    |               ^^^^
 
 warning: cast `bar` with `as fn(_) -> _` to obtain a function pointer
-  --> $DIR/function-references.rs:144:24
+  --> $DIR/function-item-references.rs:144:24
    |
 LL |     bound_by_ptr_trait(&bar);
    |                        ^^^^
 
 warning: cast `bar` with `as fn(_) -> _` to obtain a function pointer
-  --> $DIR/function-references.rs:146:30
+  --> $DIR/function-item-references.rs:146:30
    |
 LL |     bound_by_ptr_trait_tuple((&foo, &bar));
    |                              ^^^^^^^^^^^^
 
 warning: cast `foo` with `as fn() -> _` to obtain a function pointer
-  --> $DIR/function-references.rs:146:30
+  --> $DIR/function-item-references.rs:146:30
    |
 LL |     bound_by_ptr_trait_tuple((&foo, &bar));
    |                              ^^^^^^^^^^^^