Corrected suggestion for generic parameters in function_item_references lint

This lint was incorrectly suggesting casting a function to a pointer without
specifying generic type parameters or const generics. This would cause a
compiler error since the missing parameters couldn't be inferred. This commit
fixed the suggestion and added a few tests with generics.
This commit is contained in:
Ayrton 2020-11-01 23:14:21 -04:00
parent 3d0682b97a
commit ace02c40f0
3 changed files with 112 additions and 50 deletions

View File

@ -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(", "),

View File

@ -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);

View File

@ -1,5 +1,5 @@
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:40:18
--> $DIR/function-item-references.rs:44:18
|
LL | Pointer::fmt(&zst_ref, f)
| ^^^^^^^^ help: cast `foo` to obtain a function pointer: `foo as fn() -> _`
@ -11,166 +11,196 @@ LL | #![warn(function_item_references)]
| ^^^^^^^^^^^^^^^^^^^^^^^^
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:77:22
--> $DIR/function-item-references.rs:81:22
|
LL | println!("{:p}", &foo);
| ^^^^ help: cast `foo` to obtain a function pointer: `foo as fn() -> _`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:79:20
--> $DIR/function-item-references.rs:83:20
|
LL | print!("{:p}", &foo);
| ^^^^ help: cast `foo` to obtain a function pointer: `foo as fn() -> _`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:81:21
--> $DIR/function-item-references.rs:85:21
|
LL | format!("{:p}", &foo);
| ^^^^ help: cast `foo` to obtain a function pointer: `foo as fn() -> _`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:84:22
--> $DIR/function-item-references.rs:88:22
|
LL | println!("{:p}", &foo as *const _);
| ^^^^^^^^^^^^^^^^ help: cast `foo` to obtain a function pointer: `foo as fn() -> _`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:86:22
--> $DIR/function-item-references.rs:90:22
|
LL | println!("{:p}", zst_ref);
| ^^^^^^^ help: cast `foo` to obtain a function pointer: `foo as fn() -> _`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:88:22
--> $DIR/function-item-references.rs:92:22
|
LL | println!("{:p}", cast_zst_ptr);
| ^^^^^^^^^^^^ help: cast `foo` to obtain a function pointer: `foo as fn() -> _`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:90:22
--> $DIR/function-item-references.rs:94:22
|
LL | println!("{:p}", coerced_zst_ptr);
| ^^^^^^^^^^^^^^^ help: cast `foo` to obtain a function pointer: `foo as fn() -> _`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:93:22
--> $DIR/function-item-references.rs:97:22
|
LL | println!("{:p}", &fn_item);
| ^^^^^^^^ help: cast `foo` to obtain a function pointer: `foo as fn() -> _`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:95:22
--> $DIR/function-item-references.rs:99:22
|
LL | println!("{:p}", indirect_ref);
| ^^^^^^^^^^^^ help: cast `foo` to obtain a function pointer: `foo as fn() -> _`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:98:22
--> $DIR/function-item-references.rs:102:22
|
LL | println!("{:p}", &nop);
| ^^^^ help: cast `nop` to obtain a function pointer: `nop as fn()`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:100:22
--> $DIR/function-item-references.rs:104:22
|
LL | println!("{:p}", &bar);
| ^^^^ help: cast `bar` to obtain a function pointer: `bar as fn(_) -> _`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:102:22
--> $DIR/function-item-references.rs:106:22
|
LL | println!("{:p}", &baz);
| ^^^^ help: cast `baz` to obtain a function pointer: `baz as fn(_, _) -> _`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:104:22
--> $DIR/function-item-references.rs:108:22
|
LL | println!("{:p}", &unsafe_fn);
| ^^^^^^^^^^ help: cast `unsafe_fn` to obtain a function pointer: `unsafe_fn as unsafe fn()`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:106:22
--> $DIR/function-item-references.rs:110:22
|
LL | println!("{:p}", &c_fn);
| ^^^^^ help: cast `c_fn` to obtain a function pointer: `c_fn as extern "C" fn()`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:108:22
--> $DIR/function-item-references.rs:112:22
|
LL | println!("{:p}", &unsafe_c_fn);
| ^^^^^^^^^^^^ help: cast `unsafe_c_fn` to obtain a function pointer: `unsafe_c_fn as unsafe extern "C" fn()`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:110:22
--> $DIR/function-item-references.rs:114:22
|
LL | println!("{:p}", &variadic);
| ^^^^^^^^^ help: cast `variadic` to obtain a function pointer: `variadic as unsafe extern "C" fn(_, ...)`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:112:22
--> $DIR/function-item-references.rs:116:22
|
LL | println!("{:p}", &std::env::var::<String>);
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: cast `var` to obtain a function pointer: `var as fn(_) -> _`
LL | println!("{:p}", &take_generic_ref::<u32>);
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: cast `take_generic_ref` to obtain a function pointer: `take_generic_ref::<u32> as fn(_)`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:115:32
--> $DIR/function-item-references.rs:118:22
|
LL | println!("{:p}", &take_generic_array::<u32, 4>);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: cast `take_generic_array` to obtain a function pointer: `take_generic_array::<u32, 4_usize> as fn(_)`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:120:22
|
LL | println!("{:p}", &multiple_generic::<u32, f32>);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: cast `multiple_generic` to obtain a function pointer: `multiple_generic::<u32, f32> as fn(_, _)`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:122:22
|
LL | println!("{:p}", &multiple_generic_arrays::<u32, f32, 4, 8>);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: cast `multiple_generic_arrays` to obtain a function pointer: `multiple_generic_arrays::<u32, f32, 4_usize, 8_usize> as fn(_, _)`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:124:22
|
LL | println!("{:p}", &std::env::var::<String>);
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: cast `var` to obtain a function pointer: `var::<String> as fn(_) -> _`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:127:32
|
LL | println!("{:p} {:p} {:p}", &nop, &foo, &bar);
| ^^^^ help: cast `nop` to obtain a function pointer: `nop as fn()`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:115:38
--> $DIR/function-item-references.rs:127:38
|
LL | println!("{:p} {:p} {:p}", &nop, &foo, &bar);
| ^^^^ help: cast `foo` to obtain a function pointer: `foo as fn() -> _`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:115:44
--> $DIR/function-item-references.rs:127:44
|
LL | println!("{:p} {:p} {:p}", &nop, &foo, &bar);
| ^^^^ help: cast `bar` to obtain a function pointer: `bar as fn(_) -> _`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:130:41
--> $DIR/function-item-references.rs:142:41
|
LL | std::mem::transmute::<_, usize>(&foo);
| ^^^^ help: cast `foo` to obtain a function pointer: `foo as fn() -> _`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:132:50
--> $DIR/function-item-references.rs:144:50
|
LL | std::mem::transmute::<_, (usize, usize)>((&foo, &bar));
| ^^^^^^^^^^^^ help: cast `foo` to obtain a function pointer: `foo as fn() -> _`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:132:50
--> $DIR/function-item-references.rs:144:50
|
LL | std::mem::transmute::<_, (usize, usize)>((&foo, &bar));
| ^^^^^^^^^^^^ help: cast `bar` to obtain a function pointer: `bar as fn(_) -> _`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:142:15
--> $DIR/function-item-references.rs:147:41
|
LL | std::mem::transmute::<_, usize>(&take_generic_ref::<u32>);
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: cast `take_generic_ref` to obtain a function pointer: `take_generic_ref::<u32> as fn(_)`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:156:15
|
LL | print_ptr(&bar);
| ^^^^ help: cast `bar` to obtain a function pointer: `bar as fn(_) -> _`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:144:24
--> $DIR/function-item-references.rs:158:24
|
LL | bound_by_ptr_trait(&bar);
| ^^^^ help: cast `bar` to obtain a function pointer: `bar as fn(_) -> _`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:146:30
--> $DIR/function-item-references.rs:160:30
|
LL | bound_by_ptr_trait_tuple((&foo, &bar));
| ^^^^^^^^^^^^ help: cast `bar` to obtain a function pointer: `bar as fn(_) -> _`
warning: taking a reference to a function item does not give a function pointer
--> $DIR/function-item-references.rs:146:30
--> $DIR/function-item-references.rs:160:30
|
LL | bound_by_ptr_trait_tuple((&foo, &bar));
| ^^^^^^^^^^^^ help: cast `foo` to obtain a function pointer: `foo as fn() -> _`
warning: 28 warnings emitted
warning: 33 warnings emitted