Skip to content

Commit

Permalink
wf: check foreign fn decls for well-formedness
Browse files Browse the repository at this point in the history
This commit extends current well-formedness checking to apply to foreign
function declarations, re-using the existing machinery for regular
functions. In doing this, later parts of the compiler (such as the
`improper_ctypes` lint) can rely on being operations not failing as a
result of invalid code which would normally be caught earlier.

Signed-off-by: David Wood <david@davidtw.co>
  • Loading branch information
davidtwco committed Jul 20, 2020
1 parent 891e6fe commit ceac273
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 52 deletions.
48 changes: 28 additions & 20 deletions src/librustc_typeck/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use rustc_middle::ty::{
self, AdtKind, GenericParamDefKind, ToPredicate, Ty, TyCtxt, TypeFoldable, WithConstness,
};
use rustc_session::parse::feature_err;
use rustc_span::symbol::{sym, Symbol};
use rustc_span::symbol::{sym, Ident, Symbol};
use rustc_span::Span;
use rustc_trait_selection::opaque_types::may_define_opaque_type;
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
Expand Down Expand Up @@ -142,8 +142,8 @@ pub fn check_item_well_formed(tcx: TyCtxt<'_>, def_id: LocalDefId) {
_ => unreachable!(),
}
}
hir::ItemKind::Fn(..) => {
check_item_fn(tcx, item);
hir::ItemKind::Fn(ref sig, ..) => {
check_item_fn(tcx, item.hir_id, item.ident, item.span, sig.decl);
}
hir::ItemKind::Static(ref ty, ..) => {
check_item_type(tcx, item.hir_id, ty.span, false);
Expand All @@ -153,8 +153,14 @@ pub fn check_item_well_formed(tcx: TyCtxt<'_>, def_id: LocalDefId) {
}
hir::ItemKind::ForeignMod(ref module) => {
for it in module.items.iter() {
if let hir::ForeignItemKind::Static(ref ty, ..) = it.kind {
check_item_type(tcx, it.hir_id, ty.span, true);
match it.kind {
hir::ForeignItemKind::Fn(ref decl, ..) => {
check_item_fn(tcx, it.hir_id, it.ident, it.span, decl)
}
hir::ForeignItemKind::Static(ref ty, ..) => {
check_item_type(tcx, it.hir_id, ty.span, true)
}
hir::ForeignItemKind::Type => (),
}
}
}
Expand Down Expand Up @@ -303,7 +309,7 @@ fn check_associated_item(
fcx,
item.ident.span,
sig,
hir_sig,
hir_sig.decl,
item.def_id,
&mut implied_bounds,
);
Expand Down Expand Up @@ -564,22 +570,24 @@ fn check_associated_type_defaults(fcx: &FnCtxt<'_, '_>, trait_def_id: DefId) {
}
}

fn check_item_fn(tcx: TyCtxt<'_>, item: &hir::Item<'_>) {
for_item(tcx, item).with_fcx(|fcx, tcx| {
let def_id = fcx.tcx.hir().local_def_id(item.hir_id);
fn check_item_fn(
tcx: TyCtxt<'_>,
item_id: hir::HirId,
ident: Ident,
span: Span,
decl: &hir::FnDecl<'_>,
) {
for_id(tcx, item_id, span).with_fcx(|fcx, tcx| {
let def_id = fcx.tcx.hir().local_def_id(item_id);
let sig = fcx.tcx.fn_sig(def_id);
let sig = fcx.normalize_associated_types_in(item.span, &sig);
let sig = fcx.normalize_associated_types_in(span, &sig);
let mut implied_bounds = vec![];
let hir_sig = match &item.kind {
ItemKind::Fn(sig, ..) => sig,
_ => bug!("expected `ItemKind::Fn`, found `{:?}`", item.kind),
};
check_fn_or_method(
tcx,
fcx,
item.ident.span,
ident.span,
sig,
hir_sig,
decl,
def_id.to_def_id(),
&mut implied_bounds,
);
Expand Down Expand Up @@ -835,28 +843,28 @@ fn check_fn_or_method<'fcx, 'tcx>(
fcx: &FnCtxt<'fcx, 'tcx>,
span: Span,
sig: ty::PolyFnSig<'tcx>,
hir_sig: &hir::FnSig<'_>,
hir_decl: &hir::FnDecl<'_>,
def_id: DefId,
implied_bounds: &mut Vec<Ty<'tcx>>,
) {
let sig = fcx.normalize_associated_types_in(span, &sig);
let sig = fcx.tcx.liberate_late_bound_regions(def_id, &sig);

for (&input_ty, span) in sig.inputs().iter().zip(hir_sig.decl.inputs.iter().map(|t| t.span)) {
for (&input_ty, span) in sig.inputs().iter().zip(hir_decl.inputs.iter().map(|t| t.span)) {
fcx.register_wf_obligation(input_ty.into(), span, ObligationCauseCode::MiscObligation);
}
implied_bounds.extend(sig.inputs());

fcx.register_wf_obligation(
sig.output().into(),
hir_sig.decl.output.span(),
hir_decl.output.span(),
ObligationCauseCode::ReturnType,
);

// FIXME(#25759) return types should not be implied bounds
implied_bounds.push(sig.output());

check_where_clauses(tcx, fcx, span, def_id, Some((sig.output(), hir_sig.decl.output.span())));
check_where_clauses(tcx, fcx, span, def_id, Some((sig.output(), hir_decl.output.span())));
}

/// Checks "defining uses" of opaque `impl Trait` types to ensure that they meet the restrictions
Expand Down
3 changes: 2 additions & 1 deletion src/test/ui/lint/lint-ctypes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ extern crate libc;

use std::marker::PhantomData;

trait Bar { }
trait Mirror { type It: ?Sized; }
impl<T: ?Sized> Mirror for T { type It = Self; }
#[repr(C)]
Expand Down Expand Up @@ -53,7 +54,7 @@ extern {
pub fn char_type(p: char); //~ ERROR uses type `char`
pub fn i128_type(p: i128); //~ ERROR uses type `i128`
pub fn u128_type(p: u128); //~ ERROR uses type `u128`
pub fn trait_type(p: &dyn Clone); //~ ERROR uses type `dyn std::clone::Clone`
pub fn trait_type(p: &dyn Bar); //~ ERROR uses type `dyn Bar`
pub fn tuple_type(p: (i32, i32)); //~ ERROR uses type `(i32, i32)`
pub fn tuple_type2(p: I32Pair); //~ ERROR uses type `(i32, i32)`
pub fn zero_size(p: ZeroSize); //~ ERROR uses type `ZeroSize`
Expand Down
62 changes: 31 additions & 31 deletions src/test/ui/lint/lint-ctypes.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: `extern` block uses type `Foo`, which is not FFI-safe
--> $DIR/lint-ctypes.rs:46:28
--> $DIR/lint-ctypes.rs:47:28
|
LL | pub fn ptr_type1(size: *const Foo);
| ^^^^^^^^^^ not FFI-safe
Expand All @@ -12,27 +12,27 @@ LL | #![deny(improper_ctypes)]
= help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
= note: this struct has unspecified layout
note: the type is defined here
--> $DIR/lint-ctypes.rs:24:1
--> $DIR/lint-ctypes.rs:25:1
|
LL | pub struct Foo;
| ^^^^^^^^^^^^^^^

error: `extern` block uses type `Foo`, which is not FFI-safe
--> $DIR/lint-ctypes.rs:47:28
--> $DIR/lint-ctypes.rs:48:28
|
LL | pub fn ptr_type2(size: *const Foo);
| ^^^^^^^^^^ not FFI-safe
|
= help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
= note: this struct has unspecified layout
note: the type is defined here
--> $DIR/lint-ctypes.rs:24:1
--> $DIR/lint-ctypes.rs:25:1
|
LL | pub struct Foo;
| ^^^^^^^^^^^^^^^

error: `extern` block uses type `[u32]`, which is not FFI-safe
--> $DIR/lint-ctypes.rs:48:26
--> $DIR/lint-ctypes.rs:49:26
|
LL | pub fn slice_type(p: &[u32]);
| ^^^^^^ not FFI-safe
Expand All @@ -41,7 +41,7 @@ LL | pub fn slice_type(p: &[u32]);
= note: slices have no C equivalent

error: `extern` block uses type `str`, which is not FFI-safe
--> $DIR/lint-ctypes.rs:49:24
--> $DIR/lint-ctypes.rs:50:24
|
LL | pub fn str_type(p: &str);
| ^^^^ not FFI-safe
Expand All @@ -50,7 +50,7 @@ LL | pub fn str_type(p: &str);
= note: string slices have no C equivalent

error: `extern` block uses type `std::boxed::Box<u32>`, which is not FFI-safe
--> $DIR/lint-ctypes.rs:50:24
--> $DIR/lint-ctypes.rs:51:24
|
LL | pub fn box_type(p: Box<u32>);
| ^^^^^^^^ not FFI-safe
Expand All @@ -59,7 +59,7 @@ LL | pub fn box_type(p: Box<u32>);
= note: this struct has unspecified layout

error: `extern` block uses type `std::option::Option<std::boxed::Box<u32>>`, which is not FFI-safe
--> $DIR/lint-ctypes.rs:51:28
--> $DIR/lint-ctypes.rs:52:28
|
LL | pub fn opt_box_type(p: Option<Box<u32>>);
| ^^^^^^^^^^^^^^^^ not FFI-safe
Expand All @@ -68,7 +68,7 @@ LL | pub fn opt_box_type(p: Option<Box<u32>>);
= note: enum has no representation hint

error: `extern` block uses type `char`, which is not FFI-safe
--> $DIR/lint-ctypes.rs:53:25
--> $DIR/lint-ctypes.rs:54:25
|
LL | pub fn char_type(p: char);
| ^^^^ not FFI-safe
Expand All @@ -77,31 +77,31 @@ LL | pub fn char_type(p: char);
= note: the `char` type has no C equivalent

error: `extern` block uses type `i128`, which is not FFI-safe
--> $DIR/lint-ctypes.rs:54:25
--> $DIR/lint-ctypes.rs:55:25
|
LL | pub fn i128_type(p: i128);
| ^^^^ not FFI-safe
|
= note: 128-bit integers don't currently have a known stable ABI

error: `extern` block uses type `u128`, which is not FFI-safe
--> $DIR/lint-ctypes.rs:55:25
--> $DIR/lint-ctypes.rs:56:25
|
LL | pub fn u128_type(p: u128);
| ^^^^ not FFI-safe
|
= note: 128-bit integers don't currently have a known stable ABI

error: `extern` block uses type `dyn std::clone::Clone`, which is not FFI-safe
--> $DIR/lint-ctypes.rs:56:26
error: `extern` block uses type `dyn Bar`, which is not FFI-safe
--> $DIR/lint-ctypes.rs:57:26
|
LL | pub fn trait_type(p: &dyn Clone);
| ^^^^^^^^^^ not FFI-safe
LL | pub fn trait_type(p: &dyn Bar);
| ^^^^^^^^ not FFI-safe
|
= note: trait objects have no C equivalent

error: `extern` block uses type `(i32, i32)`, which is not FFI-safe
--> $DIR/lint-ctypes.rs:57:26
--> $DIR/lint-ctypes.rs:58:26
|
LL | pub fn tuple_type(p: (i32, i32));
| ^^^^^^^^^^ not FFI-safe
Expand All @@ -110,7 +110,7 @@ LL | pub fn tuple_type(p: (i32, i32));
= note: tuples have unspecified layout

error: `extern` block uses type `(i32, i32)`, which is not FFI-safe
--> $DIR/lint-ctypes.rs:58:27
--> $DIR/lint-ctypes.rs:59:27
|
LL | pub fn tuple_type2(p: I32Pair);
| ^^^^^^^ not FFI-safe
Expand All @@ -119,42 +119,42 @@ LL | pub fn tuple_type2(p: I32Pair);
= note: tuples have unspecified layout

error: `extern` block uses type `ZeroSize`, which is not FFI-safe
--> $DIR/lint-ctypes.rs:59:25
--> $DIR/lint-ctypes.rs:60:25
|
LL | pub fn zero_size(p: ZeroSize);
| ^^^^^^^^ not FFI-safe
|
= help: consider adding a member to this struct
= note: this struct has no fields
note: the type is defined here
--> $DIR/lint-ctypes.rs:20:1
--> $DIR/lint-ctypes.rs:21:1
|
LL | pub struct ZeroSize;
| ^^^^^^^^^^^^^^^^^^^^

error: `extern` block uses type `ZeroSizeWithPhantomData`, which is not FFI-safe
--> $DIR/lint-ctypes.rs:60:33
--> $DIR/lint-ctypes.rs:61:33
|
LL | pub fn zero_size_phantom(p: ZeroSizeWithPhantomData);
| ^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
|
= note: composed only of `PhantomData`
note: the type is defined here
--> $DIR/lint-ctypes.rs:43:1
--> $DIR/lint-ctypes.rs:44:1
|
LL | pub struct ZeroSizeWithPhantomData(::std::marker::PhantomData<i32>);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: `extern` block uses type `std::marker::PhantomData<bool>`, which is not FFI-safe
--> $DIR/lint-ctypes.rs:63:12
--> $DIR/lint-ctypes.rs:64:12
|
LL | -> ::std::marker::PhantomData<bool>;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
|
= note: composed only of `PhantomData`

error: `extern` block uses type `fn()`, which is not FFI-safe
--> $DIR/lint-ctypes.rs:64:23
--> $DIR/lint-ctypes.rs:65:23
|
LL | pub fn fn_type(p: RustFn);
| ^^^^^^ not FFI-safe
Expand All @@ -163,7 +163,7 @@ LL | pub fn fn_type(p: RustFn);
= note: this function pointer has Rust-specific calling convention

error: `extern` block uses type `fn()`, which is not FFI-safe
--> $DIR/lint-ctypes.rs:65:24
--> $DIR/lint-ctypes.rs:66:24
|
LL | pub fn fn_type2(p: fn());
| ^^^^ not FFI-safe
Expand All @@ -172,7 +172,7 @@ LL | pub fn fn_type2(p: fn());
= note: this function pointer has Rust-specific calling convention

error: `extern` block uses type `std::boxed::Box<u32>`, which is not FFI-safe
--> $DIR/lint-ctypes.rs:66:28
--> $DIR/lint-ctypes.rs:67:28
|
LL | pub fn fn_contained(p: RustBadRet);
| ^^^^^^^^^^ not FFI-safe
Expand All @@ -181,15 +181,15 @@ LL | pub fn fn_contained(p: RustBadRet);
= note: this struct has unspecified layout

error: `extern` block uses type `i128`, which is not FFI-safe
--> $DIR/lint-ctypes.rs:67:32
--> $DIR/lint-ctypes.rs:68:32
|
LL | pub fn transparent_i128(p: TransparentI128);
| ^^^^^^^^^^^^^^^ not FFI-safe
|
= note: 128-bit integers don't currently have a known stable ABI

error: `extern` block uses type `str`, which is not FFI-safe
--> $DIR/lint-ctypes.rs:68:31
--> $DIR/lint-ctypes.rs:69:31
|
LL | pub fn transparent_str(p: TransparentStr);
| ^^^^^^^^^^^^^^ not FFI-safe
Expand All @@ -198,7 +198,7 @@ LL | pub fn transparent_str(p: TransparentStr);
= note: string slices have no C equivalent

error: `extern` block uses type `std::boxed::Box<u32>`, which is not FFI-safe
--> $DIR/lint-ctypes.rs:69:30
--> $DIR/lint-ctypes.rs:70:30
|
LL | pub fn transparent_fn(p: TransparentBadFn);
| ^^^^^^^^^^^^^^^^ not FFI-safe
Expand All @@ -207,7 +207,7 @@ LL | pub fn transparent_fn(p: TransparentBadFn);
= note: this struct has unspecified layout

error: `extern` block uses type `[u8; 8]`, which is not FFI-safe
--> $DIR/lint-ctypes.rs:70:27
--> $DIR/lint-ctypes.rs:71:27
|
LL | pub fn raw_array(arr: [u8; 8]);
| ^^^^^^^ not FFI-safe
Expand All @@ -216,15 +216,15 @@ LL | pub fn raw_array(arr: [u8; 8]);
= note: passing raw arrays by value is not FFI-safe

error: `extern` block uses type `u128`, which is not FFI-safe
--> $DIR/lint-ctypes.rs:72:34
--> $DIR/lint-ctypes.rs:73:34
|
LL | pub static static_u128_type: u128;
| ^^^^ not FFI-safe
|
= note: 128-bit integers don't currently have a known stable ABI

error: `extern` block uses type `u128`, which is not FFI-safe
--> $DIR/lint-ctypes.rs:73:40
--> $DIR/lint-ctypes.rs:74:40
|
LL | pub static static_u128_array_type: [u128; 16];
| ^^^^^^^^^^ not FFI-safe
Expand Down
18 changes: 18 additions & 0 deletions src/test/ui/wf/wf-foreign-fn-decl-ret.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
pub trait Unsatisfied {}

#[repr(transparent)]
pub struct Bar<T: Unsatisfied>(T);

pub trait Foo {
type Assoc;
}

extern "C" {
pub fn lint_me() -> <() as Foo>::Assoc;
//~^ ERROR: the trait bound `(): Foo` is not satisfied [E0277]

pub fn lint_me_aswell() -> Bar<u32>;
//~^ ERROR: the trait bound `u32: Unsatisfied` is not satisfied [E0277]
}

fn main() {}
Loading

0 comments on commit ceac273

Please sign in to comment.