Skip to content

Commit 935fc36

Browse files
committed
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.
1 parent d6fa7e1 commit 935fc36

File tree

5 files changed

+105
-73
lines changed

5 files changed

+105
-73
lines changed

compiler/rustc_mir/src/transform/function_references.rs renamed to compiler/rustc_mir/src/transform/function_item_references.rs

+50-42
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use rustc_middle::mir::visit::Visitor;
33
use rustc_middle::mir::*;
44
use rustc_middle::ty::{
55
self,
6-
subst::{GenericArgKind, Subst},
6+
subst::{GenericArgKind, Subst, SubstsRef},
77
PredicateAtom, Ty, TyCtxt, TyS,
88
};
99
use rustc_session::lint::builtin::FUNCTION_ITEM_REFERENCES;
@@ -27,6 +27,9 @@ struct FunctionItemRefChecker<'a, 'tcx> {
2727
}
2828

2929
impl<'a, 'tcx> Visitor<'tcx> for FunctionItemRefChecker<'a, 'tcx> {
30+
/// Emits a lint for function reference arguments bound by `fmt::Pointer` or passed to
31+
/// `transmute`. This only handles arguments in calls outside macro expansions to avoid double
32+
/// counting function references formatted as pointers by macros.
3033
fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) {
3134
if let TerminatorKind::Call {
3235
func,
@@ -38,11 +41,11 @@ impl<'a, 'tcx> Visitor<'tcx> for FunctionItemRefChecker<'a, 'tcx> {
3841
} = &terminator.kind
3942
{
4043
let source_info = *self.body.source_info(location);
41-
//this handles all function calls outside macros
44+
// Only handle function calls outside macros
4245
if !source_info.span.from_expansion() {
4346
let func_ty = func.ty(self.body, self.tcx);
4447
if let ty::FnDef(def_id, substs_ref) = *func_ty.kind() {
45-
//handle `std::mem::transmute`
48+
// Handle calls to `transmute`
4649
if self.tcx.is_diagnostic_item(sym::transmute, def_id) {
4750
let arg_ty = args[0].ty(self.body, self.tcx);
4851
for generic_inner_ty in arg_ty.walk() {
@@ -55,48 +58,16 @@ impl<'a, 'tcx> Visitor<'tcx> for FunctionItemRefChecker<'a, 'tcx> {
5558
}
5659
}
5760
} else {
58-
//handle any function call with `std::fmt::Pointer` as a bound trait
59-
//this includes calls to `std::fmt::Pointer::fmt` outside of macros
60-
let param_env = self.tcx.param_env(def_id);
61-
let bounds = param_env.caller_bounds();
62-
for bound in bounds {
63-
if let Some(bound_ty) = self.is_pointer_trait(&bound.skip_binders()) {
64-
//get the argument types as they appear in the function signature
65-
let arg_defs = self.tcx.fn_sig(def_id).skip_binder().inputs();
66-
for (arg_num, arg_def) in arg_defs.iter().enumerate() {
67-
//for all types reachable from the argument type in the fn sig
68-
for generic_inner_ty in arg_def.walk() {
69-
if let GenericArgKind::Type(inner_ty) =
70-
generic_inner_ty.unpack()
71-
{
72-
//if the inner type matches the type bound by `Pointer`
73-
if TyS::same_type(inner_ty, bound_ty) {
74-
//do a substitution using the parameters from the callsite
75-
let subst_ty = inner_ty.subst(self.tcx, substs_ref);
76-
if let Some(fn_id) =
77-
FunctionItemRefChecker::is_fn_ref(subst_ty)
78-
{
79-
let ident =
80-
self.tcx.item_name(fn_id).to_ident_string();
81-
let span = self.nth_arg_span(&args, arg_num);
82-
self.emit_lint(ident, fn_id, source_info, span);
83-
}
84-
}
85-
}
86-
}
87-
}
88-
}
89-
}
61+
self.check_bound_args(def_id, substs_ref, &args, source_info);
9062
}
9163
}
9264
}
9365
}
9466
self.super_terminator(terminator, location);
9567
}
96-
//This handles `std::fmt::Pointer::fmt` when it's used in the formatting macros.
97-
//It's handled as an operand instead of a Call terminator so it won't depend on
98-
//whether the formatting macros call `fmt` directly, transmute it first or other
99-
//internal fmt details.
68+
/// Emits a lint for function references formatted with `fmt::Pointer::fmt` by macros. These
69+
/// cases are handled as operands instead of call terminators to avoid any dependence on
70+
/// unstable, internal formatting details like whether `fmt` is called directly or not.
10071
fn visit_operand(&mut self, operand: &Operand<'tcx>, location: Location) {
10172
let source_info = *self.body.source_info(location);
10273
if source_info.span.from_expansion() {
@@ -105,8 +76,8 @@ impl<'a, 'tcx> Visitor<'tcx> for FunctionItemRefChecker<'a, 'tcx> {
10576
if self.tcx.is_diagnostic_item(sym::pointer_trait_fmt, def_id) {
10677
let param_ty = substs_ref.type_at(0);
10778
if let Some(fn_id) = FunctionItemRefChecker::is_fn_ref(param_ty) {
108-
//the operand's ctxt wouldn't display the lint since it's inside a macro
109-
//so we have to use the callsite's ctxt
79+
// The operand's ctxt wouldn't display the lint since it's inside a macro so
80+
// we have to use the callsite's ctxt.
11081
let callsite_ctxt = source_info.span.source_callsite().ctxt();
11182
let span = source_info.span.with_ctxt(callsite_ctxt);
11283
let ident = self.tcx.item_name(fn_id).to_ident_string();
@@ -120,7 +91,42 @@ impl<'a, 'tcx> Visitor<'tcx> for FunctionItemRefChecker<'a, 'tcx> {
12091
}
12192

12293
impl<'a, 'tcx> FunctionItemRefChecker<'a, 'tcx> {
123-
//return the bound parameter type if the trait is `std::fmt::Pointer`
94+
/// Emits a lint for function reference arguments bound by `fmt::Pointer` in calls to the
95+
/// function defined by `def_id` with the substitutions `substs_ref`.
96+
fn check_bound_args(
97+
&self,
98+
def_id: DefId,
99+
substs_ref: SubstsRef<'tcx>,
100+
args: &Vec<Operand<'tcx>>,
101+
source_info: SourceInfo,
102+
) {
103+
let param_env = self.tcx.param_env(def_id);
104+
let bounds = param_env.caller_bounds();
105+
for bound in bounds {
106+
if let Some(bound_ty) = self.is_pointer_trait(&bound.skip_binders()) {
107+
// Get the argument types as they appear in the function signature.
108+
let arg_defs = self.tcx.fn_sig(def_id).skip_binder().inputs();
109+
for (arg_num, arg_def) in arg_defs.iter().enumerate() {
110+
// For all types reachable from the argument type in the fn sig
111+
for generic_inner_ty in arg_def.walk() {
112+
if let GenericArgKind::Type(inner_ty) = generic_inner_ty.unpack() {
113+
// If the inner type matches the type bound by `Pointer`
114+
if TyS::same_type(inner_ty, bound_ty) {
115+
// Do a substitution using the parameters from the callsite
116+
let subst_ty = inner_ty.subst(self.tcx, substs_ref);
117+
if let Some(fn_id) = FunctionItemRefChecker::is_fn_ref(subst_ty) {
118+
let ident = self.tcx.item_name(fn_id).to_ident_string();
119+
let span = self.nth_arg_span(args, arg_num);
120+
self.emit_lint(ident, fn_id, source_info, span);
121+
}
122+
}
123+
}
124+
}
125+
}
126+
}
127+
}
128+
}
129+
/// If the given predicate is the trait `fmt::Pointer`, returns the bound parameter type.
124130
fn is_pointer_trait(&self, bound: &PredicateAtom<'tcx>) -> Option<Ty<'tcx>> {
125131
if let ty::PredicateAtom::Trait(predicate, _) = bound {
126132
if self.tcx.is_diagnostic_item(sym::pointer_trait, predicate.def_id()) {
@@ -132,6 +138,8 @@ impl<'a, 'tcx> FunctionItemRefChecker<'a, 'tcx> {
132138
None
133139
}
134140
}
141+
/// If a type is a reference or raw pointer to the anonymous type of a function definition,
142+
/// returns that function's `DefId`.
135143
fn is_fn_ref(ty: Ty<'tcx>) -> Option<DefId> {
136144
let referent_ty = match ty.kind() {
137145
ty::Ref(_, referent_ty, _) => Some(referent_ty),

compiler/rustc_mir/src/transform/mod.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ pub mod dest_prop;
2727
pub mod dump_mir;
2828
pub mod early_otherwise_branch;
2929
pub mod elaborate_drops;
30-
pub mod function_references;
30+
pub mod function_item_references;
3131
pub mod generator;
3232
pub mod inline;
3333
pub mod instcombine;
@@ -267,7 +267,7 @@ fn mir_const<'tcx>(
267267
// MIR-level lints.
268268
&check_packed_ref::CheckPackedRef,
269269
&check_const_item_mutation::CheckConstItemMutation,
270-
&function_references::FunctionItemReferences,
270+
&function_item_references::FunctionItemReferences,
271271
// What we need to do constant evaluation.
272272
&simplify::SimplifyCfg::new("initial"),
273273
&rustc_peek::SanityCheck,

compiler/rustc_session/src/lint/builtin.rs

+24
Original file line numberDiff line numberDiff line change
@@ -2648,6 +2648,30 @@ declare_lint! {
26482648
}
26492649

26502650
declare_lint! {
2651+
/// The `function_item_references` lint detects function references that are
2652+
/// formatted with [`fmt::Pointer`] or transmuted.
2653+
///
2654+
/// [`fmt::Pointer`]: https://doc.rust-lang.org/std/fmt/trait.Pointer.html
2655+
///
2656+
/// ### Example
2657+
///
2658+
/// ```rust
2659+
/// fn foo() { }
2660+
///
2661+
/// fn main() {
2662+
/// println!("{:p}", &foo);
2663+
/// }
2664+
/// ```
2665+
///
2666+
/// {{produces}}
2667+
///
2668+
/// ### Explanation
2669+
///
2670+
/// Taking a reference to a function may be mistaken as a way to obtain a
2671+
/// pointer to that function. This can give unexpected results when
2672+
/// formatting the reference as a pointer or transmuting it. This lint is
2673+
/// issued when function references are formatted as pointers, passed as
2674+
/// arguments bound by [`fmt::Pointer`] or transmuted.
26512675
pub FUNCTION_ITEM_REFERENCES,
26522676
Warn,
26532677
"suggest casting functions to pointers when attempting to take references",

0 commit comments

Comments
 (0)