Skip to content

Commit

Permalink
Fixed compiler error in lint checker triggered by associated types
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@ayrtonm
ayrtonm committed Oct 27, 2020
1 parent 432ebd5 commit d6fa7e1
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 83 deletions.
118 changes: 65 additions & 53 deletions compiler/rustc_mir/src/transform/function_references.rs
View file
@@ -1,7 +1,11 @@
use rustc_hir::def_id::DefId;
use rustc_middle::mir::visit::Visitor;
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_span::{symbol::sym, Span};
use rustc_target::spec::abi::Abi;
Expand Down Expand Up @@ -33,48 +37,50 @@ impl<'a, 'tcx> Visitor<'tcx> for FunctionItemRefChecker<'a, 'tcx> {
fn_span: _,
} = &terminator.kind
{
let func_ty = func.ty(self.body, self.tcx);
if let ty::FnDef(def_id, substs_ref) = *func_ty.kind() {
//check arguments for `std::mem::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() {
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();
let source_info = *self.body.source_info(location);
let span = self.nth_arg_span(&args, 0);
self.emit_lint(ident, fn_id, source_info, span);
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);
if let ty::FnDef(def_id, substs_ref) = *func_ty.kind() {
//handle `std::mem::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() {
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();
let span = self.nth_arg_span(&args, 0);
self.emit_lint(ident, fn_id, source_info, span);
}
}
}
}
} else {
//check arguments for any function with `std::fmt::Pointer` as a bound trait
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()) {
let arg_defs = self.tcx.fn_sig(def_id).skip_binder().inputs();
for (arg_num, arg_def) in arg_defs.iter().enumerate() {
for generic_inner_ty in arg_def.walk() {
if let GenericArgKind::Type(inner_ty) =
generic_inner_ty.unpack()
{
//if any type reachable from the argument types in the fn sig matches the type bound by `Pointer`
if TyS::same_type(inner_ty, bound_ty) {
//check if this type is a function reference in the function call
let norm_ty =
self.tcx.subst_and_normalize_erasing_regions(
substs_ref, param_env, &inner_ty,
);
if let Some(fn_id) =
FunctionItemRefChecker::is_fn_ref(norm_ty)
{
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, arg_num);
self.emit_lint(ident, fn_id, source_info, span);
} 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);
}
}
}
}
Expand All @@ -87,19 +93,25 @@ impl<'a, 'tcx> Visitor<'tcx> for FunctionItemRefChecker<'a, 'tcx> {
}
self.super_terminator(terminator, location);
}
//check for `std::fmt::Pointer::<T>::fmt` where T is a function reference
//this is used in formatting macros, but doesn't rely on the specific expansion
//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.
fn visit_operand(&mut self, operand: &Operand<'tcx>, location: Location) {
let op_ty = operand.ty(self.body, self.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) {
let source_info = *self.body.source_info(location);
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);
let source_info = *self.body.source_info(location);
if source_info.span.from_expansion() {
let op_ty = operand.ty(self.body, self.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) {
//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);
}
}
}
}
Expand Down
21 changes: 21 additions & 0 deletions src/test/ui/lint/function-references.rs
View file
Expand Up @@ -2,6 +2,7 @@
#![feature(c_variadic)]
#![warn(function_item_references)]
use std::fmt::Pointer;
use std::fmt::Formatter;

fn nop() { }
fn foo() -> u32 { 42 }
Expand All @@ -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 bound_by_ptr_trait<F: Pointer>(_f: F) { }
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() {
//`let` bindings with function references shouldn't lint
Expand Down Expand Up @@ -126,6 +146,7 @@ fn main() {
bound_by_ptr_trait_tuple((&foo, &bar));
//~^ WARNING cast `foo` 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
print_ptr(bar as fn(u32) -> u32);
Expand Down
66 changes: 36 additions & 30 deletions src/test/ui/lint/function-references.stderr
View file
@@ -1,8 +1,8 @@
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
--> $DIR/function-references.rs:3:9
Expand All @@ -11,160 +11,166 @@ LL | #![warn(function_item_references)]
| ^^^^^^^^^^^^^^^^^^^^^^^^

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

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

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 _);
| ^^^^^^^^^^^^^^^^

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

warning: 27 warnings emitted
warning: 28 warnings emitted

0 comments on commit d6fa7e1

Please sign in to comment.