From 3e95dae14cd15090496b231163f0fe82cddb3d8b Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Wed, 22 Dec 2021 02:33:27 +0100 Subject: [PATCH 1/3] Relax Sized bound on `Id` to better allow `extern type`s --- objc2/CHANGELOG.md | 3 +++ objc2/src/message/mod.rs | 4 +-- objc2/src/rc/autorelease.rs | 4 +-- objc2/src/rc/id.rs | 42 +++++++++++++++++------------ objc2/src/rc/id_forwarding_impls.rs | 40 +++++++++++++-------------- objc2/src/rc/id_traits.rs | 11 ++++---- objc2/src/rc/weak_id.rs | 18 +++++++------ 7 files changed, 67 insertions(+), 55 deletions(-) diff --git a/objc2/CHANGELOG.md b/objc2/CHANGELOG.md index 966247780..cc82c23e1 100644 --- a/objc2/CHANGELOG.md +++ b/objc2/CHANGELOG.md @@ -36,6 +36,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - `Class::instance_variables` - `Protocol::protocols` - `Protocol::adopted_protocols` +* Relaxed `Sized` bound on `rc::Id` and `rc::WeakId` to prepare for + `extern type` support. +* **BREAKING**: Relaxed `Sized` bound on `rc::SliceId` and `rc::DefaultId`. ### Removed * **BREAKING**: Removed the raw FFI functions from the `runtime` module. These diff --git a/objc2/src/message/mod.rs b/objc2/src/message/mod.rs index 19954eed3..8c994875f 100644 --- a/objc2/src/message/mod.rs +++ b/objc2/src/message/mod.rs @@ -79,7 +79,7 @@ pub(crate) mod private { impl<'a, T: Message + ?Sized> Sealed for &'a T {} impl<'a, T: Message + ?Sized> Sealed for &'a mut T {} impl Sealed for NonNull {} - impl Sealed for Id {} + impl Sealed for Id {} impl Sealed for ManuallyDrop {} } @@ -248,7 +248,7 @@ unsafe impl MessageReceiver for NonNull { } } -unsafe impl MessageReceiver for Id { +unsafe impl MessageReceiver for Id { #[inline] fn as_raw_receiver(&self) -> *mut Object { // TODO: Maybe don't dereference here, just to be safe? diff --git a/objc2/src/rc/autorelease.rs b/objc2/src/rc/autorelease.rs index 4cff998eb..8e5b99493 100644 --- a/objc2/src/rc/autorelease.rs +++ b/objc2/src/rc/autorelease.rs @@ -101,7 +101,7 @@ impl AutoreleasePool { /// the lifetime is bound to the pool instead of being unbounded. #[inline] #[allow(clippy::needless_lifetimes)] - pub unsafe fn ptr_as_ref<'p, T>(&'p self, ptr: *const T) -> &'p T { + pub unsafe fn ptr_as_ref<'p, T: ?Sized>(&'p self, ptr: *const T) -> &'p T { self.__verify_is_inner(); // SAFETY: Checked by the caller unsafe { &*ptr } @@ -122,7 +122,7 @@ impl AutoreleasePool { #[inline] #[allow(clippy::needless_lifetimes)] #[allow(clippy::mut_from_ref)] - pub unsafe fn ptr_as_mut<'p, T>(&'p self, ptr: *mut T) -> &'p mut T { + pub unsafe fn ptr_as_mut<'p, T: ?Sized>(&'p self, ptr: *mut T) -> &'p mut T { self.__verify_is_inner(); // SAFETY: Checked by the caller unsafe { &mut *ptr } diff --git a/objc2/src/rc/id.rs b/objc2/src/rc/id.rs index eadf93b59..44ecc956f 100644 --- a/objc2/src/rc/id.rs +++ b/objc2/src/rc/id.rs @@ -53,6 +53,9 @@ use crate::Message; /// do so. If you need to run some code when the object is destroyed, /// implement the `dealloc` method instead. /// +/// This allows `?Sized` types `T`, but the intention is to only support when +/// `T` is an `extern type` (yet unstable). +/// /// # Examples /// /// ```no_run @@ -95,9 +98,9 @@ use crate::Message; /// ``` #[repr(transparent)] // TODO: Figure out if `Message` bound on `T` would be better here? -// TODO: Add `?Sized + ptr::Thin` bound on `T` to allow for extern types +// TODO: Add `ptr::Thin` bound on `T` to allow for only extern types // TODO: Consider changing the name of Id -> Retain -pub struct Id { +pub struct Id { /// A pointer to the contained object. The pointer is always retained. /// /// It is important that this is `NonNull`, since we want to dereference @@ -116,8 +119,7 @@ pub struct Id { own: PhantomData, } -// TODO: Maybe make most of these functions "associated" functions instead? -impl Id { +impl Id { /// Constructs an [`Id`] to an object that already has +1 retain count. /// /// This is useful when you have a retain count that has been handed off @@ -171,7 +173,10 @@ impl Id { own: PhantomData, } } +} +// TODO: Add ?Sized bound +impl Id { /// Retains the given object pointer. /// /// This is useful when you have been given a pointer to an object from @@ -254,6 +259,7 @@ impl Id { // } // } +// TODO: Add ?Sized bound impl Id { /// Autoreleases the owned [`Id`], returning a mutable reference bound to /// the pool. @@ -291,6 +297,7 @@ impl Id { } } +// TODO: Add ?Sized bound impl Id { /// Autoreleases the shared [`Id`], returning an aliased reference bound /// to the pool. @@ -308,7 +315,7 @@ impl Id { } } -impl From> for Id { +impl From> for Id { /// Downgrade from an owned to a shared [`Id`], allowing it to be cloned. #[inline] fn from(obj: Id) -> Self { @@ -318,6 +325,7 @@ impl From> for Id { } } +// TODO: Add ?Sized bound impl Clone for Id { /// Makes a clone of the shared object. /// @@ -338,7 +346,7 @@ impl Clone for Id { /// borrowed data. /// /// [dropck_eyepatch]: https://doc.rust-lang.org/nightly/nomicon/dropck.html#an-escape-hatch -impl Drop for Id { +impl Drop for Id { /// Releases the retained object. /// /// The contained object's destructor (if it has one) is never run! @@ -363,7 +371,7 @@ impl Drop for Id { /// clone a `Id`, send it to another thread, and drop the clone /// last, making `dealloc` get called on the other thread, and violate /// `T: !Send`. -unsafe impl Send for Id {} +unsafe impl Send for Id {} /// The `Sync` implementation requires `T: Sync` because `&Id` give /// access to `&T`. @@ -371,17 +379,17 @@ unsafe impl Send for Id {} /// Additiontally, it requires `T: Send`, because if `T: !Send`, you could /// clone a `&Id` from another thread, and drop the clone last, /// making `dealloc` get called on the other thread, and violate `T: !Send`. -unsafe impl Sync for Id {} +unsafe impl Sync for Id {} /// `Id` are `Send` if `T` is `Send` because they give the same /// access as having a T directly. -unsafe impl Send for Id {} +unsafe impl Send for Id {} /// `Id` are `Sync` if `T` is `Sync` because they give the same /// access as having a `T` directly. -unsafe impl Sync for Id {} +unsafe impl Sync for Id {} -impl Deref for Id { +impl Deref for Id { type Target = T; /// Obtain an immutable reference to the object. @@ -393,7 +401,7 @@ impl Deref for Id { } } -impl DerefMut for Id { +impl DerefMut for Id { /// Obtain a mutable reference to the object. #[inline] fn deref_mut(&mut self) -> &mut T { @@ -404,7 +412,7 @@ impl DerefMut for Id { } } -impl fmt::Pointer for Id { +impl fmt::Pointer for Id { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Pointer::fmt(&self.ptr.as_ptr(), f) } @@ -414,15 +422,15 @@ impl fmt::Pointer for Id { // // See https://doc.rust-lang.org/1.54.0/src/alloc/boxed.rs.html#1652-1675 // and the `Arc` implementation. -impl Unpin for Id {} +impl Unpin for Id {} -impl RefUnwindSafe for Id {} +impl RefUnwindSafe for Id {} // Same as `Arc`. -impl UnwindSafe for Id {} +impl UnwindSafe for Id {} // Same as `Box`. -impl UnwindSafe for Id {} +impl UnwindSafe for Id {} #[cfg(test)] mod tests { diff --git a/objc2/src/rc/id_forwarding_impls.rs b/objc2/src/rc/id_forwarding_impls.rs index a03d27dba..1f2b3fefc 100644 --- a/objc2/src/rc/id_forwarding_impls.rs +++ b/objc2/src/rc/id_forwarding_impls.rs @@ -21,7 +21,7 @@ use std::io; use super::{Id, Owned, Ownership}; -impl PartialEq for Id { +impl PartialEq for Id { #[inline] fn eq(&self, other: &Self) -> bool { (**self).eq(&**other) @@ -34,9 +34,9 @@ impl PartialEq for Id { } } -impl Eq for Id {} +impl Eq for Id {} -impl PartialOrd for Id { +impl PartialOrd for Id { #[inline] fn partial_cmp(&self, other: &Self) -> Option { (**self).partial_cmp(&**other) @@ -59,20 +59,20 @@ impl PartialOrd for Id { } } -impl Ord for Id { +impl Ord for Id { #[inline] fn cmp(&self, other: &Self) -> Ordering { (**self).cmp(&**other) } } -impl hash::Hash for Id { +impl hash::Hash for Id { fn hash(&self, state: &mut H) { (**self).hash(state) } } -impl hash::Hasher for Id { +impl hash::Hasher for Id { fn finish(&self) -> u64 { (**self).finish() } @@ -117,19 +117,19 @@ impl hash::Hasher for Id { } } -impl fmt::Display for Id { +impl fmt::Display for Id { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { (**self).fmt(f) } } -impl fmt::Debug for Id { +impl fmt::Debug for Id { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { (**self).fmt(f) } } -impl Iterator for Id { +impl Iterator for Id { type Item = I::Item; fn next(&mut self) -> Option { (**self).next() @@ -142,7 +142,7 @@ impl Iterator for Id { } } -impl DoubleEndedIterator for Id { +impl DoubleEndedIterator for Id { fn next_back(&mut self) -> Option { (**self).next_back() } @@ -151,13 +151,13 @@ impl DoubleEndedIterator for Id { } } -impl ExactSizeIterator for Id { +impl ExactSizeIterator for Id { fn len(&self) -> usize { (**self).len() } } -impl FusedIterator for Id {} +impl FusedIterator for Id {} impl borrow::Borrow for Id { fn borrow(&self) -> &T { @@ -171,25 +171,25 @@ impl borrow::BorrowMut for Id { } } -impl AsRef for Id { +impl AsRef for Id { fn as_ref(&self) -> &T { &**self } } -impl AsMut for Id { +impl AsMut for Id { fn as_mut(&mut self) -> &mut T { &mut **self } } -impl Error for Id { +impl Error for Id { fn source(&self) -> Option<&(dyn Error + 'static)> { (**self).source() } } -impl io::Read for Id { +impl io::Read for Id { #[inline] fn read(&mut self, buf: &mut [u8]) -> io::Result { (**self).read(buf) @@ -216,7 +216,7 @@ impl io::Read for Id { } } -impl io::Write for Id { +impl io::Write for Id { #[inline] fn write(&mut self, buf: &[u8]) -> io::Result { (**self).write(buf) @@ -243,7 +243,7 @@ impl io::Write for Id { } } -impl io::Seek for Id { +impl io::Seek for Id { #[inline] fn seek(&mut self, pos: io::SeekFrom) -> io::Result { (**self).seek(pos) @@ -255,7 +255,7 @@ impl io::Seek for Id { } } -impl io::BufRead for Id { +impl io::BufRead for Id { #[inline] fn fill_buf(&mut self) -> io::Result<&[u8]> { (**self).fill_buf() @@ -277,7 +277,7 @@ impl io::BufRead for Id { } } -impl Future for Id { +impl Future for Id { type Output = T::Output; fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { diff --git a/objc2/src/rc/id_traits.rs b/objc2/src/rc/id_traits.rs index 4ffaf9252..1e575579e 100644 --- a/objc2/src/rc/id_traits.rs +++ b/objc2/src/rc/id_traits.rs @@ -6,7 +6,7 @@ use crate::Message; /// Helper trait for functionality on slices containing [`Id`]s. pub trait SliceId { /// The type of the items in the slice. - type Item; + type Item: ?Sized; /// Convert a slice of [`Id`]s into a slice of references. fn as_slice_ref(&self) -> &[&Self::Item]; @@ -22,7 +22,7 @@ pub trait SliceIdMut: SliceId { fn as_mut_slice_mut(&mut self) -> &mut [&mut Self::Item]; } -impl SliceId for [Id] { +impl SliceId for [Id] { type Item = T; fn as_slice_ref(&self) -> &[&T] { @@ -40,7 +40,7 @@ impl SliceId for [Id] { } } -impl SliceIdMut for [Id] { +impl SliceIdMut for [Id] { fn as_mut_slice_mut(&mut self) -> &mut [&mut T] { let ptr = self as *mut Self as *mut [&mut T]; // SAFETY: Id and &mut T have the same memory layout, and the @@ -52,9 +52,8 @@ impl SliceIdMut for [Id] { /// Helper trait to implement [`Default`] on types whoose default value is an /// [`Id`]. -// TODO: Remove `Sized` bound. // TODO: Maybe make this `unsafe` and provide a default implementation? -pub trait DefaultId: Sized { +pub trait DefaultId { /// Indicates whether the default value is mutable or immutable. type Ownership: Ownership; @@ -65,7 +64,7 @@ pub trait DefaultId: Sized { fn default_id() -> Id; } -impl Default for Id { +impl Default for Id { fn default() -> Self { T::default_id() } diff --git a/objc2/src/rc/weak_id.rs b/objc2/src/rc/weak_id.rs index 965ed825f..509e223b2 100644 --- a/objc2/src/rc/weak_id.rs +++ b/objc2/src/rc/weak_id.rs @@ -16,7 +16,7 @@ use crate::Message; /// Allows breaking reference cycles and safely checking whether the object /// has been deallocated. #[repr(transparent)] -pub struct WeakId { +pub struct WeakId { /// We give the runtime the address to this box, so that it can modify it /// even if the `WeakId` is moved. /// @@ -69,7 +69,7 @@ impl WeakId { } } -impl Drop for WeakId { +impl Drop for WeakId { /// Drops the `WeakId` pointer. #[doc(alias = "objc_destroyWeak")] fn drop(&mut self) { @@ -77,6 +77,7 @@ impl Drop for WeakId { } } +// TODO: Add ?Sized impl Clone for WeakId { /// Makes a clone of the `WeakId` that points to the same object. #[doc(alias = "objc_copyWeak")] @@ -90,6 +91,7 @@ impl Clone for WeakId { } } +// TODO: Add ?Sized impl Default for WeakId { /// Constructs a new `WeakId` that doesn't reference any object. /// @@ -101,26 +103,26 @@ impl Default for WeakId { } /// This implementation follows the same reasoning as `Id`. -unsafe impl Sync for WeakId {} +unsafe impl Sync for WeakId {} /// This implementation follows the same reasoning as `Id`. -unsafe impl Send for WeakId {} +unsafe impl Send for WeakId {} // Unsure about the Debug bound on T, see std::sync::Weak -impl fmt::Debug for WeakId { +impl fmt::Debug for WeakId { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "(WeakId)") } } // Underneath this is just a `Box` -impl Unpin for WeakId {} +impl Unpin for WeakId {} // Same as `Id`. -impl RefUnwindSafe for WeakId {} +impl RefUnwindSafe for WeakId {} // Same as `Id`. -impl UnwindSafe for WeakId {} +impl UnwindSafe for WeakId {} #[cfg(test)] mod tests { From aa783eb47e3a9f086249de6f3622a41affc45023 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Wed, 22 Dec 2021 02:34:22 +0100 Subject: [PATCH 2/3] Remove Sized bound on INSObject --- objc2-foundation/CHANGELOG.md | 1 + objc2-foundation/src/array.rs | 2 +- objc2-foundation/src/dictionary.rs | 4 ++-- objc2-foundation/src/enumerator.rs | 8 ++++---- objc2-foundation/src/object.rs | 6 +----- 5 files changed, 9 insertions(+), 12 deletions(-) diff --git a/objc2-foundation/CHANGELOG.md b/objc2-foundation/CHANGELOG.md index 241496e00..ef519c821 100644 --- a/objc2-foundation/CHANGELOG.md +++ b/objc2-foundation/CHANGELOG.md @@ -21,6 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Removed * **BREAKING**: Removed associated `Ownership` type from `INSObject`; instead, it is present on the types that actually need it (for example `NSCopying`). +* **BREAKING**: Removed `Sized` bound on `INSObject`. ### Fixed * Soundness issue with `NSValue`, `NSDictionary`, `NSArray` and diff --git a/objc2-foundation/src/array.rs b/objc2-foundation/src/array.rs index bfac18047..097fe8c6d 100644 --- a/objc2-foundation/src/array.rs +++ b/objc2-foundation/src/array.rs @@ -14,7 +14,7 @@ use super::{ NSRange, }; -unsafe fn from_refs(refs: &[&A::Item]) -> Id { +unsafe fn from_refs(refs: &[&A::Item]) -> Id { let cls = A::class(); let obj: *mut A = unsafe { msg_send![cls, alloc] }; let obj: *mut A = unsafe { diff --git a/objc2-foundation/src/dictionary.rs b/objc2-foundation/src/dictionary.rs index a68d63cb4..ab60e6da6 100644 --- a/objc2-foundation/src/dictionary.rs +++ b/objc2-foundation/src/dictionary.rs @@ -11,8 +11,8 @@ use super::{INSCopying, INSFastEnumeration, INSObject, NSArray, NSEnumerator}; unsafe fn from_refs(keys: &[&T], vals: &[&D::Value]) -> Id where - D: INSDictionary, - T: INSCopying, + D: INSDictionary + ?Sized, + T: INSCopying + ?Sized, { let cls = D::class(); let count = min(keys.len(), vals.len()); diff --git a/objc2-foundation/src/enumerator.rs b/objc2-foundation/src/enumerator.rs index adfd748f3..32bcba0f7 100644 --- a/objc2-foundation/src/enumerator.rs +++ b/objc2-foundation/src/enumerator.rs @@ -94,7 +94,7 @@ unsafe impl RefEncode for NSFastEnumerationState { const ENCODING_REF: Encoding<'static> = Encoding::Pointer(&Self::ENCODING); } -fn enumerate<'a, 'b: 'a, C: INSFastEnumeration>( +fn enumerate<'a, 'b: 'a, C: INSFastEnumeration + ?Sized>( object: &'b C, state: &mut NSFastEnumerationState, buf: &'a mut [*const C::Item], @@ -119,7 +119,7 @@ fn enumerate<'a, 'b: 'a, C: INSFastEnumeration>( const FAST_ENUM_BUF_SIZE: usize = 16; -pub struct NSFastEnumerator<'a, C: 'a + INSFastEnumeration> { +pub struct NSFastEnumerator<'a, C: 'a + INSFastEnumeration + ?Sized> { object: &'a C, ptr: *const *const C::Item, @@ -129,7 +129,7 @@ pub struct NSFastEnumerator<'a, C: 'a + INSFastEnumeration> { buf: [*const C::Item; FAST_ENUM_BUF_SIZE], } -impl<'a, C: INSFastEnumeration> NSFastEnumerator<'a, C> { +impl<'a, C: INSFastEnumeration + ?Sized> NSFastEnumerator<'a, C> { fn new(object: &'a C) -> Self { Self { object, @@ -174,7 +174,7 @@ impl<'a, C: INSFastEnumeration> NSFastEnumerator<'a, C> { } } -impl<'a, C: INSFastEnumeration> Iterator for NSFastEnumerator<'a, C> { +impl<'a, C: INSFastEnumeration + ?Sized> Iterator for NSFastEnumerator<'a, C> { type Item = &'a C::Item; fn next(&mut self) -> Option<&'a C::Item> { diff --git a/objc2-foundation/src/object.rs b/objc2-foundation/src/object.rs index 11cb2b3e7..049fb5c8c 100644 --- a/objc2-foundation/src/object.rs +++ b/objc2-foundation/src/object.rs @@ -8,11 +8,7 @@ use objc2::Message; use super::NSString; -// The Sized bound is unfortunate; ideally, Objective-C objects would not be -// treated as Sized. However, rust won't allow casting a dynamically-sized -// type pointer to an Object pointer, because dynamically-sized types can have -// fat pointers (two words) instead of real pointers. -pub unsafe trait INSObject: Sized + Message { +pub unsafe trait INSObject: Message { fn class() -> &'static Class; fn hash_code(&self) -> usize { From 82bc5c8988ac2c18d6c1901a21efa07a617a24ab Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Wed, 22 Dec 2021 02:24:28 +0100 Subject: [PATCH 3/3] Relax a few more Sized bounds --- objc2/src/declare.rs | 4 ++-- objc2/src/rc/autorelease.rs | 2 +- objc2/src/runtime.rs | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/objc2/src/declare.rs b/objc2/src/declare.rs index 691d5b197..9bdb02f3a 100644 --- a/objc2/src/declare.rs +++ b/objc2/src/declare.rs @@ -45,7 +45,7 @@ use crate::{ffi, Encode, EncodeArguments, Encoding, Message}; /// Types that can be used as the implementation of an Objective-C method. pub trait MethodImplementation { /// The callee type of the method. - type Callee: Message; + type Callee: Message + ?Sized; /// The return type of the method. type Ret: Encode; /// The argument types of the method. @@ -59,7 +59,7 @@ macro_rules! method_decl_impl { (-$s:ident, $r:ident, $f:ty, $($t:ident),*) => ( impl<$s, $r, $($t),*> MethodImplementation for $f where - $s: Message, + $s: Message + ?Sized, $r: Encode, $($t: Encode,)* { diff --git a/objc2/src/rc/autorelease.rs b/objc2/src/rc/autorelease.rs index 8e5b99493..f22e3bc5e 100644 --- a/objc2/src/rc/autorelease.rs +++ b/objc2/src/rc/autorelease.rs @@ -202,7 +202,7 @@ auto_trait! { } #[cfg(not(feature = "unstable_autoreleasesafe"))] -unsafe impl AutoreleaseSafe for T {} +unsafe impl AutoreleaseSafe for T {} #[cfg(feature = "unstable_autoreleasesafe")] impl !AutoreleaseSafe for AutoreleasePool {} diff --git a/objc2/src/runtime.rs b/objc2/src/runtime.rs index cf339069d..3a3e388d9 100644 --- a/objc2/src/runtime.rs +++ b/objc2/src/runtime.rs @@ -580,17 +580,17 @@ impl Object { /// ``` /// use objc2::runtime::Object; -/// fn needs_nothing() {} +/// fn needs_nothing() {} /// needs_nothing::(); /// ``` /// ```compile_fail /// use objc2::runtime::Object; -/// fn needs_sync() {} +/// fn needs_sync() {} /// needs_sync::(); /// ``` /// ```compile_fail /// use objc2::runtime::Object; -/// fn needs_send() {} +/// fn needs_send() {} /// needs_send::(); /// ``` #[cfg(doctest)] @@ -734,7 +734,7 @@ mod tests { #[test] fn test_send_sync() { - fn assert_send_sync() {} + fn assert_send_sync() {} assert_send_sync::(); assert_send_sync::(); assert_send_sync::();