From 4f84eabf2f602050a2d00e736d8503e385764ce6 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Mon, 30 Jan 2023 02:07:14 +0100 Subject: [PATCH 1/2] Implement Encode and RefEncode for UnsafeCell --- crates/objc2/src/encode/mod.rs | 11 +++++------ crates/test-ui/ui/not_encode.rs | 6 ++++++ crates/test-ui/ui/not_encode.stderr | 27 +++++++++++++++++++++++++-- 3 files changed, 36 insertions(+), 8 deletions(-) diff --git a/crates/objc2/src/encode/mod.rs b/crates/objc2/src/encode/mod.rs index 8ba69adad..6b385ef2e 100644 --- a/crates/objc2/src/encode/mod.rs +++ b/crates/objc2/src/encode/mod.rs @@ -59,6 +59,7 @@ //! method argument, but is a very common return type, and hence implements //! [`Encode`]. +use core::cell::UnsafeCell; use core::ffi::c_void; use core::mem::{self, ManuallyDrop, MaybeUninit}; use core::num::{ @@ -484,11 +485,12 @@ encode_impls_transparent! { // TODO: With specialization: `impl Encode for ManuallyDrop>` ManuallyDrop, + // SAFETY: Guaranteed to have the same in-memory representation `T`. + // // The fact that this has `repr(no_niche)` has no effect on us, since we // don't unconditionally implement `Encode` generically over `Option`. // (e.g. an `Option>` impl is not available). - // The inner field is not public, so may not be stable. - // TODO: UnsafeCell, + UnsafeCell, // The inner field is not public, so may not be safe. // TODO: Pin, @@ -702,13 +704,10 @@ mod tests { assert_eq!(>>::ENCODING, u8::ENCODING_REF); assert_eq!(<&ManuallyDrop>>::ENCODING, <&&u8>::ENCODING); - // assert_eq!(>::ENCODING, u8::ENCODING); + assert_eq!(>::ENCODING, u8::ENCODING); // assert_eq!(>::ENCODING, u8::ENCODING); assert_eq!(>::ENCODING, u8::ENCODING); assert_eq!(>::ENCODING, u8::ENCODING); - - // Shouldn't compile - // assert_eq!(>>::ENCODING, <&u8>::ENCODING); } #[test] diff --git a/crates/test-ui/ui/not_encode.rs b/crates/test-ui/ui/not_encode.rs index 8f745340d..126deb7f9 100644 --- a/crates/test-ui/ui/not_encode.rs +++ b/crates/test-ui/ui/not_encode.rs @@ -1,4 +1,5 @@ //! Verify that certain things we don't want to be encode aren't. +use core::cell::UnsafeCell; use core::ffi::c_void; use objc2::encode::Encode; @@ -22,4 +23,9 @@ fn main() { is_encode::(); is_encode::<&Sel>(); + + // This should compile + is_encode::>(); + // But this mustn't + is_encode::>>(); } diff --git a/crates/test-ui/ui/not_encode.stderr b/crates/test-ui/ui/not_encode.stderr index c5a4e860c..293b8d680 100644 --- a/crates/test-ui/ui/not_encode.stderr +++ b/crates/test-ui/ui/not_encode.stderr @@ -224,6 +224,29 @@ note: required by a bound in `is_encode` | ^^^^^^ required by this bound in `is_encode` help: consider removing the leading `&`-reference | -24 - is_encode::<&Sel>(); -24 + is_encode::(); +25 - is_encode::<&Sel>(); +25 + is_encode::(); | + +error[E0277]: the trait bound `UnsafeCell<&u8>: OptionEncode` is not satisfied + --> ui/not_encode.rs + | + | is_encode::>>(); + | ^^^^^^^^^^^^^^^^^^^^^^^ the trait `OptionEncode` is not implemented for `UnsafeCell<&u8>` + | + = help: the following other types implement trait `OptionEncode`: + &'a T + &'a mut T + NonNull + NonNull + NonZeroI16 + NonZeroI32 + NonZeroI64 + NonZeroI8 + and $N others + = note: required for `Option>` to implement `Encode` +note: required by a bound in `is_encode` + --> ui/not_encode.rs + | + | fn is_encode() {} + | ^^^^^^ required by this bound in `is_encode` From c00d765f46a91bc699406c72e90f6e6bce77156d Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Fri, 27 Jan 2023 12:13:17 +0100 Subject: [PATCH 2/2] Implement Encode and RefEncode for Cell --- crates/objc2/src/encode/mod.rs | 11 +++++++---- crates/test-ui/ui/not_encode.rs | 5 ++++- crates/test-ui/ui/not_encode.stderr | 23 +++++++++++++++++++++++ 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/crates/objc2/src/encode/mod.rs b/crates/objc2/src/encode/mod.rs index 6b385ef2e..fc7373c64 100644 --- a/crates/objc2/src/encode/mod.rs +++ b/crates/objc2/src/encode/mod.rs @@ -59,7 +59,7 @@ //! method argument, but is a very common return type, and hence implements //! [`Encode`]. -use core::cell::UnsafeCell; +use core::cell::{Cell, UnsafeCell}; use core::ffi::c_void; use core::mem::{self, ManuallyDrop, MaybeUninit}; use core::num::{ @@ -492,6 +492,9 @@ encode_impls_transparent! { // (e.g. an `Option>` impl is not available). UnsafeCell, + // SAFETY: Guaranteed to have the same layout as `UnsafeCell`. + Cell, + // The inner field is not public, so may not be safe. // TODO: Pin, @@ -501,9 +504,6 @@ encode_impls_transparent! { // SAFETY: Guaranteed to have the same layout and ABI as `T`. Wrapping, - // It might have requirements that would disourage this impl? - // TODO: Cell - // TODO: Types that need to be made repr(transparent) first: // - core::cell::Ref? // - core::cell::RefCell? @@ -705,6 +705,9 @@ mod tests { assert_eq!(<&ManuallyDrop>>::ENCODING, <&&u8>::ENCODING); assert_eq!(>::ENCODING, u8::ENCODING); + assert_eq!(>::ENCODING, <&u8>::ENCODING); + assert_eq!(>::ENCODING, u8::ENCODING); + assert_eq!(>::ENCODING, <&u8>::ENCODING); // assert_eq!(>::ENCODING, u8::ENCODING); assert_eq!(>::ENCODING, u8::ENCODING); assert_eq!(>::ENCODING, u8::ENCODING); diff --git a/crates/test-ui/ui/not_encode.rs b/crates/test-ui/ui/not_encode.rs index 126deb7f9..a8dfe6894 100644 --- a/crates/test-ui/ui/not_encode.rs +++ b/crates/test-ui/ui/not_encode.rs @@ -1,5 +1,5 @@ //! Verify that certain things we don't want to be encode aren't. -use core::cell::UnsafeCell; +use core::cell::{Cell, UnsafeCell}; use core::ffi::c_void; use objc2::encode::Encode; @@ -28,4 +28,7 @@ fn main() { is_encode::>(); // But this mustn't is_encode::>>(); + + // Same + is_encode::>>(); } diff --git a/crates/test-ui/ui/not_encode.stderr b/crates/test-ui/ui/not_encode.stderr index 293b8d680..11b3a193f 100644 --- a/crates/test-ui/ui/not_encode.stderr +++ b/crates/test-ui/ui/not_encode.stderr @@ -250,3 +250,26 @@ note: required by a bound in `is_encode` | | fn is_encode() {} | ^^^^^^ required by this bound in `is_encode` + +error[E0277]: the trait bound `Cell<&u8>: OptionEncode` is not satisfied + --> ui/not_encode.rs + | + | is_encode::>>(); + | ^^^^^^^^^^^^^^^^^ the trait `OptionEncode` is not implemented for `Cell<&u8>` + | + = help: the following other types implement trait `OptionEncode`: + &'a T + &'a mut T + NonNull + NonNull + NonZeroI16 + NonZeroI32 + NonZeroI64 + NonZeroI8 + and $N others + = note: required for `Option>` to implement `Encode` +note: required by a bound in `is_encode` + --> ui/not_encode.rs + | + | fn is_encode() {} + | ^^^^^^ required by this bound in `is_encode`