From 1f0e1a043959461afd061be7ff92362492b2c85d Mon Sep 17 00:00:00 2001 From: Robin Kruppe Date: Thu, 15 Feb 2018 00:06:14 +0100 Subject: [PATCH 1/7] Convert compile-fail/lint-ctypes.rs to ui test --- src/test/{compile-fail => ui}/lint-ctypes.rs | 0 src/test/ui/lint-ctypes.stderr | 128 +++++++++++++++++++ 2 files changed, 128 insertions(+) rename src/test/{compile-fail => ui}/lint-ctypes.rs (100%) create mode 100644 src/test/ui/lint-ctypes.stderr diff --git a/src/test/compile-fail/lint-ctypes.rs b/src/test/ui/lint-ctypes.rs similarity index 100% rename from src/test/compile-fail/lint-ctypes.rs rename to src/test/ui/lint-ctypes.rs diff --git a/src/test/ui/lint-ctypes.stderr b/src/test/ui/lint-ctypes.stderr new file mode 100644 index 00000000000..21aadde8ac8 --- /dev/null +++ b/src/test/ui/lint-ctypes.stderr @@ -0,0 +1,128 @@ +error: found struct without foreign-function-safe representation annotation in foreign module, consider adding a #[repr(C)] attribute to the type + --> $DIR/lint-ctypes.rs:54:28 + | +54 | pub fn ptr_type1(size: *const Foo); //~ ERROR: found struct without + | ^^^^^^^^^^ + | +note: lint level defined here + --> $DIR/lint-ctypes.rs:11:9 + | +11 | #![deny(improper_ctypes)] + | ^^^^^^^^^^^^^^^ + +error: found struct without foreign-function-safe representation annotation in foreign module, consider adding a #[repr(C)] attribute to the type + --> $DIR/lint-ctypes.rs:55:28 + | +55 | pub fn ptr_type2(size: *const Foo); //~ ERROR: found struct without + | ^^^^^^^^^^ + +error: found Rust slice type in foreign module, consider using a raw pointer instead + --> $DIR/lint-ctypes.rs:56:26 + | +56 | pub fn slice_type(p: &[u32]); //~ ERROR: found Rust slice type + | ^^^^^^ + +error: found Rust type `str` in foreign module; consider using a `*const libc::c_char` + --> $DIR/lint-ctypes.rs:57:24 + | +57 | pub fn str_type(p: &str); //~ ERROR: found Rust type + | ^^^^ + +error: found struct without foreign-function-safe representation annotation in foreign module, consider adding a #[repr(C)] attribute to the type + --> $DIR/lint-ctypes.rs:58:24 + | +58 | pub fn box_type(p: Box); //~ ERROR found struct without + | ^^^^^^^^ + +error: found Rust type `char` in foreign module, while `u32` or `libc::wchar_t` should be used + --> $DIR/lint-ctypes.rs:59:25 + | +59 | pub fn char_type(p: char); //~ ERROR found Rust type + | ^^^^ + +error: found Rust type `i128` in foreign module, but 128-bit integers don't currently have a known stable ABI + --> $DIR/lint-ctypes.rs:60:25 + | +60 | pub fn i128_type(p: i128); //~ ERROR found Rust type + | ^^^^ + +error: found Rust type `u128` in foreign module, but 128-bit integers don't currently have a known stable ABI + --> $DIR/lint-ctypes.rs:61:25 + | +61 | pub fn u128_type(p: u128); //~ ERROR found Rust type + | ^^^^ + +error: found Rust trait type in foreign module, consider using a raw pointer instead + --> $DIR/lint-ctypes.rs:62:26 + | +62 | pub fn trait_type(p: &Clone); //~ ERROR found Rust trait type + | ^^^^^^ + +error: found Rust tuple type in foreign module; consider using a struct instead + --> $DIR/lint-ctypes.rs:63:26 + | +63 | pub fn tuple_type(p: (i32, i32)); //~ ERROR found Rust tuple type + | ^^^^^^^^^^ + +error: found Rust tuple type in foreign module; consider using a struct instead + --> $DIR/lint-ctypes.rs:64:27 + | +64 | pub fn tuple_type2(p: I32Pair); //~ ERROR found Rust tuple type + | ^^^^^^^ + +error: found zero-size struct in foreign module, consider adding a member to this struct + --> $DIR/lint-ctypes.rs:65:25 + | +65 | pub fn zero_size(p: ZeroSize); //~ ERROR found zero-size struct + | ^^^^^^^^ + +error: found zero-sized type composed only of phantom-data in a foreign-function. + --> $DIR/lint-ctypes.rs:66:33 + | +66 | pub fn zero_size_phantom(p: ZeroSizeWithPhantomData); //~ ERROR found zero-sized type + | ^^^^^^^^^^^^^^^^^^^^^^^ + +error: found zero-sized type composed only of phantom-data in a foreign-function. + --> $DIR/lint-ctypes.rs:68:12 + | +68 | -> ::std::marker::PhantomData; //~ ERROR: found zero-sized type + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: found function pointer with Rust calling convention in foreign module; consider using an `extern` function pointer + --> $DIR/lint-ctypes.rs:69:23 + | +69 | pub fn fn_type(p: RustFn); //~ ERROR found function pointer with Rust + | ^^^^^^ + +error: found function pointer with Rust calling convention in foreign module; consider using an `extern` function pointer + --> $DIR/lint-ctypes.rs:70:24 + | +70 | pub fn fn_type2(p: fn()); //~ ERROR found function pointer with Rust + | ^^^^ + +error: found struct without foreign-function-safe representation annotation in foreign module, consider adding a #[repr(C)] attribute to the type + --> $DIR/lint-ctypes.rs:71:28 + | +71 | pub fn fn_contained(p: RustBadRet); //~ ERROR: found struct without + | ^^^^^^^^^^ + +error: found non-foreign-function-safe member in struct marked #[repr(C)]: found Rust type `i128` in foreign module, but 128-bit integers don't currently have a known stable ABI + --> $DIR/lint-ctypes.rs:72:32 + | +72 | pub fn transparent_i128(p: TransparentI128); //~ ERROR: found Rust type `i128` + | ^^^^^^^^^^^^^^^ + +error: found non-foreign-function-safe member in struct marked #[repr(C)]: found Rust type `str` in foreign module; consider using a `*const libc::c_char` + --> $DIR/lint-ctypes.rs:73:31 + | +73 | pub fn transparent_str(p: TransparentStr); //~ ERROR: found Rust type `str` + | ^^^^^^^^^^^^^^ + +error: found non-foreign-function-safe member in struct marked #[repr(C)]: found struct without foreign-function-safe representation annotation in foreign module, consider adding a #[repr(C)] attribute to the type + --> $DIR/lint-ctypes.rs:74:30 + | +74 | pub fn transparent_fn(p: TransparentBadFn); //~ ERROR: found struct without + | ^^^^^^^^^^^^^^^^ + +error: aborting due to 20 previous errors + From 7ac5e96f4af8efe4a7f09a873d81006329cb5133 Mon Sep 17 00:00:00 2001 From: Robin Kruppe Date: Sun, 11 Feb 2018 21:31:42 +0100 Subject: [PATCH 2/7] [improper_ctypes] Use a 'help:' line for possible fixes --- src/librustc_lint/types.rs | 177 +++++++++++++++++++++------------ src/test/ui/lint-ctypes.stderr | 59 ++++++++--- 2 files changed, 156 insertions(+), 80 deletions(-) diff --git a/src/librustc_lint/types.rs b/src/librustc_lint/types.rs index f734f3182a9..b6e8ae96942 100644 --- a/src/librustc_lint/types.rs +++ b/src/librustc_lint/types.rs @@ -353,13 +353,18 @@ struct ImproperCTypesVisitor<'a, 'tcx: 'a> { cx: &'a LateContext<'a, 'tcx>, } +struct FfiError { + message: &'static str, + help: Option<&'static str>, +} + enum FfiResult { FfiSafe, FfiPhantom, - FfiUnsafe(&'static str), - FfiBadStruct(DefId, &'static str), - FfiBadUnion(DefId, &'static str), - FfiBadEnum(DefId, &'static str), + FfiUnsafe(FfiError), + FfiBadStruct(DefId, FfiError), + FfiBadUnion(DefId, FfiError), + FfiBadEnum(DefId, FfiError), } /// Check if this enum can be safely exported based on the @@ -434,14 +439,18 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { match def.adt_kind() { AdtKind::Struct => { if !def.repr.c() && !def.repr.transparent() { - return FfiUnsafe("found struct without foreign-function-safe \ - representation annotation in foreign module, \ - consider adding a #[repr(C)] attribute to the type"); + return FfiUnsafe(FfiError { + message: "found struct without foreign-function-safe \ + representation annotation in foreign module", + help: Some("consider adding a #[repr(C)] attribute to the type"), + }); } if def.non_enum_variant().fields.is_empty() { - return FfiUnsafe("found zero-size struct in foreign module, consider \ - adding a member to this struct"); + return FfiUnsafe(FfiError { + message: "found zero-size struct in foreign module", + help: Some("consider adding a member to this struct"), + }); } // We can't completely trust repr(C) and repr(transparent) markings; @@ -471,8 +480,8 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { FfiBadStruct(..) | FfiBadUnion(..) | FfiBadEnum(..) => { return r; } - FfiUnsafe(s) => { - return FfiBadStruct(def.did, s); + FfiUnsafe(err) => { + return FfiBadStruct(def.did, err); } } } @@ -481,14 +490,18 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } AdtKind::Union => { if !def.repr.c() { - return FfiUnsafe("found union without foreign-function-safe \ - representation annotation in foreign module, \ - consider adding a #[repr(C)] attribute to the type"); + return FfiUnsafe(FfiError { + message: "found union without foreign-function-safe \ + representation annotation in foreign module", + help: Some("consider adding a #[repr(C)] attribute to the type"), + }); } if def.non_enum_variant().fields.is_empty() { - return FfiUnsafe("found zero-size union in foreign module, consider \ - adding a member to this union"); + return FfiUnsafe(FfiError { + message: "found zero-size union in foreign module", + help: Some("consider adding a member to this union"), + }); } let mut all_phantom = true; @@ -505,8 +518,8 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { FfiBadStruct(..) | FfiBadUnion(..) | FfiBadEnum(..) => { return r; } - FfiUnsafe(s) => { - return FfiBadUnion(def.did, s); + FfiUnsafe(err) => { + return FfiBadUnion(def.did, err); } } } @@ -524,10 +537,12 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { if !def.repr.c() && def.repr.int.is_none() { // Special-case types like `Option`. if !is_repr_nullable_ptr(cx, def, substs) { - return FfiUnsafe("found enum without foreign-function-safe \ - representation annotation in foreign \ - module, consider adding a #[repr(...)] \ - attribute to the type"); + return FfiUnsafe(FfiError { + message: "found enum without foreign-function-safe \ + representation annotation in foreign module", + help: Some("consider adding a #[repr(...)] attribute \ + to the type"), + }); } } @@ -535,7 +550,10 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { if !is_ffi_safe(int_ty) { // FIXME: This shouldn't be reachable: we should check // this earlier. - return FfiUnsafe("enum has unexpected #[repr(...)] attribute"); + return FfiUnsafe(FfiError { + message: "enum has unexpected #[repr(...)] attribute", + help: None, + }); } // Enum with an explicitly sized discriminant; either @@ -558,11 +576,13 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { return r; } FfiPhantom => { - return FfiBadEnum(def.did, - "Found phantom data in enum variant"); + return FfiBadEnum(def.did, FfiError { + message: "Found phantom data in enum variant", + help: None, + }); } - FfiUnsafe(s) => { - return FfiBadEnum(def.did, s); + FfiUnsafe(err) => { + return FfiBadEnum(def.did, err); } } } @@ -573,43 +593,57 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } ty::TyChar => { - FfiUnsafe("found Rust type `char` in foreign module, while \ - `u32` or `libc::wchar_t` should be used") + FfiUnsafe(FfiError { + message: "found Rust type `char` in foreign module", + help: Some("consider using `u32` or `libc::wchar_t`"), + }) } ty::TyInt(ast::IntTy::I128) => { - FfiUnsafe("found Rust type `i128` in foreign module, but \ - 128-bit integers don't currently have a known \ - stable ABI") + FfiUnsafe(FfiError { + message: "found Rust type `i128` in foreign module, but 128-bit \ + integers don't currently have a known stable ABI", + help: None, + }) } ty::TyUint(ast::UintTy::U128) => { - FfiUnsafe("found Rust type `u128` in foreign module, but \ - 128-bit integers don't currently have a known \ - stable ABI") + FfiUnsafe(FfiError { + message: "found Rust type `u128` in foreign module, but 128-bit \ + integers don't currently have a known stable ABI", + help: None, + }) } // Primitive types with a stable representation. ty::TyBool | ty::TyInt(..) | ty::TyUint(..) | ty::TyFloat(..) | ty::TyNever => FfiSafe, ty::TySlice(_) => { - FfiUnsafe("found Rust slice type in foreign module, \ - consider using a raw pointer instead") + FfiUnsafe(FfiError { + message: "found Rust slice type in foreign module", + help: Some("consider using a raw pointer instead"), + }) } ty::TyDynamic(..) => { - FfiUnsafe("found Rust trait type in foreign module, \ - consider using a raw pointer instead") + FfiUnsafe(FfiError { + message: "found Rust trait type in foreign module", + help: Some("consider using a raw pointer instead"), + }) } ty::TyStr => { - FfiUnsafe("found Rust type `str` in foreign module; \ - consider using a `*const libc::c_char`") + FfiUnsafe(FfiError { + message: "found Rust type `str` in foreign module", + help: Some("consider using a `*const libc::c_char`"), + }) } ty::TyTuple(..) => { - FfiUnsafe("found Rust tuple type in foreign module; \ - consider using a struct instead") + FfiUnsafe(FfiError { + message: "found Rust tuple type in foreign module", + help: Some("consider using a struct instead"), + }) } ty::TyRawPtr(ref m) | @@ -620,9 +654,11 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { ty::TyFnPtr(sig) => { match sig.abi() { Abi::Rust | Abi::RustIntrinsic | Abi::PlatformIntrinsic | Abi::RustCall => { - return FfiUnsafe("found function pointer with Rust calling convention in \ - foreign module; consider using an `extern` function \ - pointer") + return FfiUnsafe(FfiError { + message: "found function pointer with Rust calling convention in \ + foreign module", + help: Some("consider using an `extern` function pointer"), + }) } _ => {} } @@ -676,34 +712,45 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { &format!("found zero-sized type composed only \ of phantom-data in a foreign-function.")); } - FfiResult::FfiUnsafe(s) => { - self.cx.span_lint(IMPROPER_CTYPES, sp, s); + FfiResult::FfiUnsafe(err) => { + let mut diag = self.cx.struct_span_lint(IMPROPER_CTYPES, sp, err.message); + if let Some(s) = err.help { + diag.help(s); + } + diag.emit(); } - FfiResult::FfiBadStruct(_, s) => { + FfiResult::FfiBadStruct(_, err) => { // FIXME: This diagnostic is difficult to read, and doesn't // point at the relevant field. - self.cx.span_lint(IMPROPER_CTYPES, - sp, - &format!("found non-foreign-function-safe member in struct \ - marked #[repr(C)]: {}", - s)); + let msg = format!("found non-foreign-function-safe member in struct \ + marked #[repr(C)]: {}", err.message); + let mut diag = self.cx.struct_span_lint(IMPROPER_CTYPES, sp, &msg); + if let Some(s) = err.help { + diag.help(s); + } + diag.emit(); } - FfiResult::FfiBadUnion(_, s) => { + FfiResult::FfiBadUnion(_, err) => { // FIXME: This diagnostic is difficult to read, and doesn't // point at the relevant field. - self.cx.span_lint(IMPROPER_CTYPES, - sp, - &format!("found non-foreign-function-safe member in union \ - marked #[repr(C)]: {}", - s)); + let msg = format!("found non-foreign-function-safe member in union \ + marked #[repr(C)]: {}", err.message); + let mut diag = self.cx.struct_span_lint(IMPROPER_CTYPES, sp, &msg); + if let Some(s) = err.help { + diag.help(s); + } + diag.emit(); } - FfiResult::FfiBadEnum(_, s) => { + FfiResult::FfiBadEnum(_, err) => { // FIXME: This diagnostic is difficult to read, and doesn't // point at the relevant variant. - self.cx.span_lint(IMPROPER_CTYPES, - sp, - &format!("found non-foreign-function-safe member in enum: {}", - s)); + let msg = format!("found non-foreign-function-safe member in enum: {}", + err.message); + let mut diag = self.cx.struct_span_lint(IMPROPER_CTYPES, sp, &msg); + if let Some(s) = err.help { + diag.help(s); + } + diag.emit(); } } } diff --git a/src/test/ui/lint-ctypes.stderr b/src/test/ui/lint-ctypes.stderr index 21aadde8ac8..0f7f7e048e3 100644 --- a/src/test/ui/lint-ctypes.stderr +++ b/src/test/ui/lint-ctypes.stderr @@ -1,4 +1,4 @@ -error: found struct without foreign-function-safe representation annotation in foreign module, consider adding a #[repr(C)] attribute to the type +error: found struct without foreign-function-safe representation annotation in foreign module --> $DIR/lint-ctypes.rs:54:28 | 54 | pub fn ptr_type1(size: *const Foo); //~ ERROR: found struct without @@ -9,36 +9,47 @@ note: lint level defined here | 11 | #![deny(improper_ctypes)] | ^^^^^^^^^^^^^^^ + = help: consider adding a #[repr(C)] attribute to the type -error: found struct without foreign-function-safe representation annotation in foreign module, consider adding a #[repr(C)] attribute to the type +error: found struct without foreign-function-safe representation annotation in foreign module --> $DIR/lint-ctypes.rs:55:28 | 55 | pub fn ptr_type2(size: *const Foo); //~ ERROR: found struct without | ^^^^^^^^^^ + | + = help: consider adding a #[repr(C)] attribute to the type -error: found Rust slice type in foreign module, consider using a raw pointer instead +error: found Rust slice type in foreign module --> $DIR/lint-ctypes.rs:56:26 | 56 | pub fn slice_type(p: &[u32]); //~ ERROR: found Rust slice type | ^^^^^^ + | + = help: consider using a raw pointer instead -error: found Rust type `str` in foreign module; consider using a `*const libc::c_char` +error: found Rust type `str` in foreign module --> $DIR/lint-ctypes.rs:57:24 | 57 | pub fn str_type(p: &str); //~ ERROR: found Rust type | ^^^^ + | + = help: consider using a `*const libc::c_char` -error: found struct without foreign-function-safe representation annotation in foreign module, consider adding a #[repr(C)] attribute to the type +error: found struct without foreign-function-safe representation annotation in foreign module --> $DIR/lint-ctypes.rs:58:24 | 58 | pub fn box_type(p: Box); //~ ERROR found struct without | ^^^^^^^^ + | + = help: consider adding a #[repr(C)] attribute to the type -error: found Rust type `char` in foreign module, while `u32` or `libc::wchar_t` should be used +error: found Rust type `char` in foreign module --> $DIR/lint-ctypes.rs:59:25 | 59 | pub fn char_type(p: char); //~ ERROR found Rust type | ^^^^ + | + = help: consider using `u32` or `libc::wchar_t` error: found Rust type `i128` in foreign module, but 128-bit integers don't currently have a known stable ABI --> $DIR/lint-ctypes.rs:60:25 @@ -52,29 +63,37 @@ error: found Rust type `u128` in foreign module, but 128-bit integers don't curr 61 | pub fn u128_type(p: u128); //~ ERROR found Rust type | ^^^^ -error: found Rust trait type in foreign module, consider using a raw pointer instead +error: found Rust trait type in foreign module --> $DIR/lint-ctypes.rs:62:26 | 62 | pub fn trait_type(p: &Clone); //~ ERROR found Rust trait type | ^^^^^^ + | + = help: consider using a raw pointer instead -error: found Rust tuple type in foreign module; consider using a struct instead +error: found Rust tuple type in foreign module --> $DIR/lint-ctypes.rs:63:26 | 63 | pub fn tuple_type(p: (i32, i32)); //~ ERROR found Rust tuple type | ^^^^^^^^^^ + | + = help: consider using a struct instead -error: found Rust tuple type in foreign module; consider using a struct instead +error: found Rust tuple type in foreign module --> $DIR/lint-ctypes.rs:64:27 | 64 | pub fn tuple_type2(p: I32Pair); //~ ERROR found Rust tuple type | ^^^^^^^ + | + = help: consider using a struct instead -error: found zero-size struct in foreign module, consider adding a member to this struct +error: found zero-size struct in foreign module --> $DIR/lint-ctypes.rs:65:25 | 65 | pub fn zero_size(p: ZeroSize); //~ ERROR found zero-size struct | ^^^^^^^^ + | + = help: consider adding a member to this struct error: found zero-sized type composed only of phantom-data in a foreign-function. --> $DIR/lint-ctypes.rs:66:33 @@ -88,23 +107,29 @@ error: found zero-sized type composed only of phantom-data in a foreign-function 68 | -> ::std::marker::PhantomData; //~ ERROR: found zero-sized type | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: found function pointer with Rust calling convention in foreign module; consider using an `extern` function pointer +error: found function pointer with Rust calling convention in foreign module --> $DIR/lint-ctypes.rs:69:23 | 69 | pub fn fn_type(p: RustFn); //~ ERROR found function pointer with Rust | ^^^^^^ + | + = help: consider using an `extern` function pointer -error: found function pointer with Rust calling convention in foreign module; consider using an `extern` function pointer +error: found function pointer with Rust calling convention in foreign module --> $DIR/lint-ctypes.rs:70:24 | 70 | pub fn fn_type2(p: fn()); //~ ERROR found function pointer with Rust | ^^^^ + | + = help: consider using an `extern` function pointer -error: found struct without foreign-function-safe representation annotation in foreign module, consider adding a #[repr(C)] attribute to the type +error: found struct without foreign-function-safe representation annotation in foreign module --> $DIR/lint-ctypes.rs:71:28 | 71 | pub fn fn_contained(p: RustBadRet); //~ ERROR: found struct without | ^^^^^^^^^^ + | + = help: consider adding a #[repr(C)] attribute to the type error: found non-foreign-function-safe member in struct marked #[repr(C)]: found Rust type `i128` in foreign module, but 128-bit integers don't currently have a known stable ABI --> $DIR/lint-ctypes.rs:72:32 @@ -112,17 +137,21 @@ error: found non-foreign-function-safe member in struct marked #[repr(C)]: found 72 | pub fn transparent_i128(p: TransparentI128); //~ ERROR: found Rust type `i128` | ^^^^^^^^^^^^^^^ -error: found non-foreign-function-safe member in struct marked #[repr(C)]: found Rust type `str` in foreign module; consider using a `*const libc::c_char` +error: found non-foreign-function-safe member in struct marked #[repr(C)]: found Rust type `str` in foreign module --> $DIR/lint-ctypes.rs:73:31 | 73 | pub fn transparent_str(p: TransparentStr); //~ ERROR: found Rust type `str` | ^^^^^^^^^^^^^^ + | + = help: consider using a `*const libc::c_char` -error: found non-foreign-function-safe member in struct marked #[repr(C)]: found struct without foreign-function-safe representation annotation in foreign module, consider adding a #[repr(C)] attribute to the type +error: found non-foreign-function-safe member in struct marked #[repr(C)]: found struct without foreign-function-safe representation annotation in foreign module --> $DIR/lint-ctypes.rs:74:30 | 74 | pub fn transparent_fn(p: TransparentBadFn); //~ ERROR: found struct without | ^^^^^^^^^^^^^^^^ + | + = help: consider adding a #[repr(C)] attribute to the type error: aborting due to 20 previous errors From ae92dfac5019643b8fb310de9e92f0889b0106ca Mon Sep 17 00:00:00 2001 From: Robin Kruppe Date: Sun, 11 Feb 2018 21:54:56 +0100 Subject: [PATCH 3/7] [improper_ctypes] Stop complaining about repr(usize) and repr(isize) enums This dates back to at least #26583. At the time, usize and isize were considered ffi-unsafe to nudge people away from them, but this changed in the aforementioned PR, making it inconsistent to complain about it in enum discriminants. In fact, repr(usize) is probably the best way to interface with `enum Foo : size_t { ... }`. --- src/librustc_lint/types.rs | 29 ----------------------- src/test/compile-fail/lint-ctypes-enum.rs | 12 ++++++++++ 2 files changed, 12 insertions(+), 29 deletions(-) diff --git a/src/librustc_lint/types.rs b/src/librustc_lint/types.rs index b6e8ae96942..f17fa9c7ca1 100644 --- a/src/librustc_lint/types.rs +++ b/src/librustc_lint/types.rs @@ -26,7 +26,6 @@ use std::{i8, i16, i32, i64, u8, u16, u32, u64, f32, f64}; use syntax::ast; use syntax::abi::Abi; -use syntax::attr; use syntax_pos::Span; use syntax::codemap; @@ -402,17 +401,6 @@ fn is_repr_nullable_ptr<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, false } -fn is_ffi_safe(ty: attr::IntType) -> bool { - match ty { - attr::SignedInt(ast::IntTy::I8) | attr::UnsignedInt(ast::UintTy::U8) | - attr::SignedInt(ast::IntTy::I16) | attr::UnsignedInt(ast::UintTy::U16) | - attr::SignedInt(ast::IntTy::I32) | attr::UnsignedInt(ast::UintTy::U32) | - attr::SignedInt(ast::IntTy::I64) | attr::UnsignedInt(ast::UintTy::U64) | - attr::SignedInt(ast::IntTy::I128) | attr::UnsignedInt(ast::UintTy::U128) => true, - attr::SignedInt(ast::IntTy::Isize) | attr::UnsignedInt(ast::UintTy::Usize) => false - } -} - impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { /// Check if the given type is "ffi-safe" (has a stable, well-defined /// representation which can be exported to C code). @@ -546,23 +534,6 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } } - if let Some(int_ty) = def.repr.int { - if !is_ffi_safe(int_ty) { - // FIXME: This shouldn't be reachable: we should check - // this earlier. - return FfiUnsafe(FfiError { - message: "enum has unexpected #[repr(...)] attribute", - help: None, - }); - } - - // Enum with an explicitly sized discriminant; either - // a C-style enum or a discriminated union. - - // The layout of enum variants is implicitly repr(C). - // FIXME: Is that correct? - } - // Check the contained variants. for variant in &def.variants { for field in &variant.fields { diff --git a/src/test/compile-fail/lint-ctypes-enum.rs b/src/test/compile-fail/lint-ctypes-enum.rs index e35dadbea9d..ce6afe1d219 100644 --- a/src/test/compile-fail/lint-ctypes-enum.rs +++ b/src/test/compile-fail/lint-ctypes-enum.rs @@ -16,11 +16,23 @@ enum U { A } enum B { C, D } enum T { E, F, G } +#[repr(C)] +enum ReprC { A, B, C } + +#[repr(u8)] +enum U8 { A, B, C } + +#[repr(isize)] +enum Isize { A, B, C } + extern { fn zf(x: Z); fn uf(x: U); //~ ERROR found enum without foreign-function-safe fn bf(x: B); //~ ERROR found enum without foreign-function-safe fn tf(x: T); //~ ERROR found enum without foreign-function-safe + fn reprc(x: ReprC); + fn u8(x: U8); + fn isize(x: Isize); } pub fn main() { } From 9b5f47ec48e8c12b68cff7cc64afe166358183ef Mon Sep 17 00:00:00 2001 From: Robin Kruppe Date: Mon, 12 Feb 2018 01:08:48 +0100 Subject: [PATCH 4/7] [improper_ctypes] Overhaul primary label - Always name the non-FFI-safe - Explain *why* the type is not FFI-safe - Stop vaguely gesturing at structs/enums/unions if the non-FFI-safe types occured in a field. The last part is arguably a regression, but it's minor now that the non-FFI-safe type is actually named. Removing it avoids some code duplication. --- src/librustc_lint/types.rs | 232 ++++++++------------ src/test/compile-fail/issue-14309.rs | 10 +- src/test/compile-fail/issue-16250.rs | 2 +- src/test/compile-fail/lint-ctypes-enum.rs | 6 +- src/test/compile-fail/union/union-repr-c.rs | 2 +- src/test/ui/lint-ctypes.rs | 40 ++-- src/test/ui/lint-ctypes.stderr | 100 ++++----- 7 files changed, 168 insertions(+), 224 deletions(-) diff --git a/src/librustc_lint/types.rs b/src/librustc_lint/types.rs index f17fa9c7ca1..7203b1b1e7d 100644 --- a/src/librustc_lint/types.rs +++ b/src/librustc_lint/types.rs @@ -10,7 +10,6 @@ #![allow(non_snake_case)] -use rustc::hir::def_id::DefId; use rustc::hir::map as hir_map; use rustc::ty::subst::Substs; use rustc::ty::{self, AdtKind, Ty, TyCtxt}; @@ -352,18 +351,14 @@ struct ImproperCTypesVisitor<'a, 'tcx: 'a> { cx: &'a LateContext<'a, 'tcx>, } -struct FfiError { - message: &'static str, - help: Option<&'static str>, -} - -enum FfiResult { +enum FfiResult<'tcx> { FfiSafe, - FfiPhantom, - FfiUnsafe(FfiError), - FfiBadStruct(DefId, FfiError), - FfiBadUnion(DefId, FfiError), - FfiBadEnum(DefId, FfiError), + FfiPhantom(Ty<'tcx>), + FfiUnsafe { + ty: Ty<'tcx>, + reason: &'static str, + help: Option<&'static str>, + }, } /// Check if this enum can be safely exported based on the @@ -406,7 +401,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { /// representation which can be exported to C code). fn check_type_for_ffi(&self, cache: &mut FxHashSet>, - ty: Ty<'tcx>) -> FfiResult { + ty: Ty<'tcx>) -> FfiResult<'tcx> { use self::FfiResult::*; let cx = self.cx.tcx; @@ -422,23 +417,24 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { match ty.sty { ty::TyAdt(def, substs) => { if def.is_phantom_data() { - return FfiPhantom; + return FfiPhantom(ty); } match def.adt_kind() { AdtKind::Struct => { if !def.repr.c() && !def.repr.transparent() { - return FfiUnsafe(FfiError { - message: "found struct without foreign-function-safe \ - representation annotation in foreign module", - help: Some("consider adding a #[repr(C)] attribute to the type"), - }); + return FfiUnsafe { + ty: ty, + reason: "this struct has unspecified layout", + help: Some("consider adding a #[repr(C)] attribute to this struct"), + }; } if def.non_enum_variant().fields.is_empty() { - return FfiUnsafe(FfiError { - message: "found zero-size struct in foreign module", + return FfiUnsafe { + ty: ty, + reason: "this struct has no fields", help: Some("consider adding a member to this struct"), - }); + }; } // We can't completely trust repr(C) and repr(transparent) markings; @@ -464,32 +460,30 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { FfiSafe => { all_phantom = false; } - FfiPhantom => {} - FfiBadStruct(..) | FfiBadUnion(..) | FfiBadEnum(..) => { + FfiPhantom(..) => {} + FfiUnsafe { .. } => { return r; } - FfiUnsafe(err) => { - return FfiBadStruct(def.did, err); - } } } - if all_phantom { FfiPhantom } else { FfiSafe } + if all_phantom { FfiPhantom(ty) } else { FfiSafe } } AdtKind::Union => { if !def.repr.c() { - return FfiUnsafe(FfiError { - message: "found union without foreign-function-safe \ - representation annotation in foreign module", - help: Some("consider adding a #[repr(C)] attribute to the type"), - }); + return FfiUnsafe { + ty: ty, + reason: "this union has unspecified layout", + help: Some("consider adding a #[repr(C)] attribute to this union"), + }; } if def.non_enum_variant().fields.is_empty() { - return FfiUnsafe(FfiError { - message: "found zero-size union in foreign module", - help: Some("consider adding a member to this union"), - }); + return FfiUnsafe { + ty: ty, + reason: "this union has no fields", + help: Some("consider adding a field to this union"), + }; } let mut all_phantom = true; @@ -502,17 +496,14 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { FfiSafe => { all_phantom = false; } - FfiPhantom => {} - FfiBadStruct(..) | FfiBadUnion(..) | FfiBadEnum(..) => { + FfiPhantom(..) => {} + FfiUnsafe { .. } => { return r; } - FfiUnsafe(err) => { - return FfiBadUnion(def.did, err); - } } } - if all_phantom { FfiPhantom } else { FfiSafe } + if all_phantom { FfiPhantom(ty) } else { FfiSafe } } AdtKind::Enum => { if def.variants.is_empty() { @@ -525,12 +516,12 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { if !def.repr.c() && def.repr.int.is_none() { // Special-case types like `Option`. if !is_repr_nullable_ptr(cx, def, substs) { - return FfiUnsafe(FfiError { - message: "found enum without foreign-function-safe \ - representation annotation in foreign module", + return FfiUnsafe { + ty: ty, + reason: "enum has no representation hint", help: Some("consider adding a #[repr(...)] attribute \ - to the type"), - }); + to this enum"), + }; } } @@ -543,17 +534,15 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { let r = self.check_type_for_ffi(cache, arg); match r { FfiSafe => {} - FfiBadStruct(..) | FfiBadUnion(..) | FfiBadEnum(..) => { + FfiUnsafe { .. } => { return r; } - FfiPhantom => { - return FfiBadEnum(def.did, FfiError { - message: "Found phantom data in enum variant", + FfiPhantom(..) => { + return FfiUnsafe { + ty: ty, + reason: "this enum contains a PhantomData field", help: None, - }); - } - FfiUnsafe(err) => { - return FfiBadEnum(def.did, err); + }; } } } @@ -563,59 +552,44 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } } - ty::TyChar => { - FfiUnsafe(FfiError { - message: "found Rust type `char` in foreign module", - help: Some("consider using `u32` or `libc::wchar_t`"), - }) - } + ty::TyChar => FfiUnsafe { + ty: ty, + reason: "the `char` type has no C equivalent", + help: Some("consider using `u32` or `libc::wchar_t` instead"), + }, - ty::TyInt(ast::IntTy::I128) => { - FfiUnsafe(FfiError { - message: "found Rust type `i128` in foreign module, but 128-bit \ - integers don't currently have a known stable ABI", - help: None, - }) - } - - ty::TyUint(ast::UintTy::U128) => { - FfiUnsafe(FfiError { - message: "found Rust type `u128` in foreign module, but 128-bit \ - integers don't currently have a known stable ABI", - help: None, - }) - } + ty::TyInt(ast::IntTy::I128) | ty::TyUint(ast::UintTy::U128) => FfiUnsafe { + ty: ty, + reason: "128-bit integers don't currently have a known stable ABI", + help: None, + }, // Primitive types with a stable representation. ty::TyBool | ty::TyInt(..) | ty::TyUint(..) | ty::TyFloat(..) | ty::TyNever => FfiSafe, - ty::TySlice(_) => { - FfiUnsafe(FfiError { - message: "found Rust slice type in foreign module", - help: Some("consider using a raw pointer instead"), - }) - } + ty::TySlice(_) => FfiUnsafe { + ty: ty, + reason: "slices have no C equivalent", + help: Some("consider using a raw pointer instead"), + }, - ty::TyDynamic(..) => { - FfiUnsafe(FfiError { - message: "found Rust trait type in foreign module", - help: Some("consider using a raw pointer instead"), - }) - } + ty::TyDynamic(..) => FfiUnsafe { + ty: ty, + reason: "trait objects have no C equivalent", + help: Some("consider using a raw pointer instead"), + }, - ty::TyStr => { - FfiUnsafe(FfiError { - message: "found Rust type `str` in foreign module", - help: Some("consider using a `*const libc::c_char`"), - }) - } + ty::TyStr => FfiUnsafe { + ty: ty, + reason: "string slices have no C equivalent", + help: Some("consider using `*const u8` and a length instead"), + }, - ty::TyTuple(..) => { - FfiUnsafe(FfiError { - message: "found Rust tuple type in foreign module", - help: Some("consider using a struct instead"), - }) - } + ty::TyTuple(..) => FfiUnsafe { + ty: ty, + reason: "tuples have unspecified layout", + help: Some("consider using a struct instead"), + }, ty::TyRawPtr(ref m) | ty::TyRef(_, ref m) => self.check_type_for_ffi(cache, m.ty), @@ -625,11 +599,12 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { ty::TyFnPtr(sig) => { match sig.abi() { Abi::Rust | Abi::RustIntrinsic | Abi::PlatformIntrinsic | Abi::RustCall => { - return FfiUnsafe(FfiError { - message: "found function pointer with Rust calling convention in \ - foreign module", - help: Some("consider using an `extern` function pointer"), - }) + return FfiUnsafe { + ty: ty, + reason: "this function pointer has Rust-specific calling convention", + help: Some("consider using an `fn \"extern\"(...) -> ...` \ + function pointer instead"), + } } _ => {} } @@ -677,48 +652,17 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { match self.check_type_for_ffi(&mut FxHashSet(), ty) { FfiResult::FfiSafe => {} - FfiResult::FfiPhantom => { + FfiResult::FfiPhantom(ty) => { self.cx.span_lint(IMPROPER_CTYPES, sp, - &format!("found zero-sized type composed only \ - of phantom-data in a foreign-function.")); + &format!("`extern` block uses type `{}` which is not FFI-safe: \ + composed only of PhantomData", ty)); } - FfiResult::FfiUnsafe(err) => { - let mut diag = self.cx.struct_span_lint(IMPROPER_CTYPES, sp, err.message); - if let Some(s) = err.help { - diag.help(s); - } - diag.emit(); - } - FfiResult::FfiBadStruct(_, err) => { - // FIXME: This diagnostic is difficult to read, and doesn't - // point at the relevant field. - let msg = format!("found non-foreign-function-safe member in struct \ - marked #[repr(C)]: {}", err.message); + FfiResult::FfiUnsafe { ty: unsafe_ty, reason, help } => { + let msg = format!("`extern` block uses type `{}` which is not FFI-safe: {}", + unsafe_ty, reason); let mut diag = self.cx.struct_span_lint(IMPROPER_CTYPES, sp, &msg); - if let Some(s) = err.help { - diag.help(s); - } - diag.emit(); - } - FfiResult::FfiBadUnion(_, err) => { - // FIXME: This diagnostic is difficult to read, and doesn't - // point at the relevant field. - let msg = format!("found non-foreign-function-safe member in union \ - marked #[repr(C)]: {}", err.message); - let mut diag = self.cx.struct_span_lint(IMPROPER_CTYPES, sp, &msg); - if let Some(s) = err.help { - diag.help(s); - } - diag.emit(); - } - FfiResult::FfiBadEnum(_, err) => { - // FIXME: This diagnostic is difficult to read, and doesn't - // point at the relevant variant. - let msg = format!("found non-foreign-function-safe member in enum: {}", - err.message); - let mut diag = self.cx.struct_span_lint(IMPROPER_CTYPES, sp, &msg); - if let Some(s) = err.help { + if let Some(s) = help { diag.help(s); } diag.emit(); diff --git a/src/test/compile-fail/issue-14309.rs b/src/test/compile-fail/issue-14309.rs index 56261c34a03..f76fa3e4a8e 100644 --- a/src/test/compile-fail/issue-14309.rs +++ b/src/test/compile-fail/issue-14309.rs @@ -37,13 +37,13 @@ struct D { } extern "C" { - fn foo(x: A); //~ ERROR found struct without foreign-function-safe - fn bar(x: B); //~ ERROR foreign-function-safe + fn foo(x: A); //~ ERROR type `A` which is not FFI-safe + fn bar(x: B); //~ ERROR type `A` fn baz(x: C); - fn qux(x: A2); //~ ERROR foreign-function-safe - fn quux(x: B2); //~ ERROR foreign-function-safe + fn qux(x: A2); //~ ERROR type `A` + fn quux(x: B2); //~ ERROR type `A` fn corge(x: C2); - fn fred(x: D); //~ ERROR foreign-function-safe + fn fred(x: D); //~ ERROR type `A` } fn main() { } diff --git a/src/test/compile-fail/issue-16250.rs b/src/test/compile-fail/issue-16250.rs index 288fe4a9abb..f9d01003005 100644 --- a/src/test/compile-fail/issue-16250.rs +++ b/src/test/compile-fail/issue-16250.rs @@ -13,7 +13,7 @@ pub struct Foo; extern { - pub fn foo(x: (Foo)); //~ ERROR found struct without + pub fn foo(x: (Foo)); //~ ERROR unspecified layout } fn main() { diff --git a/src/test/compile-fail/lint-ctypes-enum.rs b/src/test/compile-fail/lint-ctypes-enum.rs index ce6afe1d219..7b7ffd8fc10 100644 --- a/src/test/compile-fail/lint-ctypes-enum.rs +++ b/src/test/compile-fail/lint-ctypes-enum.rs @@ -27,9 +27,9 @@ enum Isize { A, B, C } extern { fn zf(x: Z); - fn uf(x: U); //~ ERROR found enum without foreign-function-safe - fn bf(x: B); //~ ERROR found enum without foreign-function-safe - fn tf(x: T); //~ ERROR found enum without foreign-function-safe + fn uf(x: U); //~ ERROR enum has no representation hint + fn bf(x: B); //~ ERROR enum has no representation hint + fn tf(x: T); //~ ERROR enum has no representation hint fn reprc(x: ReprC); fn u8(x: U8); fn isize(x: Isize); diff --git a/src/test/compile-fail/union/union-repr-c.rs b/src/test/compile-fail/union/union-repr-c.rs index 15a4197fe94..36c42ce1104 100644 --- a/src/test/compile-fail/union/union-repr-c.rs +++ b/src/test/compile-fail/union/union-repr-c.rs @@ -22,7 +22,7 @@ union W { extern "C" { static FOREIGN1: U; // OK - static FOREIGN2: W; //~ ERROR found union without foreign-function-safe representation + static FOREIGN2: W; //~ ERROR union has unspecified layout } fn main() {} diff --git a/src/test/ui/lint-ctypes.rs b/src/test/ui/lint-ctypes.rs index c22239dee0a..77cb1ef0f51 100644 --- a/src/test/ui/lint-ctypes.rs +++ b/src/test/ui/lint-ctypes.rs @@ -51,27 +51,27 @@ pub struct TransparentCustomZst(i32, ZeroSize); pub struct ZeroSizeWithPhantomData(::std::marker::PhantomData); extern { - pub fn ptr_type1(size: *const Foo); //~ ERROR: found struct without - pub fn ptr_type2(size: *const Foo); //~ ERROR: found struct without - pub fn slice_type(p: &[u32]); //~ ERROR: found Rust slice type - pub fn str_type(p: &str); //~ ERROR: found Rust type - pub fn box_type(p: Box); //~ ERROR found struct without - pub fn char_type(p: char); //~ ERROR found Rust type - pub fn i128_type(p: i128); //~ ERROR found Rust type - pub fn u128_type(p: u128); //~ ERROR found Rust type - pub fn trait_type(p: &Clone); //~ ERROR found Rust trait type - pub fn tuple_type(p: (i32, i32)); //~ ERROR found Rust tuple type - pub fn tuple_type2(p: I32Pair); //~ ERROR found Rust tuple type - pub fn zero_size(p: ZeroSize); //~ ERROR found zero-size struct - pub fn zero_size_phantom(p: ZeroSizeWithPhantomData); //~ ERROR found zero-sized type + pub fn ptr_type1(size: *const Foo); //~ ERROR: uses type `Foo` + pub fn ptr_type2(size: *const Foo); //~ ERROR: uses type `Foo` + pub fn slice_type(p: &[u32]); //~ ERROR: uses type `[u32]` + pub fn str_type(p: &str); //~ ERROR: uses type `str` + pub fn box_type(p: Box); //~ ERROR uses type `std::boxed::Box` + 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: &Clone); //~ ERROR uses type `std::clone::Clone` + 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 struct has no fields + pub fn zero_size_phantom(p: ZeroSizeWithPhantomData); //~ ERROR composed only of PhantomData pub fn zero_size_phantom_toplevel() - -> ::std::marker::PhantomData; //~ ERROR: found zero-sized type - pub fn fn_type(p: RustFn); //~ ERROR found function pointer with Rust - pub fn fn_type2(p: fn()); //~ ERROR found function pointer with Rust - pub fn fn_contained(p: RustBadRet); //~ ERROR: found struct without - pub fn transparent_i128(p: TransparentI128); //~ ERROR: found Rust type `i128` - pub fn transparent_str(p: TransparentStr); //~ ERROR: found Rust type `str` - pub fn transparent_fn(p: TransparentBadFn); //~ ERROR: found struct without + -> ::std::marker::PhantomData; //~ ERROR: composed only of PhantomData + pub fn fn_type(p: RustFn); //~ ERROR function pointer has Rust-specific + pub fn fn_type2(p: fn()); //~ ERROR function pointer has Rust-specific + pub fn fn_contained(p: RustBadRet); //~ ERROR: uses type `std::boxed::Box` + pub fn transparent_i128(p: TransparentI128); //~ ERROR: uses type `i128` + pub fn transparent_str(p: TransparentStr); //~ ERROR: uses type `str` + pub fn transparent_fn(p: TransparentBadFn); //~ ERROR: uses type `std::boxed::Box` pub fn good3(fptr: Option); pub fn good4(aptr: &[u8; 4 as usize]); diff --git a/src/test/ui/lint-ctypes.stderr b/src/test/ui/lint-ctypes.stderr index 0f7f7e048e3..8ecdae07a53 100644 --- a/src/test/ui/lint-ctypes.stderr +++ b/src/test/ui/lint-ctypes.stderr @@ -1,7 +1,7 @@ -error: found struct without foreign-function-safe representation annotation in foreign module +error: `extern` block uses type `Foo` which is not FFI-safe: this struct has unspecified layout --> $DIR/lint-ctypes.rs:54:28 | -54 | pub fn ptr_type1(size: *const Foo); //~ ERROR: found struct without +54 | pub fn ptr_type1(size: *const Foo); //~ ERROR: uses type `Foo` | ^^^^^^^^^^ | note: lint level defined here @@ -9,149 +9,149 @@ note: lint level defined here | 11 | #![deny(improper_ctypes)] | ^^^^^^^^^^^^^^^ - = help: consider adding a #[repr(C)] attribute to the type + = help: consider adding a #[repr(C)] attribute to this struct -error: found struct without foreign-function-safe representation annotation in foreign module +error: `extern` block uses type `Foo` which is not FFI-safe: this struct has unspecified layout --> $DIR/lint-ctypes.rs:55:28 | -55 | pub fn ptr_type2(size: *const Foo); //~ ERROR: found struct without +55 | pub fn ptr_type2(size: *const Foo); //~ ERROR: uses type `Foo` | ^^^^^^^^^^ | - = help: consider adding a #[repr(C)] attribute to the type + = help: consider adding a #[repr(C)] attribute to this struct -error: found Rust slice type in foreign module +error: `extern` block uses type `[u32]` which is not FFI-safe: slices have no C equivalent --> $DIR/lint-ctypes.rs:56:26 | -56 | pub fn slice_type(p: &[u32]); //~ ERROR: found Rust slice type +56 | pub fn slice_type(p: &[u32]); //~ ERROR: uses type `[u32]` | ^^^^^^ | = help: consider using a raw pointer instead -error: found Rust type `str` in foreign module +error: `extern` block uses type `str` which is not FFI-safe: string slices have no C equivalent --> $DIR/lint-ctypes.rs:57:24 | -57 | pub fn str_type(p: &str); //~ ERROR: found Rust type +57 | pub fn str_type(p: &str); //~ ERROR: uses type `str` | ^^^^ | - = help: consider using a `*const libc::c_char` + = help: consider using `*const u8` and a length instead -error: found struct without foreign-function-safe representation annotation in foreign module +error: `extern` block uses type `std::boxed::Box` which is not FFI-safe: this struct has unspecified layout --> $DIR/lint-ctypes.rs:58:24 | -58 | pub fn box_type(p: Box); //~ ERROR found struct without +58 | pub fn box_type(p: Box); //~ ERROR uses type `std::boxed::Box` | ^^^^^^^^ | - = help: consider adding a #[repr(C)] attribute to the type + = help: consider adding a #[repr(C)] attribute to this struct -error: found Rust type `char` in foreign module +error: `extern` block uses type `char` which is not FFI-safe: the `char` type has no C equivalent --> $DIR/lint-ctypes.rs:59:25 | -59 | pub fn char_type(p: char); //~ ERROR found Rust type +59 | pub fn char_type(p: char); //~ ERROR uses type `char` | ^^^^ | - = help: consider using `u32` or `libc::wchar_t` + = help: consider using `u32` or `libc::wchar_t` instead -error: found Rust type `i128` in foreign module, but 128-bit integers don't currently have a known stable ABI +error: `extern` block uses type `i128` which is not FFI-safe: 128-bit integers don't currently have a known stable ABI --> $DIR/lint-ctypes.rs:60:25 | -60 | pub fn i128_type(p: i128); //~ ERROR found Rust type +60 | pub fn i128_type(p: i128); //~ ERROR uses type `i128` | ^^^^ -error: found Rust type `u128` in foreign module, but 128-bit integers don't currently have a known stable ABI +error: `extern` block uses type `u128` which is not FFI-safe: 128-bit integers don't currently have a known stable ABI --> $DIR/lint-ctypes.rs:61:25 | -61 | pub fn u128_type(p: u128); //~ ERROR found Rust type +61 | pub fn u128_type(p: u128); //~ ERROR uses type `u128` | ^^^^ -error: found Rust trait type in foreign module +error: `extern` block uses type `std::clone::Clone` which is not FFI-safe: trait objects have no C equivalent --> $DIR/lint-ctypes.rs:62:26 | -62 | pub fn trait_type(p: &Clone); //~ ERROR found Rust trait type +62 | pub fn trait_type(p: &Clone); //~ ERROR uses type `std::clone::Clone` | ^^^^^^ | = help: consider using a raw pointer instead -error: found Rust tuple type in foreign module +error: `extern` block uses type `(i32, i32)` which is not FFI-safe: tuples have unspecified layout --> $DIR/lint-ctypes.rs:63:26 | -63 | pub fn tuple_type(p: (i32, i32)); //~ ERROR found Rust tuple type +63 | pub fn tuple_type(p: (i32, i32)); //~ ERROR uses type `(i32, i32)` | ^^^^^^^^^^ | = help: consider using a struct instead -error: found Rust tuple type in foreign module +error: `extern` block uses type `(i32, i32)` which is not FFI-safe: tuples have unspecified layout --> $DIR/lint-ctypes.rs:64:27 | -64 | pub fn tuple_type2(p: I32Pair); //~ ERROR found Rust tuple type +64 | pub fn tuple_type2(p: I32Pair); //~ ERROR uses type `(i32, i32)` | ^^^^^^^ | = help: consider using a struct instead -error: found zero-size struct in foreign module +error: `extern` block uses type `ZeroSize` which is not FFI-safe: this struct has no fields --> $DIR/lint-ctypes.rs:65:25 | -65 | pub fn zero_size(p: ZeroSize); //~ ERROR found zero-size struct +65 | pub fn zero_size(p: ZeroSize); //~ ERROR struct has no fields | ^^^^^^^^ | = help: consider adding a member to this struct -error: found zero-sized type composed only of phantom-data in a foreign-function. +error: `extern` block uses type `ZeroSizeWithPhantomData` which is not FFI-safe: composed only of PhantomData --> $DIR/lint-ctypes.rs:66:33 | -66 | pub fn zero_size_phantom(p: ZeroSizeWithPhantomData); //~ ERROR found zero-sized type +66 | pub fn zero_size_phantom(p: ZeroSizeWithPhantomData); //~ ERROR composed only of PhantomData | ^^^^^^^^^^^^^^^^^^^^^^^ -error: found zero-sized type composed only of phantom-data in a foreign-function. +error: `extern` block uses type `std::marker::PhantomData` which is not FFI-safe: composed only of PhantomData --> $DIR/lint-ctypes.rs:68:12 | -68 | -> ::std::marker::PhantomData; //~ ERROR: found zero-sized type +68 | -> ::std::marker::PhantomData; //~ ERROR: composed only of PhantomData | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: found function pointer with Rust calling convention in foreign module +error: `extern` block uses type `fn()` which is not FFI-safe: this function pointer has Rust-specific calling convention --> $DIR/lint-ctypes.rs:69:23 | -69 | pub fn fn_type(p: RustFn); //~ ERROR found function pointer with Rust +69 | pub fn fn_type(p: RustFn); //~ ERROR function pointer has Rust-specific | ^^^^^^ | - = help: consider using an `extern` function pointer + = help: consider using an `fn "extern"(...) -> ...` function pointer instead -error: found function pointer with Rust calling convention in foreign module +error: `extern` block uses type `fn()` which is not FFI-safe: this function pointer has Rust-specific calling convention --> $DIR/lint-ctypes.rs:70:24 | -70 | pub fn fn_type2(p: fn()); //~ ERROR found function pointer with Rust +70 | pub fn fn_type2(p: fn()); //~ ERROR function pointer has Rust-specific | ^^^^ | - = help: consider using an `extern` function pointer + = help: consider using an `fn "extern"(...) -> ...` function pointer instead -error: found struct without foreign-function-safe representation annotation in foreign module +error: `extern` block uses type `std::boxed::Box` which is not FFI-safe: this struct has unspecified layout --> $DIR/lint-ctypes.rs:71:28 | -71 | pub fn fn_contained(p: RustBadRet); //~ ERROR: found struct without +71 | pub fn fn_contained(p: RustBadRet); //~ ERROR: uses type `std::boxed::Box` | ^^^^^^^^^^ | - = help: consider adding a #[repr(C)] attribute to the type + = help: consider adding a #[repr(C)] attribute to this struct -error: found non-foreign-function-safe member in struct marked #[repr(C)]: found Rust type `i128` in foreign module, but 128-bit integers don't currently have a known stable ABI +error: `extern` block uses type `i128` which is not FFI-safe: 128-bit integers don't currently have a known stable ABI --> $DIR/lint-ctypes.rs:72:32 | -72 | pub fn transparent_i128(p: TransparentI128); //~ ERROR: found Rust type `i128` +72 | pub fn transparent_i128(p: TransparentI128); //~ ERROR: uses type `i128` | ^^^^^^^^^^^^^^^ -error: found non-foreign-function-safe member in struct marked #[repr(C)]: found Rust type `str` in foreign module +error: `extern` block uses type `str` which is not FFI-safe: string slices have no C equivalent --> $DIR/lint-ctypes.rs:73:31 | -73 | pub fn transparent_str(p: TransparentStr); //~ ERROR: found Rust type `str` +73 | pub fn transparent_str(p: TransparentStr); //~ ERROR: uses type `str` | ^^^^^^^^^^^^^^ | - = help: consider using a `*const libc::c_char` + = help: consider using `*const u8` and a length instead -error: found non-foreign-function-safe member in struct marked #[repr(C)]: found struct without foreign-function-safe representation annotation in foreign module +error: `extern` block uses type `std::boxed::Box` which is not FFI-safe: this struct has unspecified layout --> $DIR/lint-ctypes.rs:74:30 | -74 | pub fn transparent_fn(p: TransparentBadFn); //~ ERROR: found struct without +74 | pub fn transparent_fn(p: TransparentBadFn); //~ ERROR: uses type `std::boxed::Box` | ^^^^^^^^^^^^^^^^ | - = help: consider adding a #[repr(C)] attribute to the type + = help: consider adding a #[repr(C)] attribute to this struct error: aborting due to 20 previous errors From 22a171609bf555833b277a70420a24696dd8805d Mon Sep 17 00:00:00 2001 From: Robin Kruppe Date: Wed, 14 Feb 2018 19:39:04 +0100 Subject: [PATCH 5/7] [improper_ctypes] Suggest repr(transparent) for structs The suggestion is unconditional, so following it could lead to further errors. This is already the case for the repr(C) suggestion, which makes this acceptable, though not *good*. Checking up-front whether the suggestion can help would be great but applies more broadly (and would require some refactoring to avoid duplicating the checks). --- src/librustc_lint/types.rs | 3 ++- src/test/ui/lint-ctypes.stderr | 10 +++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/librustc_lint/types.rs b/src/librustc_lint/types.rs index 7203b1b1e7d..378fe99bf31 100644 --- a/src/librustc_lint/types.rs +++ b/src/librustc_lint/types.rs @@ -425,7 +425,8 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { return FfiUnsafe { ty: ty, reason: "this struct has unspecified layout", - help: Some("consider adding a #[repr(C)] attribute to this struct"), + help: Some("consider adding a #[repr(C)] or #[repr(transparent)] \ + attribute to this struct"), }; } diff --git a/src/test/ui/lint-ctypes.stderr b/src/test/ui/lint-ctypes.stderr index 8ecdae07a53..a8628c8b3d2 100644 --- a/src/test/ui/lint-ctypes.stderr +++ b/src/test/ui/lint-ctypes.stderr @@ -9,7 +9,7 @@ note: lint level defined here | 11 | #![deny(improper_ctypes)] | ^^^^^^^^^^^^^^^ - = help: consider adding a #[repr(C)] attribute to this struct + = help: consider adding a #[repr(C)] or #[repr(transparent)] attribute to this struct error: `extern` block uses type `Foo` which is not FFI-safe: this struct has unspecified layout --> $DIR/lint-ctypes.rs:55:28 @@ -17,7 +17,7 @@ error: `extern` block uses type `Foo` which is not FFI-safe: this struct has uns 55 | pub fn ptr_type2(size: *const Foo); //~ ERROR: uses type `Foo` | ^^^^^^^^^^ | - = help: consider adding a #[repr(C)] attribute to this struct + = help: consider adding a #[repr(C)] or #[repr(transparent)] attribute to this struct error: `extern` block uses type `[u32]` which is not FFI-safe: slices have no C equivalent --> $DIR/lint-ctypes.rs:56:26 @@ -41,7 +41,7 @@ error: `extern` block uses type `std::boxed::Box` which is not FFI-safe: th 58 | pub fn box_type(p: Box); //~ ERROR uses type `std::boxed::Box` | ^^^^^^^^ | - = help: consider adding a #[repr(C)] attribute to this struct + = help: consider adding a #[repr(C)] or #[repr(transparent)] attribute to this struct error: `extern` block uses type `char` which is not FFI-safe: the `char` type has no C equivalent --> $DIR/lint-ctypes.rs:59:25 @@ -129,7 +129,7 @@ error: `extern` block uses type `std::boxed::Box` which is not FFI-safe: th 71 | pub fn fn_contained(p: RustBadRet); //~ ERROR: uses type `std::boxed::Box` | ^^^^^^^^^^ | - = help: consider adding a #[repr(C)] attribute to this struct + = help: consider adding a #[repr(C)] or #[repr(transparent)] attribute to this struct error: `extern` block uses type `i128` which is not FFI-safe: 128-bit integers don't currently have a known stable ABI --> $DIR/lint-ctypes.rs:72:32 @@ -151,7 +151,7 @@ error: `extern` block uses type `std::boxed::Box` which is not FFI-safe: th 74 | pub fn transparent_fn(p: TransparentBadFn); //~ ERROR: uses type `std::boxed::Box` | ^^^^^^^^^^^^^^^^ | - = help: consider adding a #[repr(C)] attribute to this struct + = help: consider adding a #[repr(C)] or #[repr(transparent)] attribute to this struct error: aborting due to 20 previous errors From 9d493c897b4382dc145b9448b3fafdfbbaecf528 Mon Sep 17 00:00:00 2001 From: Robin Kruppe Date: Wed, 14 Feb 2018 23:01:39 +0100 Subject: [PATCH 6/7] [improper_ctypes] Point at definition of non-FFI-safe type if possible --- src/librustc_lint/types.rs | 5 +++++ src/test/ui/lint-ctypes.stderr | 15 +++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/src/librustc_lint/types.rs b/src/librustc_lint/types.rs index 378fe99bf31..396d830d85b 100644 --- a/src/librustc_lint/types.rs +++ b/src/librustc_lint/types.rs @@ -666,6 +666,11 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { if let Some(s) = help { diag.help(s); } + if let ty::TyAdt(def, _) = unsafe_ty.sty { + if let Some(sp) = self.cx.tcx.hir.span_if_local(def.did) { + diag.span_note(sp, "type defined here"); + } + } diag.emit(); } } diff --git a/src/test/ui/lint-ctypes.stderr b/src/test/ui/lint-ctypes.stderr index a8628c8b3d2..2abf08d55f7 100644 --- a/src/test/ui/lint-ctypes.stderr +++ b/src/test/ui/lint-ctypes.stderr @@ -10,6 +10,11 @@ note: lint level defined here 11 | #![deny(improper_ctypes)] | ^^^^^^^^^^^^^^^ = help: consider adding a #[repr(C)] or #[repr(transparent)] attribute to this struct +note: type defined here + --> $DIR/lint-ctypes.rs:32:1 + | +32 | pub struct Foo; + | ^^^^^^^^^^^^^^^ error: `extern` block uses type `Foo` which is not FFI-safe: this struct has unspecified layout --> $DIR/lint-ctypes.rs:55:28 @@ -18,6 +23,11 @@ error: `extern` block uses type `Foo` which is not FFI-safe: this struct has uns | ^^^^^^^^^^ | = help: consider adding a #[repr(C)] or #[repr(transparent)] attribute to this struct +note: type defined here + --> $DIR/lint-ctypes.rs:32:1 + | +32 | pub struct Foo; + | ^^^^^^^^^^^^^^^ error: `extern` block uses type `[u32]` which is not FFI-safe: slices have no C equivalent --> $DIR/lint-ctypes.rs:56:26 @@ -94,6 +104,11 @@ error: `extern` block uses type `ZeroSize` which is not FFI-safe: this struct ha | ^^^^^^^^ | = help: consider adding a member to this struct +note: type defined here + --> $DIR/lint-ctypes.rs:28:1 + | +28 | pub struct ZeroSize; + | ^^^^^^^^^^^^^^^^^^^^ error: `extern` block uses type `ZeroSizeWithPhantomData` which is not FFI-safe: composed only of PhantomData --> $DIR/lint-ctypes.rs:66:33 From 051ea5cc9bac00c7f588b4eae72b8a382d8ceb68 Mon Sep 17 00:00:00 2001 From: Robin Kruppe Date: Wed, 14 Feb 2018 23:11:29 +0100 Subject: [PATCH 7/7] [improper_ctypes] Don't suggest raw pointers when encountering trait objects It's unhelpful since raw pointers to trait objects are also FFI-unsafe and casting to a thin raw pointer loses the vtable. There are working solutions that _involve_ raw pointers but they're too complex to explain in one line and have serious trade offs. --- src/librustc_lint/types.rs | 2 +- src/test/ui/lint-ctypes.stderr | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/librustc_lint/types.rs b/src/librustc_lint/types.rs index 396d830d85b..ef9b3d38c63 100644 --- a/src/librustc_lint/types.rs +++ b/src/librustc_lint/types.rs @@ -577,7 +577,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { ty::TyDynamic(..) => FfiUnsafe { ty: ty, reason: "trait objects have no C equivalent", - help: Some("consider using a raw pointer instead"), + help: None, }, ty::TyStr => FfiUnsafe { diff --git a/src/test/ui/lint-ctypes.stderr b/src/test/ui/lint-ctypes.stderr index 2abf08d55f7..748c311055f 100644 --- a/src/test/ui/lint-ctypes.stderr +++ b/src/test/ui/lint-ctypes.stderr @@ -78,8 +78,6 @@ error: `extern` block uses type `std::clone::Clone` which is not FFI-safe: trait | 62 | pub fn trait_type(p: &Clone); //~ ERROR uses type `std::clone::Clone` | ^^^^^^ - | - = help: consider using a raw pointer instead error: `extern` block uses type `(i32, i32)` which is not FFI-safe: tuples have unspecified layout --> $DIR/lint-ctypes.rs:63:26