Fixed compiler error in lint checker triggered by associated types

When a function argument bound by `Pointer` is an associated type, we only
perform substitutions using the parameters from the callsite but don't attempt
to normalize since it may not succeed. A simplified version of the scenario that
triggered this error was added as a test case. Also fixed `Pointer::fmt` which
was being double-counted when called outside of macros and added a test case for
this.
This commit is contained in:
Ayrton 2020-10-06 17:55:46 -04:00
parent 432ebd57ef
commit d6fa7e15d6
3 changed files with 122 additions and 83 deletions

View File

@ -1,7 +1,11 @@
use rustc_hir::def_id::DefId; use rustc_hir::def_id::DefId;
use rustc_middle::mir::visit::Visitor; use rustc_middle::mir::visit::Visitor;
use rustc_middle::mir::*; use rustc_middle::mir::*;
use rustc_middle::ty::{self, subst::GenericArgKind, PredicateAtom, Ty, TyCtxt, TyS}; use rustc_middle::ty::{
self,
subst::{GenericArgKind, Subst},
PredicateAtom, Ty, TyCtxt, TyS,
};
use rustc_session::lint::builtin::FUNCTION_ITEM_REFERENCES; use rustc_session::lint::builtin::FUNCTION_ITEM_REFERENCES;
use rustc_span::{symbol::sym, Span}; use rustc_span::{symbol::sym, Span};
use rustc_target::spec::abi::Abi; use rustc_target::spec::abi::Abi;
@ -33,46 +37,47 @@ impl<'a, 'tcx> Visitor<'tcx> for FunctionItemRefChecker<'a, 'tcx> {
fn_span: _, fn_span: _,
} = &terminator.kind } = &terminator.kind
{ {
let source_info = *self.body.source_info(location);
//this handles all function calls outside macros
if !source_info.span.from_expansion() {
let func_ty = func.ty(self.body, self.tcx); let func_ty = func.ty(self.body, self.tcx);
if let ty::FnDef(def_id, substs_ref) = *func_ty.kind() { if let ty::FnDef(def_id, substs_ref) = *func_ty.kind() {
//check arguments for `std::mem::transmute` //handle `std::mem::transmute`
if self.tcx.is_diagnostic_item(sym::transmute, def_id) { if self.tcx.is_diagnostic_item(sym::transmute, def_id) {
let arg_ty = args[0].ty(self.body, self.tcx); let arg_ty = args[0].ty(self.body, self.tcx);
for generic_inner_ty in arg_ty.walk() { for generic_inner_ty in arg_ty.walk() {
if let GenericArgKind::Type(inner_ty) = generic_inner_ty.unpack() { if let GenericArgKind::Type(inner_ty) = generic_inner_ty.unpack() {
if let Some(fn_id) = FunctionItemRefChecker::is_fn_ref(inner_ty) { if let Some(fn_id) = FunctionItemRefChecker::is_fn_ref(inner_ty) {
let ident = self.tcx.item_name(fn_id).to_ident_string(); let ident = self.tcx.item_name(fn_id).to_ident_string();
let source_info = *self.body.source_info(location);
let span = self.nth_arg_span(&args, 0); let span = self.nth_arg_span(&args, 0);
self.emit_lint(ident, fn_id, source_info, span); self.emit_lint(ident, fn_id, source_info, span);
} }
} }
} }
} else { } else {
//check arguments for any function with `std::fmt::Pointer` as a bound trait //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 param_env = self.tcx.param_env(def_id);
let bounds = param_env.caller_bounds(); let bounds = param_env.caller_bounds();
for bound in bounds { for bound in bounds {
if let Some(bound_ty) = self.is_pointer_trait(&bound.skip_binders()) { 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(); let arg_defs = self.tcx.fn_sig(def_id).skip_binder().inputs();
for (arg_num, arg_def) in arg_defs.iter().enumerate() { 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() { for generic_inner_ty in arg_def.walk() {
if let GenericArgKind::Type(inner_ty) = if let GenericArgKind::Type(inner_ty) =
generic_inner_ty.unpack() generic_inner_ty.unpack()
{ {
//if any type reachable from the argument types in the fn sig matches the type bound by `Pointer` //if the inner type matches the type bound by `Pointer`
if TyS::same_type(inner_ty, bound_ty) { if TyS::same_type(inner_ty, bound_ty) {
//check if this type is a function reference in the function call //do a substitution using the parameters from the callsite
let norm_ty = let subst_ty = inner_ty.subst(self.tcx, substs_ref);
self.tcx.subst_and_normalize_erasing_regions(
substs_ref, param_env, &inner_ty,
);
if let Some(fn_id) = if let Some(fn_id) =
FunctionItemRefChecker::is_fn_ref(norm_ty) FunctionItemRefChecker::is_fn_ref(subst_ty)
{ {
let ident = let ident =
self.tcx.item_name(fn_id).to_ident_string(); self.tcx.item_name(fn_id).to_ident_string();
let source_info = *self.body.source_info(location);
let span = self.nth_arg_span(&args, arg_num); let span = self.nth_arg_span(&args, arg_num);
self.emit_lint(ident, fn_id, source_info, span); self.emit_lint(ident, fn_id, source_info, span);
} }
@ -85,17 +90,23 @@ impl<'a, 'tcx> Visitor<'tcx> for FunctionItemRefChecker<'a, 'tcx> {
} }
} }
} }
}
self.super_terminator(terminator, location); self.super_terminator(terminator, location);
} }
//check for `std::fmt::Pointer::<T>::fmt` where T is a function reference //This handles `std::fmt::Pointer::fmt` when it's used in the formatting macros.
//this is used in formatting macros, but doesn't rely on the specific expansion //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.
fn visit_operand(&mut self, operand: &Operand<'tcx>, location: Location) { fn visit_operand(&mut self, operand: &Operand<'tcx>, location: Location) {
let source_info = *self.body.source_info(location);
if source_info.span.from_expansion() {
let op_ty = operand.ty(self.body, self.tcx); let op_ty = operand.ty(self.body, self.tcx);
if let ty::FnDef(def_id, substs_ref) = *op_ty.kind() { if let ty::FnDef(def_id, substs_ref) = *op_ty.kind() {
if self.tcx.is_diagnostic_item(sym::pointer_trait_fmt, def_id) { if self.tcx.is_diagnostic_item(sym::pointer_trait_fmt, def_id) {
let param_ty = substs_ref.type_at(0); let param_ty = substs_ref.type_at(0);
if let Some(fn_id) = FunctionItemRefChecker::is_fn_ref(param_ty) { if let Some(fn_id) = FunctionItemRefChecker::is_fn_ref(param_ty) {
let source_info = *self.body.source_info(location); //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 callsite_ctxt = source_info.span.source_callsite().ctxt();
let span = source_info.span.with_ctxt(callsite_ctxt); let span = source_info.span.with_ctxt(callsite_ctxt);
let ident = self.tcx.item_name(fn_id).to_ident_string(); let ident = self.tcx.item_name(fn_id).to_ident_string();
@ -103,6 +114,7 @@ impl<'a, 'tcx> Visitor<'tcx> for FunctionItemRefChecker<'a, 'tcx> {
} }
} }
} }
}
self.super_operand(operand, location); self.super_operand(operand, location);
} }
} }

View File

@ -2,6 +2,7 @@
#![feature(c_variadic)] #![feature(c_variadic)]
#![warn(function_item_references)] #![warn(function_item_references)]
use std::fmt::Pointer; use std::fmt::Pointer;
use std::fmt::Formatter;
fn nop() { } fn nop() { }
fn foo() -> u32 { 42 } fn foo() -> u32 { 42 }
@ -20,6 +21,25 @@ fn parameterized_call_fn<F: Fn(u32) -> u32>(f: &F, x: u32) { f(x); }
fn print_ptr<F: Pointer>(f: F) { println!("{:p}", f); } fn print_ptr<F: Pointer>(f: F) { println!("{:p}", f); }
fn bound_by_ptr_trait<F: Pointer>(_f: F) { } fn bound_by_ptr_trait<F: Pointer>(_f: F) { }
fn bound_by_ptr_trait_tuple<F: Pointer, G: Pointer>(_t: (F, G)) { } fn bound_by_ptr_trait_tuple<F: Pointer, G: Pointer>(_t: (F, G)) { }
fn implicit_ptr_trait<F>(f: &F) { println!("{:p}", f); }
//case found in tinyvec that triggered a compiler error in an earlier version of the lint checker
trait HasItem {
type Item;
fn assoc_item(&self) -> Self::Item;
}
fn _format_assoc_item<T: HasItem>(data: T, f: &mut Formatter) -> std::fmt::Result
where T::Item: Pointer {
//when the arg type bound by `Pointer` is an associated type, we shouldn't attempt to normalize
Pointer::fmt(&data.assoc_item(), f)
}
//simple test to make sure that calls to `Pointer::fmt` aren't double counted
fn _call_pointer_fmt(f: &mut Formatter) -> std::fmt::Result {
let zst_ref = &foo;
Pointer::fmt(&zst_ref, f)
//~^ WARNING cast `foo` with `as fn() -> _` to obtain a function pointer
}
fn main() { fn main() {
//`let` bindings with function references shouldn't lint //`let` bindings with function references shouldn't lint
@ -126,6 +146,7 @@ fn main() {
bound_by_ptr_trait_tuple((&foo, &bar)); bound_by_ptr_trait_tuple((&foo, &bar));
//~^ WARNING cast `foo` with `as fn() -> _` to obtain a function pointer //~^ WARNING cast `foo` with `as fn() -> _` to obtain a function pointer
//~^^ WARNING cast `bar` with `as fn(_) -> _` to obtain a function pointer //~^^ WARNING cast `bar` with `as fn(_) -> _` to obtain a function pointer
implicit_ptr_trait(&bar); // ignore
//correct ways to pass function pointers as arguments bound by std::fmt::Pointer //correct ways to pass function pointers as arguments bound by std::fmt::Pointer
print_ptr(bar as fn(u32) -> u32); print_ptr(bar as fn(u32) -> u32);

View File

@ -1,8 +1,8 @@
warning: cast `foo` with `as fn() -> _` to obtain a function pointer warning: cast `foo` with `as fn() -> _` to obtain a function pointer
--> $DIR/function-references.rs:57:22 --> $DIR/function-references.rs:40:18
| |
LL | println!("{:p}", &foo); LL | Pointer::fmt(&zst_ref, f)
| ^^^^ | ^^^^^^^^
| |
note: the lint level is defined here note: the lint level is defined here
--> $DIR/function-references.rs:3:9 --> $DIR/function-references.rs:3:9
@ -11,160 +11,166 @@ LL | #![warn(function_item_references)]
| ^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^
warning: cast `foo` with `as fn() -> _` to obtain a function pointer warning: cast `foo` with `as fn() -> _` to obtain a function pointer
--> $DIR/function-references.rs:59:20 --> $DIR/function-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
| |
LL | print!("{:p}", &foo); LL | print!("{:p}", &foo);
| ^^^^ | ^^^^
warning: cast `foo` with `as fn() -> _` to obtain a function pointer warning: cast `foo` with `as fn() -> _` to obtain a function pointer
--> $DIR/function-references.rs:61:21 --> $DIR/function-references.rs:81:21
| |
LL | format!("{:p}", &foo); LL | format!("{:p}", &foo);
| ^^^^ | ^^^^
warning: cast `foo` with `as fn() -> _` to obtain a function pointer warning: cast `foo` with `as fn() -> _` to obtain a function pointer
--> $DIR/function-references.rs:64:22 --> $DIR/function-references.rs:84:22
| |
LL | println!("{:p}", &foo as *const _); LL | println!("{:p}", &foo as *const _);
| ^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^
warning: cast `foo` with `as fn() -> _` to obtain a function pointer warning: cast `foo` with `as fn() -> _` to obtain a function pointer
--> $DIR/function-references.rs:66:22 --> $DIR/function-references.rs:86:22
| |
LL | println!("{:p}", zst_ref); LL | println!("{:p}", zst_ref);
| ^^^^^^^ | ^^^^^^^
warning: cast `foo` with `as fn() -> _` to obtain a function pointer warning: cast `foo` with `as fn() -> _` to obtain a function pointer
--> $DIR/function-references.rs:68:22 --> $DIR/function-references.rs:88:22
| |
LL | println!("{:p}", cast_zst_ptr); LL | println!("{:p}", cast_zst_ptr);
| ^^^^^^^^^^^^ | ^^^^^^^^^^^^
warning: cast `foo` with `as fn() -> _` to obtain a function pointer warning: cast `foo` with `as fn() -> _` to obtain a function pointer
--> $DIR/function-references.rs:70:22 --> $DIR/function-references.rs:90:22
| |
LL | println!("{:p}", coerced_zst_ptr); LL | println!("{:p}", coerced_zst_ptr);
| ^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^
warning: cast `foo` with `as fn() -> _` to obtain a function pointer warning: cast `foo` with `as fn() -> _` to obtain a function pointer
--> $DIR/function-references.rs:73:22 --> $DIR/function-references.rs:93:22
| |
LL | println!("{:p}", &fn_item); LL | println!("{:p}", &fn_item);
| ^^^^^^^^ | ^^^^^^^^
warning: cast `foo` with `as fn() -> _` to obtain a function pointer warning: cast `foo` with `as fn() -> _` to obtain a function pointer
--> $DIR/function-references.rs:75:22 --> $DIR/function-references.rs:95:22
| |
LL | println!("{:p}", indirect_ref); LL | println!("{:p}", indirect_ref);
| ^^^^^^^^^^^^ | ^^^^^^^^^^^^
warning: cast `nop` with `as fn()` to obtain a function pointer warning: cast `nop` with `as fn()` to obtain a function pointer
--> $DIR/function-references.rs:78:22 --> $DIR/function-references.rs:98:22
| |
LL | println!("{:p}", &nop); LL | println!("{:p}", &nop);
| ^^^^ | ^^^^
warning: cast `bar` with `as fn(_) -> _` to obtain a function pointer warning: cast `bar` with `as fn(_) -> _` to obtain a function pointer
--> $DIR/function-references.rs:80:22 --> $DIR/function-references.rs:100:22
| |
LL | println!("{:p}", &bar); LL | println!("{:p}", &bar);
| ^^^^ | ^^^^
warning: cast `baz` with `as fn(_, _) -> _` to obtain a function pointer warning: cast `baz` with `as fn(_, _) -> _` to obtain a function pointer
--> $DIR/function-references.rs:82:22 --> $DIR/function-references.rs:102:22
| |
LL | println!("{:p}", &baz); LL | println!("{:p}", &baz);
| ^^^^ | ^^^^
warning: cast `unsafe_fn` with `as unsafe fn()` to obtain a function pointer warning: cast `unsafe_fn` with `as unsafe fn()` to obtain a function pointer
--> $DIR/function-references.rs:84:22 --> $DIR/function-references.rs:104:22
| |
LL | println!("{:p}", &unsafe_fn); LL | println!("{:p}", &unsafe_fn);
| ^^^^^^^^^^ | ^^^^^^^^^^
warning: cast `c_fn` with `as extern "C" fn()` to obtain a function pointer warning: cast `c_fn` with `as extern "C" fn()` to obtain a function pointer
--> $DIR/function-references.rs:86:22 --> $DIR/function-references.rs:106:22
| |
LL | println!("{:p}", &c_fn); LL | println!("{:p}", &c_fn);
| ^^^^^ | ^^^^^
warning: cast `unsafe_c_fn` with `as unsafe extern "C" fn()` to obtain a function pointer warning: cast `unsafe_c_fn` with `as unsafe extern "C" fn()` to obtain a function pointer
--> $DIR/function-references.rs:88:22 --> $DIR/function-references.rs:108:22
| |
LL | println!("{:p}", &unsafe_c_fn); LL | println!("{:p}", &unsafe_c_fn);
| ^^^^^^^^^^^^ | ^^^^^^^^^^^^
warning: cast `variadic` with `as unsafe extern "C" fn(_, ...)` to obtain a function pointer warning: cast `variadic` with `as unsafe extern "C" fn(_, ...)` to obtain a function pointer
--> $DIR/function-references.rs:90:22 --> $DIR/function-references.rs:110:22
| |
LL | println!("{:p}", &variadic); LL | println!("{:p}", &variadic);
| ^^^^^^^^^ | ^^^^^^^^^
warning: cast `var` with `as fn(_) -> _` to obtain a function pointer warning: cast `var` with `as fn(_) -> _` to obtain a function pointer
--> $DIR/function-references.rs:92:22 --> $DIR/function-references.rs:112:22
| |
LL | println!("{:p}", &std::env::var::<String>); LL | println!("{:p}", &std::env::var::<String>);
| ^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^
warning: cast `nop` with `as fn()` to obtain a function pointer warning: cast `nop` with `as fn()` to obtain a function pointer
--> $DIR/function-references.rs:95:32 --> $DIR/function-references.rs:115:32
| |
LL | println!("{:p} {:p} {:p}", &nop, &foo, &bar); LL | println!("{:p} {:p} {:p}", &nop, &foo, &bar);
| ^^^^ | ^^^^
warning: cast `foo` with `as fn() -> _` to obtain a function pointer warning: cast `foo` with `as fn() -> _` to obtain a function pointer
--> $DIR/function-references.rs:95:38 --> $DIR/function-references.rs:115:38
| |
LL | println!("{:p} {:p} {:p}", &nop, &foo, &bar); LL | println!("{:p} {:p} {:p}", &nop, &foo, &bar);
| ^^^^ | ^^^^
warning: cast `bar` with `as fn(_) -> _` to obtain a function pointer warning: cast `bar` with `as fn(_) -> _` to obtain a function pointer
--> $DIR/function-references.rs:95:44 --> $DIR/function-references.rs:115:44
| |
LL | println!("{:p} {:p} {:p}", &nop, &foo, &bar); LL | println!("{:p} {:p} {:p}", &nop, &foo, &bar);
| ^^^^ | ^^^^
warning: cast `foo` with `as fn() -> _` to obtain a function pointer warning: cast `foo` with `as fn() -> _` to obtain a function pointer
--> $DIR/function-references.rs:110:41 --> $DIR/function-references.rs:130:41
| |
LL | std::mem::transmute::<_, usize>(&foo); LL | std::mem::transmute::<_, usize>(&foo);
| ^^^^ | ^^^^
warning: cast `foo` with `as fn() -> _` to obtain a function pointer warning: cast `foo` with `as fn() -> _` to obtain a function pointer
--> $DIR/function-references.rs:112:50 --> $DIR/function-references.rs:132:50
| |
LL | std::mem::transmute::<_, (usize, usize)>((&foo, &bar)); LL | std::mem::transmute::<_, (usize, usize)>((&foo, &bar));
| ^^^^^^^^^^^^ | ^^^^^^^^^^^^
warning: cast `bar` with `as fn(_) -> _` to obtain a function pointer warning: cast `bar` with `as fn(_) -> _` to obtain a function pointer
--> $DIR/function-references.rs:112:50 --> $DIR/function-references.rs:132:50
| |
LL | std::mem::transmute::<_, (usize, usize)>((&foo, &bar)); LL | std::mem::transmute::<_, (usize, usize)>((&foo, &bar));
| ^^^^^^^^^^^^ | ^^^^^^^^^^^^
warning: cast `bar` with `as fn(_) -> _` to obtain a function pointer warning: cast `bar` with `as fn(_) -> _` to obtain a function pointer
--> $DIR/function-references.rs:122:15 --> $DIR/function-references.rs:142:15
| |
LL | print_ptr(&bar); LL | print_ptr(&bar);
| ^^^^ | ^^^^
warning: cast `bar` with `as fn(_) -> _` to obtain a function pointer warning: cast `bar` with `as fn(_) -> _` to obtain a function pointer
--> $DIR/function-references.rs:124:24 --> $DIR/function-references.rs:144:24
| |
LL | bound_by_ptr_trait(&bar); LL | bound_by_ptr_trait(&bar);
| ^^^^ | ^^^^
warning: cast `bar` with `as fn(_) -> _` to obtain a function pointer warning: cast `bar` with `as fn(_) -> _` to obtain a function pointer
--> $DIR/function-references.rs:126:30 --> $DIR/function-references.rs:146:30
| |
LL | bound_by_ptr_trait_tuple((&foo, &bar)); LL | bound_by_ptr_trait_tuple((&foo, &bar));
| ^^^^^^^^^^^^ | ^^^^^^^^^^^^
warning: cast `foo` with `as fn() -> _` to obtain a function pointer warning: cast `foo` with `as fn() -> _` to obtain a function pointer
--> $DIR/function-references.rs:126:30 --> $DIR/function-references.rs:146:30
| |
LL | bound_by_ptr_trait_tuple((&foo, &bar)); LL | bound_by_ptr_trait_tuple((&foo, &bar));
| ^^^^^^^^^^^^ | ^^^^^^^^^^^^
warning: 27 warnings emitted warning: 28 warnings emitted