From 56e115a2627ba8bdd2e66c759457af96b2b0286a Mon Sep 17 00:00:00 2001 From: luxx4x Date: Fri, 19 Jun 2020 16:36:23 -0400 Subject: [PATCH 01/10] Allow dynamic linking for iOS/tvOS targets. --- src/librustc_target/spec/apple_sdk_base.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/librustc_target/spec/apple_sdk_base.rs b/src/librustc_target/spec/apple_sdk_base.rs index b07c2aef1ca..0d0a0da9d1c 100644 --- a/src/librustc_target/spec/apple_sdk_base.rs +++ b/src/librustc_target/spec/apple_sdk_base.rs @@ -141,7 +141,6 @@ pub fn opts(arch: Arch, os: AppleOS) -> Result { let pre_link_args = build_pre_link_args(arch, os)?; Ok(TargetOptions { cpu: target_cpu(arch), - dynamic_linking: false, executables: true, pre_link_args, link_env_remove: link_env_remove(arch), From 314e621198942a6dfd5db8beea27065f0ee69aa3 Mon Sep 17 00:00:00 2001 From: Ivan Tham Date: Mon, 22 Jun 2020 20:52:02 +0800 Subject: [PATCH 02/10] Liballoc minor hash import tweak --- src/liballoc/vec.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/liballoc/vec.rs b/src/liballoc/vec.rs index 95c3b3b1861..e9d2aa17162 100644 --- a/src/liballoc/vec.rs +++ b/src/liballoc/vec.rs @@ -62,7 +62,7 @@ use core::array::LengthAtMost32; use core::cmp::{self, Ordering}; use core::fmt; -use core::hash::{self, Hash}; +use core::hash::{Hash, Hasher}; use core::intrinsics::{arith_offset, assume}; use core::iter::{FromIterator, FusedIterator, TrustedLen}; use core::marker::PhantomData; @@ -1945,7 +1945,7 @@ fn clone_from(&mut self, other: &Vec) { #[stable(feature = "rust1", since = "1.0.0")] impl Hash for Vec { #[inline] - fn hash(&self, state: &mut H) { + fn hash(&self, state: &mut H) { Hash::hash(&**self, state) } } From 0a454e5398d017a0b442d56c75ac470e996ae52a Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Mon, 22 Jun 2020 23:37:05 +0100 Subject: [PATCH 03/10] Add UI test for issue 73592 --- .../issue-73592-borrow_mut-through-deref.rs | 58 +++++++++++++++++++ ...ssue-73592-borrow_mut-through-deref.stderr | 24 ++++++++ 2 files changed, 82 insertions(+) create mode 100644 src/test/ui/typeck/issue-73592-borrow_mut-through-deref.rs create mode 100644 src/test/ui/typeck/issue-73592-borrow_mut-through-deref.stderr diff --git a/src/test/ui/typeck/issue-73592-borrow_mut-through-deref.rs b/src/test/ui/typeck/issue-73592-borrow_mut-through-deref.rs new file mode 100644 index 00000000000..0cf77da5594 --- /dev/null +++ b/src/test/ui/typeck/issue-73592-borrow_mut-through-deref.rs @@ -0,0 +1,58 @@ +// check-pass +// +// rust-lang/rust#73592: borrow_mut through Deref should work. +// +// Before #72280, when we see something like `&mut *rcvr.method()`, we +// incorrectly requires `rcvr` to be type-checked as a mut place. While this +// requirement is usually correct for smart pointers, it is overly restrictive +// for types like `Mutex` or `RefCell` which can produce a guard that +// implements `DerefMut` from `&self`. +// +// Making it more confusing, because we use Deref as the fallback when DerefMut +// is implemented, we won't see an issue when the smart pointer does not +// implement `DerefMut`. It only causes an issue when `rcvr` is obtained via a +// type that implements both `Deref` or `DerefMut`. +// +// This bug is only discovered in #73592 after it is already fixed as a side-effect +// of a refactoring made in #72280. + +#![warn(unused_mut)] + +use std::pin::Pin; +use std::cell::RefCell; + +struct S(RefCell<()>); + +fn test_pin(s: Pin<&S>) { + // This works before #72280. + let _ = &mut *s.0.borrow_mut(); +} + +fn test_pin_mut(s: Pin<&mut S>) { + // This should compile but didn't before #72280. + let _ = &mut *s.0.borrow_mut(); +} + +fn test_vec(s: &Vec>) { + // This should compile but didn't before #72280. + let _ = &mut *s[0].borrow_mut(); +} + +fn test_mut_pin(mut s: Pin<&S>) { + //~^ WARN variable does not need to be mutable + let _ = &mut *s.0.borrow_mut(); +} + +fn test_mut_pin_mut(mut s: Pin<&mut S>) { + //~^ WARN variable does not need to be mutable + let _ = &mut *s.0.borrow_mut(); +} + +fn main() { + let mut s = S(RefCell::new(())); + test_pin(Pin::new(&s)); + test_pin_mut(Pin::new(&mut s)); + test_mut_pin(Pin::new(&s)); + test_mut_pin_mut(Pin::new(&mut s)); + test_vec(&vec![s.0]); +} diff --git a/src/test/ui/typeck/issue-73592-borrow_mut-through-deref.stderr b/src/test/ui/typeck/issue-73592-borrow_mut-through-deref.stderr new file mode 100644 index 00000000000..51303adc9e5 --- /dev/null +++ b/src/test/ui/typeck/issue-73592-borrow_mut-through-deref.stderr @@ -0,0 +1,24 @@ +warning: variable does not need to be mutable + --> $DIR/issue-73592-borrow_mut-through-deref.rs:41:17 + | +LL | fn test_mut_pin(mut s: Pin<&S>) { + | ----^ + | | + | help: remove this `mut` + | +note: the lint level is defined here + --> $DIR/issue-73592-borrow_mut-through-deref.rs:19:9 + | +LL | #![warn(unused_mut)] + | ^^^^^^^^^^ + +warning: variable does not need to be mutable + --> $DIR/issue-73592-borrow_mut-through-deref.rs:46:21 + | +LL | fn test_mut_pin_mut(mut s: Pin<&mut S>) { + | ----^ + | | + | help: remove this `mut` + +warning: 2 warnings emitted + From 14ea7a777f924995256a05da55133d265ed169be Mon Sep 17 00:00:00 2001 From: David Wood Date: Thu, 28 May 2020 15:57:09 +0100 Subject: [PATCH 04/10] lints: add `improper_ctypes_definitions` This commit adds a new lint - `improper_ctypes_definitions` - which functions identically to `improper_ctypes`, but on `extern "C" fn` definitions (as opposed to `improper_ctypes`'s `extern "C" {}` declarations). Signed-off-by: David Wood --- src/liballoc/boxed.rs | 2 + src/libpanic_abort/lib.rs | 1 + src/libpanic_unwind/lib.rs | 1 + src/librustc_lint/lib.rs | 3 +- src/librustc_lint/types.rs | 107 ++++++-- src/librustc_llvm/lib.rs | 1 + src/libstd/sys/sgx/abi/mod.rs | 1 + src/test/ui/abi/abi-sysv64-register-usage.rs | 1 + src/test/ui/align-with-extern-c-fn.rs | 1 + src/test/ui/issues/issue-16441.rs | 1 + src/test/ui/issues/issue-26997.rs | 1 + src/test/ui/issues/issue-28600.rs | 1 + src/test/ui/issues/issue-38763.rs | 1 + src/test/ui/issues/issue-51907.rs | 2 + src/test/ui/lint/lint-ctypes-fn.rs | 186 ++++++++++++++ src/test/ui/lint/lint-ctypes-fn.stderr | 247 +++++++++++++++++++ src/test/ui/mir/mir_cast_fn_ret.rs | 2 + src/test/ui/mir/mir_codegen_calls.rs | 1 + 18 files changed, 533 insertions(+), 27 deletions(-) create mode 100644 src/test/ui/lint/lint-ctypes-fn.rs create mode 100644 src/test/ui/lint/lint-ctypes-fn.stderr diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index d10cbf1afab..f1b560b9b96 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -92,11 +92,13 @@ //! pub struct Foo; //! //! #[no_mangle] +//! #[allow(improper_ctypes_definitions)] //! pub extern "C" fn foo_new() -> Box { //! Box::new(Foo) //! } //! //! #[no_mangle] +//! #[allow(improper_ctypes_definitions)] //! pub extern "C" fn foo_delete(_: Option>) {} //! ``` //! diff --git a/src/libpanic_abort/lib.rs b/src/libpanic_abort/lib.rs index fd3e11858ce..27056d5f934 100644 --- a/src/libpanic_abort/lib.rs +++ b/src/libpanic_abort/lib.rs @@ -21,6 +21,7 @@ use core::any::Any; #[rustc_std_internal_symbol] +#[cfg_attr(not(bootstrap), allow(improper_ctypes_definitions))] pub unsafe extern "C" fn __rust_panic_cleanup(_: *mut u8) -> *mut (dyn Any + Send + 'static) { unreachable!() } diff --git a/src/libpanic_unwind/lib.rs b/src/libpanic_unwind/lib.rs index f791fe82a27..f361354da2a 100644 --- a/src/libpanic_unwind/lib.rs +++ b/src/libpanic_unwind/lib.rs @@ -81,6 +81,7 @@ mod dwarf; #[rustc_std_internal_symbol] +#[cfg_attr(not(bootstrap), allow(improper_ctypes_definitions))] pub unsafe extern "C" fn __rust_panic_cleanup(payload: *mut u8) -> *mut (dyn Any + Send + 'static) { Box::into_raw(imp::cleanup(payload)) } diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index ca2ca3145ab..b39abe7b411 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -167,7 +167,8 @@ macro_rules! late_lint_mod_passes { $args, [ HardwiredLints: HardwiredLints, - ImproperCTypes: ImproperCTypes, + ImproperCTypesDeclarations: ImproperCTypesDeclarations, + ImproperCTypesDefinitions: ImproperCTypesDefinitions, VariantSizeDifferences: VariantSizeDifferences, BoxPointers: BoxPointers, PathStatements: PathStatements, diff --git a/src/librustc_lint/types.rs b/src/librustc_lint/types.rs index a19c9a35579..4c8b79308ed 100644 --- a/src/librustc_lint/types.rs +++ b/src/librustc_lint/types.rs @@ -498,10 +498,24 @@ fn is_comparison(binop: hir::BinOp) -> bool { "proper use of libc types in foreign modules" } -declare_lint_pass!(ImproperCTypes => [IMPROPER_CTYPES]); +declare_lint_pass!(ImproperCTypesDeclarations => [IMPROPER_CTYPES]); + +declare_lint! { + IMPROPER_CTYPES_DEFINITIONS, + Warn, + "proper use of libc types in foreign item definitions" +} + +declare_lint_pass!(ImproperCTypesDefinitions => [IMPROPER_CTYPES_DEFINITIONS]); + +enum ImproperCTypesMode { + Declarations, + Definitions, +} struct ImproperCTypesVisitor<'a, 'tcx> { cx: &'a LateContext<'a, 'tcx>, + mode: ImproperCTypesMode, } enum FfiResult<'tcx> { @@ -811,20 +825,16 @@ fn check_type_for_ffi(&self, cache: &mut FxHashSet>, ty: Ty<'tcx>) -> F ty::Array(inner_ty, _) => self.check_type_for_ffi(cache, inner_ty), ty::FnPtr(sig) => { - match sig.abi() { - Abi::Rust | Abi::RustIntrinsic | Abi::PlatformIntrinsic | Abi::RustCall => { - return FfiUnsafe { - ty, - reason: "this function pointer has Rust-specific calling convention" + if self.is_internal_abi(sig.abi()) { + return FfiUnsafe { + ty, + reason: "this function pointer has Rust-specific calling convention".into(), + help: Some( + "consider using an `extern fn(...) -> ...` \ + function pointer instead" .into(), - help: Some( - "consider using an `extern fn(...) -> ...` \ - function pointer instead" - .into(), - ), - }; - } - _ => {} + ), + }; } let sig = cx.erase_late_bound_regions(&sig); @@ -857,15 +867,17 @@ fn check_type_for_ffi(&self, cache: &mut FxHashSet>, ty: Ty<'tcx>) -> F FfiUnsafe { ty, reason: "opaque types have no C equivalent".into(), help: None } } - ty::Param(..) - | ty::Infer(..) + // `extern "C" fn` functions can have type parameters, which may or may not be FFI-safe, + // so they are currently ignored for the purposes of this lint. + ty::Param(..) | ty::Projection(..) => FfiSafe, + + ty::Infer(..) | ty::Bound(..) | ty::Error(_) | ty::Closure(..) | ty::Generator(..) | ty::GeneratorWitness(..) | ty::Placeholder(..) - | ty::Projection(..) | ty::FnDef(..) => bug!("unexpected type in foreign function: {:?}", ty), } } @@ -877,9 +889,20 @@ fn emit_ffi_unsafe_type_lint( note: &str, help: Option<&str>, ) { - self.cx.struct_span_lint(IMPROPER_CTYPES, sp, |lint| { - let mut diag = - lint.build(&format!("`extern` block uses type `{}`, which is not FFI-safe", ty)); + let lint = match self.mode { + ImproperCTypesMode::Declarations => IMPROPER_CTYPES, + ImproperCTypesMode::Definitions => IMPROPER_CTYPES_DEFINITIONS, + }; + + self.cx.struct_span_lint(lint, sp, |lint| { + let item_description = match self.mode { + ImproperCTypesMode::Declarations => "block", + ImproperCTypesMode::Definitions => "fn", + }; + let mut diag = lint.build(&format!( + "`extern` {} uses type `{}`, which is not FFI-safe", + item_description, ty + )); diag.span_label(sp, "not FFI-safe"); if let Some(help) = help { diag.help(help); @@ -947,7 +970,7 @@ fn check_type_for_ffi_and_report_errors( // it is only OK to use this function because extern fns cannot have // any generic types right now: - let ty = self.cx.tcx.normalize_erasing_regions(ParamEnv::reveal_all(), ty); + let ty = self.cx.tcx.normalize_erasing_regions(self.cx.param_env, ty); // C doesn't really support passing arrays by value - the only way to pass an array by value // is through a struct. So, first test that the top level isn't an array, and then @@ -997,15 +1020,22 @@ fn check_foreign_static(&mut self, id: hir::HirId, span: Span) { let ty = self.cx.tcx.type_of(def_id); self.check_type_for_ffi_and_report_errors(span, ty, true, false); } + + fn is_internal_abi(&self, abi: Abi) -> bool { + if let Abi::Rust | Abi::RustCall | Abi::RustIntrinsic | Abi::PlatformIntrinsic = abi { + true + } else { + false + } + } } -impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImproperCTypes { +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImproperCTypesDeclarations { fn check_foreign_item(&mut self, cx: &LateContext<'_, '_>, it: &hir::ForeignItem<'_>) { - let mut vis = ImproperCTypesVisitor { cx }; + let mut vis = ImproperCTypesVisitor { cx, mode: ImproperCTypesMode::Declarations }; let abi = cx.tcx.hir().get_foreign_abi(it.hir_id); - if let Abi::Rust | Abi::RustCall | Abi::RustIntrinsic | Abi::PlatformIntrinsic = abi { - // Don't worry about types in internal ABIs. - } else { + + if !vis.is_internal_abi(abi) { match it.kind { hir::ForeignItemKind::Fn(ref decl, _, _) => { vis.check_foreign_fn(it.hir_id, decl); @@ -1019,6 +1049,31 @@ fn check_foreign_item(&mut self, cx: &LateContext<'_, '_>, it: &hir::ForeignItem } } +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImproperCTypesDefinitions { + fn check_fn( + &mut self, + cx: &LateContext<'a, 'tcx>, + kind: hir::intravisit::FnKind<'tcx>, + decl: &'tcx hir::FnDecl<'_>, + _: &'tcx hir::Body<'_>, + _: Span, + hir_id: hir::HirId, + ) { + use hir::intravisit::FnKind; + + let abi = match kind { + FnKind::ItemFn(_, _, header, ..) => header.abi, + FnKind::Method(_, sig, ..) => sig.header.abi, + _ => return, + }; + + let mut vis = ImproperCTypesVisitor { cx, mode: ImproperCTypesMode::Definitions }; + if !vis.is_internal_abi(abi) { + vis.check_foreign_fn(hir_id, decl); + } + } +} + declare_lint_pass!(VariantSizeDifferences => [VARIANT_SIZE_DIFFERENCES]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for VariantSizeDifferences { diff --git a/src/librustc_llvm/lib.rs b/src/librustc_llvm/lib.rs index 79e1a6cc5dc..f54ed9b9202 100644 --- a/src/librustc_llvm/lib.rs +++ b/src/librustc_llvm/lib.rs @@ -15,6 +15,7 @@ pub struct RustString { /// Appending to a Rust string -- used by RawRustStringOstream. #[no_mangle] +#[cfg_attr(not(bootstrap), allow(improper_ctypes_definitions))] pub unsafe extern "C" fn LLVMRustStringWriteImpl( sr: &RustString, ptr: *const c_char, diff --git a/src/libstd/sys/sgx/abi/mod.rs b/src/libstd/sys/sgx/abi/mod.rs index 87e7a5da2b7..5ef26d4cc4d 100644 --- a/src/libstd/sys/sgx/abi/mod.rs +++ b/src/libstd/sys/sgx/abi/mod.rs @@ -56,6 +56,7 @@ // able to specify this #[cfg(not(test))] #[no_mangle] +#[allow(improper_ctypes_definitions)] extern "C" fn entry(p1: u64, p2: u64, p3: u64, secondary: bool, p4: u64, p5: u64) -> (u64, u64) { // FIXME: how to support TLS in library mode? let tls = Box::new(tls::Tls::new()); diff --git a/src/test/ui/abi/abi-sysv64-register-usage.rs b/src/test/ui/abi/abi-sysv64-register-usage.rs index fcdff59ffa9..32864dba458 100644 --- a/src/test/ui/abi/abi-sysv64-register-usage.rs +++ b/src/test/ui/abi/abi-sysv64-register-usage.rs @@ -38,6 +38,7 @@ pub extern "sysv64" fn all_the_registers(rdi: i64, rsi: i64, rdx: i64, #[cfg(target_arch = "x86_64")] #[inline(never)] +#[allow(improper_ctypes_definitions)] pub extern "sysv64" fn large_struct_by_val(mut foo: LargeStruct) -> LargeStruct { foo.0 *= 1; foo.1 *= 2; diff --git a/src/test/ui/align-with-extern-c-fn.rs b/src/test/ui/align-with-extern-c-fn.rs index 09abe4fbf7e..f77f40998de 100644 --- a/src/test/ui/align-with-extern-c-fn.rs +++ b/src/test/ui/align-with-extern-c-fn.rs @@ -10,6 +10,7 @@ #[repr(align(16))] pub struct A(i64); +#[allow(improper_ctypes_definitions)] pub extern "C" fn foo(x: A) {} fn main() { diff --git a/src/test/ui/issues/issue-16441.rs b/src/test/ui/issues/issue-16441.rs index bae3813f9da..bafa204e06b 100644 --- a/src/test/ui/issues/issue-16441.rs +++ b/src/test/ui/issues/issue-16441.rs @@ -5,6 +5,7 @@ struct Empty; // This used to cause an ICE +#[allow(improper_ctypes_definitions)] extern "C" fn ice(_a: Empty) {} fn main() { diff --git a/src/test/ui/issues/issue-26997.rs b/src/test/ui/issues/issue-26997.rs index f6d349a38f5..fcabd1d8455 100644 --- a/src/test/ui/issues/issue-26997.rs +++ b/src/test/ui/issues/issue-26997.rs @@ -6,6 +6,7 @@ pub struct Foo { } impl Foo { + #[allow(improper_ctypes_definitions)] pub extern fn foo_new() -> Foo { Foo { x: 21, y: 33 } } diff --git a/src/test/ui/issues/issue-28600.rs b/src/test/ui/issues/issue-28600.rs index 3bbe4ae29bd..297519b9a79 100644 --- a/src/test/ui/issues/issue-28600.rs +++ b/src/test/ui/issues/issue-28600.rs @@ -6,6 +6,7 @@ impl Test { #[allow(dead_code)] #[allow(unused_variables)] + #[allow(improper_ctypes_definitions)] pub extern fn test(val: &str) { } diff --git a/src/test/ui/issues/issue-38763.rs b/src/test/ui/issues/issue-38763.rs index 6e6de09225f..a966cf217e1 100644 --- a/src/test/ui/issues/issue-38763.rs +++ b/src/test/ui/issues/issue-38763.rs @@ -5,6 +5,7 @@ pub struct Foo(i128); #[no_mangle] +#[allow(improper_ctypes_definitions)] pub extern "C" fn foo(x: Foo) -> Foo { x } fn main() { diff --git a/src/test/ui/issues/issue-51907.rs b/src/test/ui/issues/issue-51907.rs index 3691fe19117..52d26d0954a 100644 --- a/src/test/ui/issues/issue-51907.rs +++ b/src/test/ui/issues/issue-51907.rs @@ -6,7 +6,9 @@ trait Foo { struct Bar; impl Foo for Bar { + #[allow(improper_ctypes_definitions)] extern fn borrow(&self) {} + #[allow(improper_ctypes_definitions)] extern fn take(self: Box) {} } diff --git a/src/test/ui/lint/lint-ctypes-fn.rs b/src/test/ui/lint/lint-ctypes-fn.rs new file mode 100644 index 00000000000..c9c95e71484 --- /dev/null +++ b/src/test/ui/lint/lint-ctypes-fn.rs @@ -0,0 +1,186 @@ +#![feature(rustc_private)] + +#![allow(private_in_public)] +#![deny(improper_ctypes_definitions)] + +extern crate libc; + +use std::default::Default; +use std::marker::PhantomData; + +trait Mirror { type It: ?Sized; } + +impl Mirror for T { type It = Self; } + +#[repr(C)] +pub struct StructWithProjection(*mut ::It); + +#[repr(C)] +pub struct StructWithProjectionAndLifetime<'a>( + &'a mut as Mirror>::It +); + +pub type I32Pair = (i32, i32); + +#[repr(C)] +pub struct ZeroSize; + +pub type RustFn = fn(); + +pub type RustBadRet = extern fn() -> Box; + +pub type CVoidRet = (); + +pub struct Foo; + +#[repr(transparent)] +pub struct TransparentI128(i128); + +#[repr(transparent)] +pub struct TransparentStr(&'static str); + +#[repr(transparent)] +pub struct TransparentBadFn(RustBadRet); + +#[repr(transparent)] +pub struct TransparentInt(u32); + +#[repr(transparent)] +pub struct TransparentRef<'a>(&'a TransparentInt); + +#[repr(transparent)] +pub struct TransparentLifetime<'a>(*const u8, PhantomData<&'a ()>); + +#[repr(transparent)] +pub struct TransparentUnit(f32, PhantomData); + +#[repr(transparent)] +pub struct TransparentCustomZst(i32, ZeroSize); + +#[repr(C)] +pub struct ZeroSizeWithPhantomData(PhantomData); + +pub extern "C" fn ptr_type1(size: *const Foo) { } +//~^ ERROR: uses type `Foo` + +pub extern "C" fn ptr_type2(size: *const Foo) { } +//~^ ERROR: uses type `Foo` + +pub extern "C" fn slice_type(p: &[u32]) { } +//~^ ERROR: uses type `[u32]` + +pub extern "C" fn str_type(p: &str) { } +//~^ ERROR: uses type `str` + +pub extern "C" fn box_type(p: Box) { } +//~^ ERROR uses type `std::boxed::Box` + +pub extern "C" fn char_type(p: char) { } +//~^ ERROR uses type `char` + +pub extern "C" fn i128_type(p: i128) { } +//~^ ERROR uses type `i128` + +pub extern "C" fn u128_type(p: u128) { } +//~^ ERROR uses type `u128` + +pub extern "C" fn tuple_type(p: (i32, i32)) { } +//~^ ERROR uses type `(i32, i32)` + +pub extern "C" fn tuple_type2(p: I32Pair) { } +//~^ ERROR uses type `(i32, i32)` + +pub extern "C" fn zero_size(p: ZeroSize) { } +//~^ ERROR uses type `ZeroSize` + +pub extern "C" fn zero_size_phantom(p: ZeroSizeWithPhantomData) { } +//~^ ERROR uses type `ZeroSizeWithPhantomData` + +pub extern "C" fn zero_size_phantom_toplevel() -> PhantomData { +//~^ ERROR uses type `std::marker::PhantomData` + Default::default() +} + +pub extern "C" fn fn_type(p: RustFn) { } +//~^ ERROR uses type `fn()` + +pub extern "C" fn fn_type2(p: fn()) { } +//~^ ERROR uses type `fn()` + +pub extern "C" fn fn_contained(p: RustBadRet) { } +//~^ ERROR: uses type `std::boxed::Box` + +pub extern "C" fn transparent_i128(p: TransparentI128) { } +//~^ ERROR: uses type `i128` + +pub extern "C" fn transparent_str(p: TransparentStr) { } +//~^ ERROR: uses type `str` + +pub extern "C" fn transparent_fn(p: TransparentBadFn) { } +//~^ ERROR: uses type `std::boxed::Box` + +pub extern "C" fn good3(fptr: Option) { } + +pub extern "C" fn good4(aptr: &[u8; 4 as usize]) { } + +pub extern "C" fn good5(s: StructWithProjection) { } + +pub extern "C" fn good6(s: StructWithProjectionAndLifetime) { } + +pub extern "C" fn good7(fptr: extern fn() -> ()) { } + +pub extern "C" fn good8(fptr: extern fn() -> !) { } + +pub extern "C" fn good9() -> () { } + +pub extern "C" fn good10() -> CVoidRet { } + +pub extern "C" fn good11(size: isize) { } + +pub extern "C" fn good12(size: usize) { } + +pub extern "C" fn good13(n: TransparentInt) { } + +pub extern "C" fn good14(p: TransparentRef) { } + +pub extern "C" fn good15(p: TransparentLifetime) { } + +pub extern "C" fn good16(p: TransparentUnit) { } + +pub extern "C" fn good17(p: TransparentCustomZst) { } + +#[allow(improper_ctypes_definitions)] +pub extern "C" fn good18(_: &String) { } + +#[cfg(not(target_arch = "wasm32"))] +pub extern "C" fn good1(size: *const libc::c_int) { } + +#[cfg(not(target_arch = "wasm32"))] +pub extern "C" fn good2(size: *const libc::c_uint) { } + +pub extern "C" fn unused_generic1(size: *const Foo) { } +//~^ ERROR: uses type `Foo` + +pub extern "C" fn unused_generic2() -> PhantomData { +//~^ ERROR uses type `std::marker::PhantomData` + Default::default() +} + +pub extern "C" fn used_generic1(x: T) { } + +pub extern "C" fn used_generic2(x: T, size: *const Foo) { } +//~^ ERROR: uses type `Foo` + +pub extern "C" fn used_generic3() -> T { + Default::default() +} + +pub extern "C" fn used_generic4(x: Vec) { } +//~^ ERROR: uses type `std::vec::Vec` + +pub extern "C" fn used_generic5() -> Vec { +//~^ ERROR: uses type `std::vec::Vec` + Default::default() +} + +fn main() {} diff --git a/src/test/ui/lint/lint-ctypes-fn.stderr b/src/test/ui/lint/lint-ctypes-fn.stderr new file mode 100644 index 00000000000..3ba16b622cb --- /dev/null +++ b/src/test/ui/lint/lint-ctypes-fn.stderr @@ -0,0 +1,247 @@ +error: `extern` fn uses type `Foo`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:63:35 + | +LL | pub extern "C" fn ptr_type1(size: *const Foo) { } + | ^^^^^^^^^^ not FFI-safe + | +note: the lint level is defined here + --> $DIR/lint-ctypes-fn.rs:4:9 + | +LL | #![deny(improper_ctypes_definitions)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = 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-fn.rs:34:1 + | +LL | pub struct Foo; + | ^^^^^^^^^^^^^^^ + +error: `extern` fn uses type `Foo`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:66:35 + | +LL | pub extern "C" 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-fn.rs:34:1 + | +LL | pub struct Foo; + | ^^^^^^^^^^^^^^^ + +error: `extern` fn uses type `[u32]`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:69:33 + | +LL | pub extern "C" fn slice_type(p: &[u32]) { } + | ^^^^^^ not FFI-safe + | + = help: consider using a raw pointer instead + = note: slices have no C equivalent + +error: `extern` fn uses type `str`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:72:31 + | +LL | pub extern "C" fn str_type(p: &str) { } + | ^^^^ not FFI-safe + | + = help: consider using `*const u8` and a length instead + = note: string slices have no C equivalent + +error: `extern` fn uses type `std::boxed::Box`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:75:31 + | +LL | pub extern "C" fn box_type(p: Box) { } + | ^^^^^^^^ not FFI-safe + | + = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct + = note: this struct has unspecified layout + +error: `extern` fn uses type `char`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:78:32 + | +LL | pub extern "C" fn char_type(p: char) { } + | ^^^^ not FFI-safe + | + = help: consider using `u32` or `libc::wchar_t` instead + = note: the `char` type has no C equivalent + +error: `extern` fn uses type `i128`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:81:32 + | +LL | pub extern "C" fn i128_type(p: i128) { } + | ^^^^ not FFI-safe + | + = note: 128-bit integers don't currently have a known stable ABI + +error: `extern` fn uses type `u128`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:84:32 + | +LL | pub extern "C" fn u128_type(p: u128) { } + | ^^^^ not FFI-safe + | + = note: 128-bit integers don't currently have a known stable ABI + +error: `extern` fn uses type `(i32, i32)`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:87:33 + | +LL | pub extern "C" fn tuple_type(p: (i32, i32)) { } + | ^^^^^^^^^^ not FFI-safe + | + = help: consider using a struct instead + = note: tuples have unspecified layout + +error: `extern` fn uses type `(i32, i32)`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:90:34 + | +LL | pub extern "C" fn tuple_type2(p: I32Pair) { } + | ^^^^^^^ not FFI-safe + | + = help: consider using a struct instead + = note: tuples have unspecified layout + +error: `extern` fn uses type `ZeroSize`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:93:32 + | +LL | pub extern "C" 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-fn.rs:26:1 + | +LL | pub struct ZeroSize; + | ^^^^^^^^^^^^^^^^^^^^ + +error: `extern` fn uses type `ZeroSizeWithPhantomData`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:96:40 + | +LL | pub extern "C" fn zero_size_phantom(p: ZeroSizeWithPhantomData) { } + | ^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe + | + = note: composed only of `PhantomData` +note: the type is defined here + --> $DIR/lint-ctypes-fn.rs:61:1 + | +LL | pub struct ZeroSizeWithPhantomData(PhantomData); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: `extern` fn uses type `std::marker::PhantomData`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:99:51 + | +LL | pub extern "C" fn zero_size_phantom_toplevel() -> PhantomData { + | ^^^^^^^^^^^^^^^^^ not FFI-safe + | + = note: composed only of `PhantomData` + +error: `extern` fn uses type `fn()`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:104:30 + | +LL | pub extern "C" fn fn_type(p: RustFn) { } + | ^^^^^^ not FFI-safe + | + = help: consider using an `extern fn(...) -> ...` function pointer instead + = note: this function pointer has Rust-specific calling convention + +error: `extern` fn uses type `fn()`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:107:31 + | +LL | pub extern "C" fn fn_type2(p: fn()) { } + | ^^^^ not FFI-safe + | + = help: consider using an `extern fn(...) -> ...` function pointer instead + = note: this function pointer has Rust-specific calling convention + +error: `extern` fn uses type `std::boxed::Box`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:110:35 + | +LL | pub extern "C" fn fn_contained(p: RustBadRet) { } + | ^^^^^^^^^^ not FFI-safe + | + = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct + = note: this struct has unspecified layout + +error: `extern` fn uses type `i128`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:113:39 + | +LL | pub extern "C" fn transparent_i128(p: TransparentI128) { } + | ^^^^^^^^^^^^^^^ not FFI-safe + | + = note: 128-bit integers don't currently have a known stable ABI + +error: `extern` fn uses type `str`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:116:38 + | +LL | pub extern "C" fn transparent_str(p: TransparentStr) { } + | ^^^^^^^^^^^^^^ not FFI-safe + | + = help: consider using `*const u8` and a length instead + = note: string slices have no C equivalent + +error: `extern` fn uses type `std::boxed::Box`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:119:37 + | +LL | pub extern "C" fn transparent_fn(p: TransparentBadFn) { } + | ^^^^^^^^^^^^^^^^ not FFI-safe + | + = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct + = note: this struct has unspecified layout + +error: `extern` fn uses type `Foo`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:161:44 + | +LL | pub extern "C" fn unused_generic1(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-fn.rs:34:1 + | +LL | pub struct Foo; + | ^^^^^^^^^^^^^^^ + +error: `extern` fn uses type `std::marker::PhantomData`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:164:43 + | +LL | pub extern "C" fn unused_generic2() -> PhantomData { + | ^^^^^^^^^^^^^^^^^ not FFI-safe + | + = note: composed only of `PhantomData` + +error: `extern` fn uses type `Foo`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:171:48 + | +LL | pub extern "C" fn used_generic2(x: T, 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-fn.rs:34:1 + | +LL | pub struct Foo; + | ^^^^^^^^^^^^^^^ + +error: `extern` fn uses type `std::vec::Vec`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:178:39 + | +LL | pub extern "C" fn used_generic4(x: Vec) { } + | ^^^^^^ not FFI-safe + | + = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct + = note: this struct has unspecified layout + +error: `extern` fn uses type `std::vec::Vec`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:181:41 + | +LL | pub extern "C" fn used_generic5() -> Vec { + | ^^^^^^ not FFI-safe + | + = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct + = note: this struct has unspecified layout + +error: aborting due to 24 previous errors + diff --git a/src/test/ui/mir/mir_cast_fn_ret.rs b/src/test/ui/mir/mir_cast_fn_ret.rs index 69fd64c1c09..4574dbd8529 100644 --- a/src/test/ui/mir/mir_cast_fn_ret.rs +++ b/src/test/ui/mir/mir_cast_fn_ret.rs @@ -1,8 +1,10 @@ // run-pass +#[allow(improper_ctypes_definitions)] pub extern "C" fn tuple2() -> (u16, u8) { (1, 2) } +#[allow(improper_ctypes_definitions)] pub extern "C" fn tuple3() -> (u8, u8, u8) { (1, 2, 3) } diff --git a/src/test/ui/mir/mir_codegen_calls.rs b/src/test/ui/mir/mir_codegen_calls.rs index fc0db03e3a9..d93a25c8ef4 100644 --- a/src/test/ui/mir/mir_codegen_calls.rs +++ b/src/test/ui/mir/mir_codegen_calls.rs @@ -74,6 +74,7 @@ fn test8() -> isize { Two::two() } +#[allow(improper_ctypes_definitions)] extern fn simple_extern(x: u32, y: (u32, u32)) -> u32 { x + y.0 * y.1 } From 5c8634805cbaed6010e15a15f15e840bd019079a Mon Sep 17 00:00:00 2001 From: David Wood Date: Thu, 28 May 2020 17:11:39 +0100 Subject: [PATCH 05/10] improper_ctypes: allow pointers to sized types This commit changes the improper ctypes lint (when operating on definitions) to consider raw pointers or references to sized types as FFI-safe. Signed-off-by: David Wood --- src/librustc_lint/types.rs | 11 ++- src/test/ui/lint/lint-ctypes-fn.rs | 4 - src/test/ui/lint/lint-ctypes-fn.stderr | 104 ++++++------------------- 3 files changed, 34 insertions(+), 85 deletions(-) diff --git a/src/librustc_lint/types.rs b/src/librustc_lint/types.rs index 4c8b79308ed..cce6a40c23a 100644 --- a/src/librustc_lint/types.rs +++ b/src/librustc_lint/types.rs @@ -14,7 +14,7 @@ use rustc_middle::ty::{self, AdtKind, ParamEnv, Ty, TyCtxt, TypeFoldable}; use rustc_span::source_map; use rustc_span::symbol::sym; -use rustc_span::Span; +use rustc_span::{Span, DUMMY_SP}; use rustc_target::abi::{Integer, LayoutOf, TagEncoding, VariantIdx, Variants}; use rustc_target::spec::abi::Abi; @@ -818,6 +818,15 @@ fn check_type_for_ffi(&self, cache: &mut FxHashSet>, ty: Ty<'tcx>) -> F help: Some("consider using a struct instead".into()), }, + ty::RawPtr(ty::TypeAndMut { ty, .. }) | ty::Ref(_, ty, _) + if { + matches!(self.mode, ImproperCTypesMode::Definitions) + && ty.is_sized(self.cx.tcx.at(DUMMY_SP), self.cx.param_env) + } => + { + FfiSafe + } + ty::RawPtr(ty::TypeAndMut { ty, .. }) | ty::Ref(_, ty, _) => { self.check_type_for_ffi(cache, ty) } diff --git a/src/test/ui/lint/lint-ctypes-fn.rs b/src/test/ui/lint/lint-ctypes-fn.rs index c9c95e71484..67dd7abcf79 100644 --- a/src/test/ui/lint/lint-ctypes-fn.rs +++ b/src/test/ui/lint/lint-ctypes-fn.rs @@ -61,10 +61,8 @@ pub struct StructWithProjectionAndLifetime<'a>( pub struct ZeroSizeWithPhantomData(PhantomData); pub extern "C" fn ptr_type1(size: *const Foo) { } -//~^ ERROR: uses type `Foo` pub extern "C" fn ptr_type2(size: *const Foo) { } -//~^ ERROR: uses type `Foo` pub extern "C" fn slice_type(p: &[u32]) { } //~^ ERROR: uses type `[u32]` @@ -159,7 +157,6 @@ pub extern "C" fn good1(size: *const libc::c_int) { } pub extern "C" fn good2(size: *const libc::c_uint) { } pub extern "C" fn unused_generic1(size: *const Foo) { } -//~^ ERROR: uses type `Foo` pub extern "C" fn unused_generic2() -> PhantomData { //~^ ERROR uses type `std::marker::PhantomData` @@ -169,7 +166,6 @@ pub extern "C" fn unused_generic2() -> PhantomData { pub extern "C" fn used_generic1(x: T) { } pub extern "C" fn used_generic2(x: T, size: *const Foo) { } -//~^ ERROR: uses type `Foo` pub extern "C" fn used_generic3() -> T { Default::default() diff --git a/src/test/ui/lint/lint-ctypes-fn.stderr b/src/test/ui/lint/lint-ctypes-fn.stderr index 3ba16b622cb..66cf1953278 100644 --- a/src/test/ui/lint/lint-ctypes-fn.stderr +++ b/src/test/ui/lint/lint-ctypes-fn.stderr @@ -1,47 +1,19 @@ -error: `extern` fn uses type `Foo`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:63:35 +error: `extern` fn uses type `[u32]`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:67:33 | -LL | pub extern "C" fn ptr_type1(size: *const Foo) { } - | ^^^^^^^^^^ not FFI-safe +LL | pub extern "C" fn slice_type(p: &[u32]) { } + | ^^^^^^ not FFI-safe | note: the lint level is defined here --> $DIR/lint-ctypes-fn.rs:4:9 | LL | #![deny(improper_ctypes_definitions)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = 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-fn.rs:34:1 - | -LL | pub struct Foo; - | ^^^^^^^^^^^^^^^ - -error: `extern` fn uses type `Foo`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:66:35 - | -LL | pub extern "C" 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-fn.rs:34:1 - | -LL | pub struct Foo; - | ^^^^^^^^^^^^^^^ - -error: `extern` fn uses type `[u32]`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:69:33 - | -LL | pub extern "C" fn slice_type(p: &[u32]) { } - | ^^^^^^ not FFI-safe - | = help: consider using a raw pointer instead = note: slices have no C equivalent error: `extern` fn uses type `str`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:72:31 + --> $DIR/lint-ctypes-fn.rs:70:31 | LL | pub extern "C" fn str_type(p: &str) { } | ^^^^ not FFI-safe @@ -50,7 +22,7 @@ LL | pub extern "C" fn str_type(p: &str) { } = note: string slices have no C equivalent error: `extern` fn uses type `std::boxed::Box`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:75:31 + --> $DIR/lint-ctypes-fn.rs:73:31 | LL | pub extern "C" fn box_type(p: Box) { } | ^^^^^^^^ not FFI-safe @@ -59,7 +31,7 @@ LL | pub extern "C" fn box_type(p: Box) { } = note: this struct has unspecified layout error: `extern` fn uses type `char`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:78:32 + --> $DIR/lint-ctypes-fn.rs:76:32 | LL | pub extern "C" fn char_type(p: char) { } | ^^^^ not FFI-safe @@ -68,7 +40,7 @@ LL | pub extern "C" fn char_type(p: char) { } = note: the `char` type has no C equivalent error: `extern` fn uses type `i128`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:81:32 + --> $DIR/lint-ctypes-fn.rs:79:32 | LL | pub extern "C" fn i128_type(p: i128) { } | ^^^^ not FFI-safe @@ -76,7 +48,7 @@ LL | pub extern "C" fn i128_type(p: i128) { } = note: 128-bit integers don't currently have a known stable ABI error: `extern` fn uses type `u128`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:84:32 + --> $DIR/lint-ctypes-fn.rs:82:32 | LL | pub extern "C" fn u128_type(p: u128) { } | ^^^^ not FFI-safe @@ -84,7 +56,7 @@ LL | pub extern "C" fn u128_type(p: u128) { } = note: 128-bit integers don't currently have a known stable ABI error: `extern` fn uses type `(i32, i32)`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:87:33 + --> $DIR/lint-ctypes-fn.rs:85:33 | LL | pub extern "C" fn tuple_type(p: (i32, i32)) { } | ^^^^^^^^^^ not FFI-safe @@ -93,7 +65,7 @@ LL | pub extern "C" fn tuple_type(p: (i32, i32)) { } = note: tuples have unspecified layout error: `extern` fn uses type `(i32, i32)`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:90:34 + --> $DIR/lint-ctypes-fn.rs:88:34 | LL | pub extern "C" fn tuple_type2(p: I32Pair) { } | ^^^^^^^ not FFI-safe @@ -102,7 +74,7 @@ LL | pub extern "C" fn tuple_type2(p: I32Pair) { } = note: tuples have unspecified layout error: `extern` fn uses type `ZeroSize`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:93:32 + --> $DIR/lint-ctypes-fn.rs:91:32 | LL | pub extern "C" fn zero_size(p: ZeroSize) { } | ^^^^^^^^ not FFI-safe @@ -116,7 +88,7 @@ LL | pub struct ZeroSize; | ^^^^^^^^^^^^^^^^^^^^ error: `extern` fn uses type `ZeroSizeWithPhantomData`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:96:40 + --> $DIR/lint-ctypes-fn.rs:94:40 | LL | pub extern "C" fn zero_size_phantom(p: ZeroSizeWithPhantomData) { } | ^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -129,7 +101,7 @@ LL | pub struct ZeroSizeWithPhantomData(PhantomData); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: `extern` fn uses type `std::marker::PhantomData`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:99:51 + --> $DIR/lint-ctypes-fn.rs:97:51 | LL | pub extern "C" fn zero_size_phantom_toplevel() -> PhantomData { | ^^^^^^^^^^^^^^^^^ not FFI-safe @@ -137,7 +109,7 @@ LL | pub extern "C" fn zero_size_phantom_toplevel() -> PhantomData { = note: composed only of `PhantomData` error: `extern` fn uses type `fn()`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:104:30 + --> $DIR/lint-ctypes-fn.rs:102:30 | LL | pub extern "C" fn fn_type(p: RustFn) { } | ^^^^^^ not FFI-safe @@ -146,7 +118,7 @@ LL | pub extern "C" fn fn_type(p: RustFn) { } = note: this function pointer has Rust-specific calling convention error: `extern` fn uses type `fn()`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:107:31 + --> $DIR/lint-ctypes-fn.rs:105:31 | LL | pub extern "C" fn fn_type2(p: fn()) { } | ^^^^ not FFI-safe @@ -155,7 +127,7 @@ LL | pub extern "C" fn fn_type2(p: fn()) { } = note: this function pointer has Rust-specific calling convention error: `extern` fn uses type `std::boxed::Box`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:110:35 + --> $DIR/lint-ctypes-fn.rs:108:35 | LL | pub extern "C" fn fn_contained(p: RustBadRet) { } | ^^^^^^^^^^ not FFI-safe @@ -164,7 +136,7 @@ LL | pub extern "C" fn fn_contained(p: RustBadRet) { } = note: this struct has unspecified layout error: `extern` fn uses type `i128`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:113:39 + --> $DIR/lint-ctypes-fn.rs:111:39 | LL | pub extern "C" fn transparent_i128(p: TransparentI128) { } | ^^^^^^^^^^^^^^^ not FFI-safe @@ -172,7 +144,7 @@ LL | pub extern "C" fn transparent_i128(p: TransparentI128) { } = note: 128-bit integers don't currently have a known stable ABI error: `extern` fn uses type `str`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:116:38 + --> $DIR/lint-ctypes-fn.rs:114:38 | LL | pub extern "C" fn transparent_str(p: TransparentStr) { } | ^^^^^^^^^^^^^^ not FFI-safe @@ -181,7 +153,7 @@ LL | pub extern "C" fn transparent_str(p: TransparentStr) { } = note: string slices have no C equivalent error: `extern` fn uses type `std::boxed::Box`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:119:37 + --> $DIR/lint-ctypes-fn.rs:117:37 | LL | pub extern "C" fn transparent_fn(p: TransparentBadFn) { } | ^^^^^^^^^^^^^^^^ not FFI-safe @@ -189,44 +161,16 @@ LL | pub extern "C" fn transparent_fn(p: TransparentBadFn) { } = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct = note: this struct has unspecified layout -error: `extern` fn uses type `Foo`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:161:44 - | -LL | pub extern "C" fn unused_generic1(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-fn.rs:34:1 - | -LL | pub struct Foo; - | ^^^^^^^^^^^^^^^ - error: `extern` fn uses type `std::marker::PhantomData`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:164:43 + --> $DIR/lint-ctypes-fn.rs:161:43 | LL | pub extern "C" fn unused_generic2() -> PhantomData { | ^^^^^^^^^^^^^^^^^ not FFI-safe | = note: composed only of `PhantomData` -error: `extern` fn uses type `Foo`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:171:48 - | -LL | pub extern "C" fn used_generic2(x: T, 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-fn.rs:34:1 - | -LL | pub struct Foo; - | ^^^^^^^^^^^^^^^ - error: `extern` fn uses type `std::vec::Vec`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:178:39 + --> $DIR/lint-ctypes-fn.rs:174:39 | LL | pub extern "C" fn used_generic4(x: Vec) { } | ^^^^^^ not FFI-safe @@ -235,7 +179,7 @@ LL | pub extern "C" fn used_generic4(x: Vec) { } = note: this struct has unspecified layout error: `extern` fn uses type `std::vec::Vec`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:181:41 + --> $DIR/lint-ctypes-fn.rs:177:41 | LL | pub extern "C" fn used_generic5() -> Vec { | ^^^^^^ not FFI-safe @@ -243,5 +187,5 @@ LL | pub extern "C" fn used_generic5() -> Vec { = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct = note: this struct has unspecified layout -error: aborting due to 24 previous errors +error: aborting due to 20 previous errors From 4504648090a9b8cae909424826dc4190f23df044 Mon Sep 17 00:00:00 2001 From: David Wood Date: Sun, 21 Jun 2020 20:30:41 +0100 Subject: [PATCH 06/10] improper_ctypes: only allow params in defns mode This commit adjusts the behaviour introduced in a previous commit so that generic parameters and projections are only allowed in the definitions mode - and are otherwise a bug. Generic parameters in declarations are prohibited earlier in the compiler, so if that branch were reached, it would be a bug. Signed-off-by: David Wood --- src/librustc_lint/types.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/librustc_lint/types.rs b/src/librustc_lint/types.rs index cce6a40c23a..cfafb86fbed 100644 --- a/src/librustc_lint/types.rs +++ b/src/librustc_lint/types.rs @@ -878,9 +878,15 @@ fn check_type_for_ffi(&self, cache: &mut FxHashSet>, ty: Ty<'tcx>) -> F // `extern "C" fn` functions can have type parameters, which may or may not be FFI-safe, // so they are currently ignored for the purposes of this lint. - ty::Param(..) | ty::Projection(..) => FfiSafe, + ty::Param(..) | ty::Projection(..) + if matches!(self.mode, ImproperCTypesMode::Definitions) => + { + FfiSafe + } - ty::Infer(..) + ty::Param(..) + | ty::Projection(..) + | ty::Infer(..) | ty::Bound(..) | ty::Error(_) | ty::Closure(..) From 03661630c92365572bc5bbe51677f1190bab2131 Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Wed, 24 Jun 2020 13:52:57 +0200 Subject: [PATCH 07/10] Document the self keyword --- src/libstd/keyword_docs.rs | 88 +++++++++++++++++++++++++++++++++++++- 1 file changed, 86 insertions(+), 2 deletions(-) diff --git a/src/libstd/keyword_docs.rs b/src/libstd/keyword_docs.rs index a4996d9eee8..6d98c4d01c0 100644 --- a/src/libstd/keyword_docs.rs +++ b/src/libstd/keyword_docs.rs @@ -1009,9 +1009,93 @@ mod return_keyword {} // /// The receiver of a method, or the current module. /// -/// The documentation for this keyword is [not yet complete]. Pull requests welcome! +/// `self` is used in two situations: referencing the current module and marking +/// the receiver of a method. /// -/// [not yet complete]: https://github.com/rust-lang/rust/issues/34601 +/// In paths, `self` can be used to refer to the current module, either in a +/// [`use`] statement or in a path to access an element: +/// +/// ``` +/// # #![allow(unused_imports)] +/// use std::io::{self, Read}; +/// ``` +/// +/// Is functionally the same as: +/// +/// ``` +/// # #![allow(unused_imports)] +/// use std::io; +/// use std::io::Read; +/// ``` +/// +/// Using `self` to access an element in the current module: +/// +/// ``` +/// # #![allow(dead_code)] +/// # fn main() {} +/// fn foo() {} +/// fn bar() { +/// self::foo() +/// } +/// ``` +/// +/// `self` as the current receiver for a method allows to omit the parameter +/// type most of the time. With the exception of this particularity, `self` is +/// used much like any other parameter: +/// +/// ``` +/// struct Foo(i32); +/// +/// impl Foo { +/// // No `self`. +/// fn new() -> Self { +/// Self(0) +/// } +/// +/// // Consuming `self`. +/// fn consume(self) -> Self { +/// Self(self.0 + 1) +/// } +/// +/// // Borrowing `self`. +/// fn borrow(&self) -> &i32 { +/// &self.0 +/// } +/// +/// // Borrowing `self` mutably. +/// fn borrow_mut(&mut self) -> &mut i32 { +/// &mut self.0 +/// } +/// } +/// +/// // This method must be called with a `Type::` prefix. +/// let foo = Foo::new(); +/// assert_eq!(foo.0, 0); +/// +/// // Those two calls produces the same result. +/// let foo = Foo::consume(foo); +/// assert_eq!(foo.0, 1); +/// let foo = foo.consume(); +/// assert_eq!(foo.0, 2); +/// +/// // Borrowing is handled automatically with the second syntax. +/// let borrow_1 = Foo::borrow(&foo); +/// let borrow_2 = foo.borrow(); +/// assert_eq!(borrow_1, borrow_2); +/// +/// // Borrowing mutably is handled automatically too with the second syntax. +/// let mut foo = Foo::new(); +/// *Foo::borrow_mut(&mut foo) += 1; +/// assert_eq!(foo.0, 1); +/// *foo.borrow_mut() += 1; +/// assert_eq!(foo.0, 2); +/// ``` +/// +/// Note that this automatic conversion when calling `foo.method()` is not +/// limited to the examples above. See the [Reference] for more information. +/// +/// [`use`]: keyword.use.html +/// [Reference]: ../reference/items/associated-items.html#methods mod self_keyword {} #[doc(keyword = "Self")] From 78baf421aa31ccd04243ee6b6e804999f835f880 Mon Sep 17 00:00:00 2001 From: LeSeulArtichaut Date: Wed, 24 Jun 2020 15:33:08 +0200 Subject: [PATCH 08/10] Add procedure for prioritization notifications on Zulip --- triagebot.toml | 76 +++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 69 insertions(+), 7 deletions(-) diff --git a/triagebot.toml b/triagebot.toml index 5361a618d4e..a939a3e8106 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -78,42 +78,104 @@ exclude_labels = [ [notify-zulip."I-prioritize"] zulip_stream = 227806 # #t-compiler/wg-prioritization topic = "I-prioritize #{number} {title}" -message_on_add = "@*WG-prioritization* issue #{number} has been requested for prioritization." +message_on_add = """\ +@*WG-prioritization* issue #{number} has been requested for prioritization. + +# [Procedure](https://hackmd.io/WJ0G17DHTHGgv0OW9I2PxA?view#Unprioritized-I-prioritize) +- Priority? +- Regression? +- Notify people/groups? +- Needs `I-nominated`? +""" message_on_remove = "Issue #{number}'s prioritization request has been removed." [notify-zulip."I-nominated"] required_labels = ["T-compiler"] zulip_stream = 227806 # #t-compiler/wg-prioritization topic = "I-prioritize #{number} {title}" -message_on_add = "@*WG-prioritization* #{number} has been nominated for discussion in `T-compiler` meeting." +message_on_add = """\ +#{number} has been nominated for discussion in `T-compiler` meeting. + +# [Procedure](https://hackmd.io/WJ0G17DHTHGgv0OW9I2PxA?view#I-nominated) +- Already discussed? +- Worth the meeting time? +- Add agenda entry: + - Why nominated? + - Assignee? + - Issue? PR? What's the status? + - Summary and important details? +""" message_on_remove = "#{number}'s nomination has been removed." [notify-zulip."beta-nominated"] zulip_stream = 227806 # #t-compiler/wg-prioritization topic = "Backport #{number} {title}" -message_on_add = "@*WG-prioritization* PR #{number} has been requested for beta backport." +message_on_add = """\ +PR #{number} has been requested for beta backport. + +# [Procedure](https://hackmd.io/WJ0G17DHTHGgv0OW9I2PxA?view#StableBeta-nominations) +Prepare agenda entry: +- Why nominated? +- Author, assignee? +- Important details? +""" message_on_remove = "PR #{number}'s beta backport request has been removed." [notify-zulip."stable-nominated"] zulip_stream = 227806 # #t-compiler/wg-prioritization topic = "Backport #{number} {title}" -message_on_add = "@*WG-prioritization* PR #{number} has been requested for stable backport." +message_on_add = """\ +PR #{number} has been requested for stable backport. + +# [Procedure](https://hackmd.io/WJ0G17DHTHGgv0OW9I2PxA?view#StableBeta-nominations) +Prepare agenda entry: +- Why nominated? +- Author, assignee? +- Important details? +""" message_on_remove = "PR #{number}'s stable backport request has been removed." [notify-zulip."S-waiting-on-team"] required_labels = ["T-compiler"] zulip_stream = 227806 # #t-compiler/wg-prioritization topic = "S-waiting-on-team #{number} {title}" -message_on_add = "@*WG-prioritization* PR #{number} is waiting on `T-compiler`." +message_on_add = """\ +PR #{number} is waiting on `T-compiler`. + +# [Procedure](https://hackmd.io/WJ0G17DHTHGgv0OW9I2PxA?view#PR%E2%80%99s-waiting-on-team) +- Prepare agenda entry: + - What is it waiting for? + - Important details? +- Could be resolved quickly? Tag `I-nominated`. +""" message_on_remove = "PR #{number}'s is no longer waiting on `T-compiler`." [notify-zulip."P-critical"] zulip_stream = 227806 # #t-compiler/wg-prioritization topic = "P-critical #{number} {title}" -message_on_add = "@*WG-prioritization* issue #{number} has been assigned `P-critical`." +message_on_add = """\ +Issue #{number} has been assigned `P-critical`. + +# [Procedure](https://hackmd.io/WJ0G17DHTHGgv0OW9I2PxA?view#P-critical-and-Unassigned-P-high-regressions) +- Notify people/groups? +- Assign if possible? +- Add to agenda: + - Assignee? + - Summary and important details? +- Other actions to move forward? +""" [notify-zulip."P-high"] required_labels = ["regression-from-stable-to-[bn]*"] # only nightly and beta regressions zulip_stream = 227806 # #t-compiler/wg-prioritization topic = "P-high regression #{number} {title}" -message_on_add = "@*WG-prioritization* issue #{number} has been assigned `P-high` and is a regression." +message_on_add = """\ +Issue #{number} has been assigned `P-high` and is a regression. + +# [Procedure](https://hackmd.io/WJ0G17DHTHGgv0OW9I2PxA?view#P-critical-and-Unassigned-P-high-regressions) +Is issue assigned? If not: +- Try to find an assignee? +- Otherwise add to agenda: + - Mark as unassigned. + - Summary and important details? +""" From 5881b52d9af78fad44320bfe1a8be9369160205c Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Wed, 24 Jun 2020 12:32:44 -0300 Subject: [PATCH 09/10] Change wg-prioritization stream --- triagebot.toml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/triagebot.toml b/triagebot.toml index a939a3e8106..6b57b634004 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -76,7 +76,7 @@ exclude_labels = [ ] [notify-zulip."I-prioritize"] -zulip_stream = 227806 # #t-compiler/wg-prioritization +zulip_stream = 245100 # #t-compiler/wg-prioritization/alerts topic = "I-prioritize #{number} {title}" message_on_add = """\ @*WG-prioritization* issue #{number} has been requested for prioritization. @@ -91,7 +91,7 @@ message_on_remove = "Issue #{number}'s prioritization request has been removed." [notify-zulip."I-nominated"] required_labels = ["T-compiler"] -zulip_stream = 227806 # #t-compiler/wg-prioritization +zulip_stream = 245100 # #t-compiler/wg-prioritization/alerts topic = "I-prioritize #{number} {title}" message_on_add = """\ #{number} has been nominated for discussion in `T-compiler` meeting. @@ -108,7 +108,7 @@ message_on_add = """\ message_on_remove = "#{number}'s nomination has been removed." [notify-zulip."beta-nominated"] -zulip_stream = 227806 # #t-compiler/wg-prioritization +zulip_stream = 245100 # #t-compiler/wg-prioritization/alerts topic = "Backport #{number} {title}" message_on_add = """\ PR #{number} has been requested for beta backport. @@ -122,7 +122,7 @@ Prepare agenda entry: message_on_remove = "PR #{number}'s beta backport request has been removed." [notify-zulip."stable-nominated"] -zulip_stream = 227806 # #t-compiler/wg-prioritization +zulip_stream = 245100 # #t-compiler/wg-prioritization/alerts topic = "Backport #{number} {title}" message_on_add = """\ PR #{number} has been requested for stable backport. @@ -137,7 +137,7 @@ message_on_remove = "PR #{number}'s stable backport request has been removed." [notify-zulip."S-waiting-on-team"] required_labels = ["T-compiler"] -zulip_stream = 227806 # #t-compiler/wg-prioritization +zulip_stream = 245100 # #t-compiler/wg-prioritization/alerts topic = "S-waiting-on-team #{number} {title}" message_on_add = """\ PR #{number} is waiting on `T-compiler`. @@ -151,7 +151,7 @@ PR #{number} is waiting on `T-compiler`. message_on_remove = "PR #{number}'s is no longer waiting on `T-compiler`." [notify-zulip."P-critical"] -zulip_stream = 227806 # #t-compiler/wg-prioritization +zulip_stream = 245100 # #t-compiler/wg-prioritization/alerts topic = "P-critical #{number} {title}" message_on_add = """\ Issue #{number} has been assigned `P-critical`. @@ -167,7 +167,7 @@ Issue #{number} has been assigned `P-critical`. [notify-zulip."P-high"] required_labels = ["regression-from-stable-to-[bn]*"] # only nightly and beta regressions -zulip_stream = 227806 # #t-compiler/wg-prioritization +zulip_stream = 245100 # #t-compiler/wg-prioritization/alerts topic = "P-high regression #{number} {title}" message_on_add = """\ Issue #{number} has been assigned `P-high` and is a regression. From ff068762a0a1a7bc9559101ba74d16862b7028b6 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Wed, 24 Jun 2020 12:36:17 -0300 Subject: [PATCH 10/10] Alert @WG-prioritization/alerts instead of @WG-prioritization --- triagebot.toml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/triagebot.toml b/triagebot.toml index 6b57b634004..73ca7abfed3 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -79,7 +79,7 @@ exclude_labels = [ zulip_stream = 245100 # #t-compiler/wg-prioritization/alerts topic = "I-prioritize #{number} {title}" message_on_add = """\ -@*WG-prioritization* issue #{number} has been requested for prioritization. +@*WG-prioritization/alerts* issue #{number} has been requested for prioritization. # [Procedure](https://hackmd.io/WJ0G17DHTHGgv0OW9I2PxA?view#Unprioritized-I-prioritize) - Priority? @@ -94,7 +94,7 @@ required_labels = ["T-compiler"] zulip_stream = 245100 # #t-compiler/wg-prioritization/alerts topic = "I-prioritize #{number} {title}" message_on_add = """\ -#{number} has been nominated for discussion in `T-compiler` meeting. +@*WG-prioritization/alerts* #{number} has been nominated for discussion in `T-compiler` meeting. # [Procedure](https://hackmd.io/WJ0G17DHTHGgv0OW9I2PxA?view#I-nominated) - Already discussed? @@ -111,7 +111,7 @@ message_on_remove = "#{number}'s nomination has been removed." zulip_stream = 245100 # #t-compiler/wg-prioritization/alerts topic = "Backport #{number} {title}" message_on_add = """\ -PR #{number} has been requested for beta backport. +@*WG-prioritization/alerts* PR #{number} has been requested for beta backport. # [Procedure](https://hackmd.io/WJ0G17DHTHGgv0OW9I2PxA?view#StableBeta-nominations) Prepare agenda entry: @@ -125,7 +125,7 @@ message_on_remove = "PR #{number}'s beta backport request has been removed." zulip_stream = 245100 # #t-compiler/wg-prioritization/alerts topic = "Backport #{number} {title}" message_on_add = """\ -PR #{number} has been requested for stable backport. +@*WG-prioritization/alerts* PR #{number} has been requested for stable backport. # [Procedure](https://hackmd.io/WJ0G17DHTHGgv0OW9I2PxA?view#StableBeta-nominations) Prepare agenda entry: @@ -140,7 +140,7 @@ required_labels = ["T-compiler"] zulip_stream = 245100 # #t-compiler/wg-prioritization/alerts topic = "S-waiting-on-team #{number} {title}" message_on_add = """\ -PR #{number} is waiting on `T-compiler`. +@*WG-prioritization/alerts* PR #{number} is waiting on `T-compiler`. # [Procedure](https://hackmd.io/WJ0G17DHTHGgv0OW9I2PxA?view#PR%E2%80%99s-waiting-on-team) - Prepare agenda entry: @@ -154,7 +154,7 @@ message_on_remove = "PR #{number}'s is no longer waiting on `T-compiler`." zulip_stream = 245100 # #t-compiler/wg-prioritization/alerts topic = "P-critical #{number} {title}" message_on_add = """\ -Issue #{number} has been assigned `P-critical`. +@*WG-prioritization/alerts* issue #{number} has been assigned `P-critical`. # [Procedure](https://hackmd.io/WJ0G17DHTHGgv0OW9I2PxA?view#P-critical-and-Unassigned-P-high-regressions) - Notify people/groups? @@ -170,7 +170,7 @@ required_labels = ["regression-from-stable-to-[bn]*"] # only nightly and beta re zulip_stream = 245100 # #t-compiler/wg-prioritization/alerts topic = "P-high regression #{number} {title}" message_on_add = """\ -Issue #{number} has been assigned `P-high` and is a regression. +@*WG-prioritization/alerts* issue #{number} has been assigned `P-high` and is a regression. # [Procedure](https://hackmd.io/WJ0G17DHTHGgv0OW9I2PxA?view#P-critical-and-Unassigned-P-high-regressions) Is issue assigned? If not: