Skip to content

Corrected suggestion for generic parameters in function_item_references lint #78659

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 3, 2020
Merged

Corrected suggestion for generic parameters in function_item_references lint #78659

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 36 additions & 18 deletions compiler/rustc_mir/src/transform/function_item_references.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand All @@ -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.
Expand All @@ -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);
}
}
}
Expand Down Expand Up @@ -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);
}
}
}
Expand All @@ -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 {
Expand All @@ -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) => {
Expand All @@ -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()
Expand All @@ -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 { " -> _" };
Expand All @@ -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(", "),
Expand Down
16 changes: 15 additions & 1 deletion src/test/ui/lint/function-item-references.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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); }
Expand Down Expand Up @@ -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

Expand All @@ -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);
Expand Down
Loading