From fc4d73b0afb765916145c82e100f06020b3418fa Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Wed, 8 Sep 2021 20:11:07 +0200 Subject: [PATCH 01/16] Make Id able to use the null pointer optimization --- objc2_id/Cargo.toml | 1 + objc2_id/src/id.rs | 107 ++++++++++++++++++++++++++++++++++---------- 2 files changed, 84 insertions(+), 24 deletions(-) diff --git a/objc2_id/Cargo.toml b/objc2_id/Cargo.toml index 7fb326346..3389bb6a4 100644 --- a/objc2_id/Cargo.toml +++ b/objc2_id/Cargo.toml @@ -18,3 +18,4 @@ license = "MIT" [dependencies] objc2 = { path = "../objc2" } +objc2_sys = { path = "../objc2_sys" } diff --git a/objc2_id/src/id.rs b/objc2_id/src/id.rs index f7ff02760..e5bd5ed44 100644 --- a/objc2_id/src/id.rs +++ b/objc2_id/src/id.rs @@ -2,9 +2,11 @@ use core::any::Any; use core::fmt; use core::hash; use core::marker::PhantomData; +use core::mem::ManuallyDrop; use core::ops::{Deref, DerefMut}; +use core::ptr::NonNull; -use objc2::rc::{StrongPtr, WeakPtr}; +use objc2::rc::WeakPtr; use objc2::runtime::Object; use objc2::Message; @@ -35,9 +37,23 @@ impl Ownership for Shared {} /// provide more references to the object. /// An owned [`Id`] can be "downgraded" freely to a [`ShareId`], but there is /// no way to safely upgrade back. +#[repr(transparent)] pub struct Id { - ptr: StrongPtr, + /// A pointer to the contained object. The pointer is always retained. + /// + /// It is important that this is `NonNull`, since we want to dereference + /// it later, and be able to use the null-pointer optimization. + /// + /// Additionally, covariance is correct because we're either the unique + /// owner of `T` (O = Owned), or `T` is immutable (O = Shared). + ptr: NonNull, + /// Necessary for dropck even though we never actually run T's destructor, + /// because it might have a `dealloc` that assumes that contained + /// references outlive the type. + /// + /// See item: PhantomData, + /// To prevent warnings about unused type parameters. own: PhantomData, } @@ -46,7 +62,7 @@ where T: Message, O: Ownership, { - unsafe fn new(ptr: StrongPtr) -> Id { + unsafe fn new(ptr: NonNull) -> Id { Id { ptr, item: PhantomData, @@ -66,11 +82,14 @@ where /// The pointer must be to a valid object and the caller must ensure the /// ownership is correct. pub unsafe fn from_ptr(ptr: *mut T) -> Id { - assert!( - !ptr.is_null(), - "Attempted to construct an Id from a null pointer" + let nonnull = NonNull::new(ptr).expect("Attempted to construct an Id from a null pointer"); + let ptr: *mut T = objc2_sys::objc_retain(nonnull.as_ptr() as *mut _) as *mut _; + debug_assert_eq!( + ptr, + nonnull.as_ptr(), + "objc_retain did not return the same pointer" ); - Id::new(StrongPtr::retain(ptr as *mut Object)) + Id::new(NonNull::new_unchecked(ptr)) } /// Constructs an [`Id`] from a pointer to a retained object; this won't @@ -86,11 +105,7 @@ where /// The pointer must be to a valid object and the caller must ensure the /// ownership is correct. pub unsafe fn from_retained_ptr(ptr: *mut T) -> Id { - assert!( - !ptr.is_null(), - "Attempted to construct an Id from a null pointer" - ); - Id::new(StrongPtr::new(ptr as *mut Object)) + Id::new(NonNull::new(ptr).expect("Attempted to construct an Id from a null pointer")) } } @@ -100,7 +115,7 @@ where { /// Downgrade an owned [`Id`] to a [`ShareId`], allowing it to be cloned. pub fn share(self) -> ShareId { - let Id { ptr, .. } = self; + let ptr = ManuallyDrop::new(self).ptr; unsafe { Id::new(ptr) } } } @@ -110,7 +125,32 @@ where T: Message, { fn clone(&self) -> ShareId { - unsafe { Id::new(self.ptr.clone()) } + unsafe { Id::from_ptr(self.ptr.as_ptr()) } + } +} + +/// `#[may_dangle]` (see [this][dropck_eyepatch]) doesn't apply here since we +/// don't run `T`'s destructor (rather, we want to discourage having `T`s with +/// a destructor); and even if we did run the destructor, it would not be safe +/// to add since we cannot verify that a `dealloc` method doesn't access +/// borrowed data. +/// +/// [dropck_eyepatch]: https://doc.rust-lang.org/nightly/nomicon/dropck.html#an-escape-hatch +impl Drop for Id { + /// Releases the retained object. + /// + /// The contained object's destructor (if it has one) is never run! + #[doc(alias = "objc_release")] + #[doc(alias = "release")] + #[inline] + fn drop(&mut self) { + // We could technically run the destructor for `T` when `O = Owned`, + // and when `O = Shared` with (retainCount == 1), but that would be + // confusing and inconsistent since we cannot guarantee that it's run. + + // SAFETY: The `ptr` is guaranteed to be valid and have at least one + // retain count + unsafe { objc2_sys::objc_release(self.ptr.as_ptr() as *mut _) }; } } @@ -143,13 +183,13 @@ impl Deref for Id { type Target = T; fn deref(&self) -> &T { - unsafe { &*(*self.ptr as *mut T) } + unsafe { self.ptr.as_ref() } } } impl DerefMut for Id { fn deref_mut(&mut self) -> &mut T { - unsafe { &mut *(*self.ptr as *mut T) } + unsafe { self.ptr.as_mut() } } } @@ -187,7 +227,7 @@ where impl fmt::Pointer for Id { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Pointer::fmt(&self.ptr, f) + fmt::Pointer::fmt(&self.ptr.as_ptr(), f) } } @@ -207,8 +247,10 @@ where { /// Construct a new [`WeakId`] referencing the given [`ShareId`]. pub fn new(obj: &ShareId) -> WeakId { + // SAFETY: The pointer is valid + let ptr = unsafe { WeakPtr::new(obj.ptr.as_ptr() as *mut Object) }; WeakId { - ptr: obj.ptr.weak(), + ptr, item: PhantomData, } } @@ -218,11 +260,8 @@ where /// Returns [`None`] if the object has been deallocated. pub fn load(&self) -> Option> { let obj = self.ptr.load(); - if obj.is_null() { - None - } else { - Some(unsafe { Id::new(obj) }) - } + let ptr = *ManuallyDrop::new(obj).deref().deref() as *mut T; + NonNull::new(ptr).map(|ptr| unsafe { Id::new(ptr) }) } } @@ -234,7 +273,9 @@ unsafe impl Send for WeakId {} #[cfg(test)] mod tests { - use super::{Id, ShareId, WeakId}; + use core::mem::size_of; + + use super::{Id, Owned, ShareId, Shared, WeakId}; use objc2::runtime::Object; use objc2::{class, msg_send}; @@ -248,6 +289,24 @@ mod tests { unsafe { msg_send![obj, retainCount] } } + pub struct TestType { + _data: [u8; 0], // TODO: `UnsafeCell`? + } + + #[test] + fn test_size_of() { + assert_eq!(size_of::>(), size_of::<&TestType>()); + assert_eq!(size_of::>(), size_of::<&TestType>()); + assert_eq!( + size_of::>>(), + size_of::<&TestType>() + ); + assert_eq!( + size_of::>>(), + size_of::<&TestType>() + ); + } + #[test] fn test_clone() { let cls = class!(NSObject); From ff7952b0711b7062e44ef141f90b4699bb21b7a6 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Wed, 8 Sep 2021 20:10:08 +0200 Subject: [PATCH 02/16] Add some more documentation to `Id` --- objc2_id/src/id.rs | 72 ++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 60 insertions(+), 12 deletions(-) diff --git a/objc2_id/src/id.rs b/objc2_id/src/id.rs index e5bd5ed44..29cb1885a 100644 --- a/objc2_id/src/id.rs +++ b/objc2_id/src/id.rs @@ -23,21 +23,59 @@ pub trait Ownership: Any {} impl Ownership for Owned {} impl Ownership for Shared {} -/// A pointer type for Objective-C's reference counted objects. +/// An pointer for Objective-C reference counted objects. /// -/// The object of an [`Id`] is retained and sent a `release` message when -/// the [`Id`] is dropped. +/// [`Id`] strongly references or "retains" the given object `T`, and +/// "releases" it again when dropped, thereby ensuring it will be deallocated +/// at the right time. /// -/// An [`Id`] may be either [`Owned`] or [`Shared`], represented by the types -/// [`Id`] and [`ShareId`], respectively. -/// If owned, there are no other references to the object and the [`Id`] can -/// be mutably dereferenced. -/// [`ShareId`], however, can only be immutably dereferenced because there may -/// be other references to the object, but a [`ShareId`] can be cloned to -/// provide more references to the object. -/// An owned [`Id`] can be "downgraded" freely to a [`ShareId`], but there is -/// no way to safely upgrade back. +/// An [`Id`] can either be [`Owned`] or [`Shared`], represented with the `O` +/// type parameter. +/// +/// If owned, it is guaranteed that there are no other references to the +/// object, and the [`Id`] can therefore be mutably dereferenced. +/// +/// If shared, however, it can only be immutably dereferenced because there +/// may be other references to the object, since a shared [`Id`] can be cloned +/// to provide exactly that. +/// +/// An owned [`Id`] can be safely "downgraded", that is, turned into to a +/// shared [`Id`] using [`Id::share`], but there is no way to do the opposite +/// safely. +/// +/// `Option>` is guaranteed to have the same size as a pointer to the +/// object. +/// +/// # Comparison to `std` types +/// +/// `Id` can be thought of as the Objective-C equivalent of [`Box`] +/// from the standard library: It is a unique pointer to some allocated +/// object, and that means you're allowed to get a mutable reference to it. +/// +/// Likewise, `Id` is the Objective-C equivalent of [`Arc`]: It is +/// a reference-counting pointer that, when cloned, increases the reference +/// count. +/// +/// # Caveats +/// +/// If the inner type implements [`Drop`], that implementation will not be +/// called, since there is no way to ensure that the Objective-C runtime will +/// do so. If you need to run some code when the object is destroyed, +/// implement the `dealloc` method instead. +/// +/// # Examples +/// +/// ```no_run +/// let owned: Id; +/// let mut_ref: &mut T = *owned; +/// // Do something with `&mut T` here +/// +/// let shared: Id = owned.share(); +/// let cloned: Id = shared.clone(); +/// // Do something with `&T` here +/// ``` #[repr(transparent)] +// TODO: Figure out if `Message` bound on `T` would be better here? pub struct Id { /// A pointer to the contained object. The pointer is always retained. /// @@ -57,6 +95,7 @@ pub struct Id { own: PhantomData, } +// TODO: Maybe make most of these functions "associated" functions instead? impl Id where T: Message, @@ -81,6 +120,7 @@ where /// /// The pointer must be to a valid object and the caller must ensure the /// ownership is correct. + #[doc(alias = "objc_retain")] pub unsafe fn from_ptr(ptr: *mut T) -> Id { let nonnull = NonNull::new(ptr).expect("Attempted to construct an Id from a null pointer"); let ptr: *mut T = objc2_sys::objc_retain(nonnull.as_ptr() as *mut _) as *mut _; @@ -109,6 +149,7 @@ where } } +// TODO: Change this to be a [`From`] implementation. impl Id where T: Message, @@ -124,7 +165,14 @@ impl Clone for Id where T: Message, { + /// Makes a clone of the shared object. + /// + /// This increases the object's reference count. + #[doc(alias = "objc_retain")] + #[doc(alias = "retain")] + #[inline] fn clone(&self) -> ShareId { + // SAFETY: We already know the `ptr` is valid unsafe { Id::from_ptr(self.ptr.as_ptr()) } } } From ec47c987537c6dbf180fa3a10cd78abf621ed9d0 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Wed, 8 Sep 2021 23:52:40 +0200 Subject: [PATCH 03/16] Change Id::share to a From implementation and add Id::from_shared --- objc2_id/README.md | 2 +- objc2_id/src/id.rs | 53 +++++++++++++++++++++++++++++++-------------- objc2_id/src/lib.rs | 4 ++-- 3 files changed, 40 insertions(+), 19 deletions(-) diff --git a/objc2_id/README.md b/objc2_id/README.md index 91c77a85f..1ba8efc60 100644 --- a/objc2_id/README.md +++ b/objc2_id/README.md @@ -28,7 +28,7 @@ let obj: Id = unsafe { // obj will be released when it goes out of scope // share the object so we can clone it -let obj = obj.share(); +let obj: Id<_, Shared> = obj.into(); let another_ref = obj.clone(); // dropping our other reference will decrement the retain count drop(another_ref); diff --git a/objc2_id/src/id.rs b/objc2_id/src/id.rs index 29cb1885a..97338e408 100644 --- a/objc2_id/src/id.rs +++ b/objc2_id/src/id.rs @@ -39,9 +39,9 @@ impl Ownership for Shared {} /// may be other references to the object, since a shared [`Id`] can be cloned /// to provide exactly that. /// -/// An owned [`Id`] can be safely "downgraded", that is, turned into to a -/// shared [`Id`] using [`Id::share`], but there is no way to do the opposite -/// safely. +/// An [`Id`] can be safely "downgraded", that is, turned into to a +/// `Id` using `From`/`Into`. The opposite is not safely possible, +/// but the unsafe option [`Id::from_shared`] is provided. /// /// `Option>` is guaranteed to have the same size as a pointer to the /// object. @@ -66,11 +66,16 @@ impl Ownership for Shared {} /// # Examples /// /// ```no_run -/// let owned: Id; -/// let mut_ref: &mut T = *owned; +/// # use objc2::{class, msg_send}; +/// # use objc2::runtime::Object; +/// # use objc2_id::{Id, Owned, Shared}; +/// # type T = Object; +/// let mut owned: Id; +/// # owned = unsafe { Id::from_retained_ptr(msg_send![class!(NSObject), new]) }; +/// let mut_ref: &mut T = &mut *owned; /// // Do something with `&mut T` here /// -/// let shared: Id = owned.share(); +/// let shared: Id = owned.into(); /// let cloned: Id = shared.clone(); /// // Do something with `&T` here /// ``` @@ -149,15 +154,31 @@ where } } -// TODO: Change this to be a [`From`] implementation. -impl Id -where - T: Message, -{ - /// Downgrade an owned [`Id`] to a [`ShareId`], allowing it to be cloned. - pub fn share(self) -> ShareId { - let ptr = ManuallyDrop::new(self).ptr; - unsafe { Id::new(ptr) } +impl Id { + /// Promote a shared [`Id`] to an owned one, allowing it to be mutated. + /// + /// # Safety + /// + /// The caller must ensure that there are no other pointers to the same + /// object (which also means that the given [`Id`] should have a retain + /// count of exactly 1 all cases, except when autoreleases are involved). + #[inline] + pub unsafe fn from_shared(obj: Id) -> Self { + // Note: We can't debug_assert retainCount because of autoreleases + let ptr = ManuallyDrop::new(obj).ptr; + // SAFETY: The pointer is valid + // Ownership rules are upheld by the caller + >::new(ptr) + } +} + +impl From> for Id { + /// Downgrade from an owned to a shared [`Id`], allowing it to be cloned. + #[inline] + fn from(obj: Id) -> Self { + let ptr = ManuallyDrop::new(obj).ptr; + // SAFETY: The pointer is valid, and ownership is simply decreased + unsafe { >::new(ptr) } } } @@ -365,7 +386,7 @@ mod tests { }; assert!(retain_count(&obj) == 1); - let obj = obj.share(); + let obj: Id<_, Shared> = obj.into(); assert!(retain_count(&obj) == 1); let cloned = obj.clone(); diff --git a/objc2_id/src/lib.rs b/objc2_id/src/lib.rs index 68ec9272d..08ac28807 100644 --- a/objc2_id/src/lib.rs +++ b/objc2_id/src/lib.rs @@ -14,7 +14,7 @@ Weak references may be created using the [`WeakId`] struct. ```no_run # use objc2::msg_send; use objc2::runtime::{Class, Object}; -use objc2_id::{Id, WeakId}; +use objc2_id::{Id, Shared, WeakId}; let cls = Class::get("NSObject").unwrap(); let obj: Id = unsafe { @@ -23,7 +23,7 @@ let obj: Id = unsafe { // obj will be released when it goes out of scope // share the object so we can clone it -let obj = obj.share(); +let obj: Id<_, Shared> = obj.into(); let another_ref = obj.clone(); // dropping our other reference will decrement the retain count drop(another_ref); From 3801e6a62ae7d222ae50df41aecaaddfa56f7ac8 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Thu, 9 Sep 2021 00:16:42 +0200 Subject: [PATCH 04/16] Rename Id::from_ptr -> Id::retain and Id::from_retained_ptr to Id::new These names are shorter and more pretty clearly signifies the intention. We also now take `NonNull` arguments so that there's less stuff we have to verify. --- objc2_foundation/src/array.rs | 21 ++--- objc2_foundation/src/data.rs | 6 +- objc2_foundation/src/dictionary.rs | 10 +-- objc2_foundation/src/enumerator.rs | 3 +- objc2_foundation/src/object.rs | 10 +-- objc2_foundation/src/string.rs | 7 +- objc2_foundation/src/value.rs | 3 +- objc2_id/README.md | 2 +- objc2_id/src/id.rs | 125 ++++++++++++++++++++--------- objc2_id/src/lib.rs | 2 +- 10 files changed, 118 insertions(+), 71 deletions(-) diff --git a/objc2_foundation/src/array.rs b/objc2_foundation/src/array.rs index f537fdade..b484ec65b 100644 --- a/objc2_foundation/src/array.rs +++ b/objc2_foundation/src/array.rs @@ -3,6 +3,7 @@ use core::cmp::Ordering; use core::ffi::c_void; use core::marker::PhantomData; use core::ops::{Index, Range}; +use core::ptr::NonNull; use objc2::runtime::{Class, Object}; use objc2::{class, msg_send}; @@ -78,7 +79,7 @@ where let obj: *mut A = msg_send![cls, alloc]; let obj: *mut A = msg_send![obj, initWithObjects:refs.as_ptr() count:refs.len()]; - Id::from_retained_ptr(obj) + Id::new(NonNull::new_unchecked(obj)) } pub trait INSArray: INSObject { @@ -148,10 +149,7 @@ pub trait INSArray: INSObject { array .to_vec() .into_iter() - .map(|obj| unsafe { - let obj_ptr: *const Self::Item = obj; - Id::from_ptr(obj_ptr as *mut Self::Item) - }) + .map(|obj| unsafe { Id::retain(obj.into()) }) .collect() } @@ -170,7 +168,7 @@ pub trait INSArray: INSObject { Self: INSArray, { let obj = self.object_at(index); - unsafe { Id::from_ptr(obj as *const _ as *mut Self::Item) } + unsafe { Id::retain(obj.into()) } } fn from_slice(slice: &[ShareId]) -> Id @@ -187,10 +185,7 @@ pub trait INSArray: INSObject { { self.to_vec() .into_iter() - .map(|obj| unsafe { - let obj_ptr: *const Self::Item = obj; - Id::from_ptr(obj_ptr as *mut Self::Item) - }) + .map(|obj| unsafe { Id::retain(obj.into()) }) .collect() } } @@ -276,7 +271,7 @@ pub trait INSMutableArray: INSArray { ) -> Id { let old_obj = unsafe { let obj = self.object_at(index); - Id::from_ptr(obj as *const _ as *mut Self::Item) + Id::retain(obj.into()) }; unsafe { let _: () = msg_send![self, replaceObjectAtIndex:index @@ -288,7 +283,7 @@ pub trait INSMutableArray: INSArray { fn remove_object_at(&mut self, index: usize) -> Id { let obj = unsafe { let obj = self.object_at(index); - Id::from_ptr(obj as *const _ as *mut Self::Item) + Id::retain(obj.into()) }; unsafe { let _: () = msg_send![self, removeObjectAtIndex: index]; @@ -299,7 +294,7 @@ pub trait INSMutableArray: INSArray { fn remove_last_object(&mut self) -> Id { let obj = self .last_object() - .map(|obj| unsafe { Id::from_ptr(obj as *const _ as *mut Self::Item) }); + .map(|obj| unsafe { Id::retain(obj.into()) }); unsafe { let _: () = msg_send![self, removeLastObject]; } diff --git a/objc2_foundation/src/data.rs b/objc2_foundation/src/data.rs index 1e752357c..75c66e104 100644 --- a/objc2_foundation/src/data.rs +++ b/objc2_foundation/src/data.rs @@ -1,8 +1,8 @@ #[cfg(feature = "block")] use alloc::vec::Vec; -use core::ffi::c_void; use core::ops::Range; use core::slice; +use core::{ffi::c_void, ptr::NonNull}; use super::{INSCopying, INSMutableCopying, INSObject, NSRange}; use objc2::msg_send; @@ -37,7 +37,7 @@ pub trait INSData: INSObject { let obj: *mut Self = msg_send![cls, alloc]; let obj: *mut Self = msg_send![obj, initWithBytes:bytes_ptr length:bytes.len()]; - Id::from_retained_ptr(obj) + Id::new(NonNull::new_unchecked(obj)) } } @@ -60,7 +60,7 @@ pub trait INSData: INSObject { length:bytes.len() deallocator:dealloc]; core::mem::forget(bytes); - Id::from_retained_ptr(obj) + Id::new(NonNull::new_unchecked(obj)) } } } diff --git a/objc2_foundation/src/dictionary.rs b/objc2_foundation/src/dictionary.rs index 4ff9f1997..48e2a31eb 100644 --- a/objc2_foundation/src/dictionary.rs +++ b/objc2_foundation/src/dictionary.rs @@ -2,7 +2,7 @@ use alloc::vec::Vec; use core::cmp::min; use core::marker::PhantomData; use core::ops::Index; -use core::ptr; +use core::ptr::{self, NonNull}; use objc2::runtime::Class; use objc2::{class, msg_send}; @@ -21,7 +21,7 @@ where let obj: *mut D = msg_send![obj, initWithObjects:vals.as_ptr() forKeys:keys.as_ptr() count:count]; - Id::from_retained_ptr(obj) + Id::new(NonNull::new_unchecked(obj)) } pub trait INSDictionary: INSObject { @@ -95,8 +95,8 @@ pub trait INSDictionary: INSObject { fn keys_array(&self) -> Id> { unsafe { - let keys: *mut NSSharedArray = msg_send![self, allKeys]; - Id::from_ptr(keys) + let keys = msg_send![self, allKeys]; + Id::retain(NonNull::new_unchecked(keys)) } } @@ -111,7 +111,7 @@ pub trait INSDictionary: INSObject { fn into_values_array(dict: Id) -> Id> { unsafe { let vals = msg_send![dict, allValues]; - Id::from_ptr(vals) + Id::retain(NonNull::new_unchecked(vals)) } } } diff --git a/objc2_foundation/src/enumerator.rs b/objc2_foundation/src/enumerator.rs index 16e80c581..9b21f447f 100644 --- a/objc2_foundation/src/enumerator.rs +++ b/objc2_foundation/src/enumerator.rs @@ -1,6 +1,7 @@ use core::marker::PhantomData; use core::mem; use core::ptr; +use core::ptr::NonNull; use core::slice; use std::os::raw::c_ulong; @@ -30,7 +31,7 @@ where /// ownership. pub unsafe fn from_ptr(ptr: *mut Object) -> NSEnumerator<'a, T> { NSEnumerator { - id: Id::from_ptr(ptr), + id: Id::retain(NonNull::new(ptr).unwrap()), item: PhantomData, } } diff --git a/objc2_foundation/src/object.rs b/objc2_foundation/src/object.rs index 00572ba3a..a1a0bbb8d 100644 --- a/objc2_foundation/src/object.rs +++ b/objc2_foundation/src/object.rs @@ -1,4 +1,5 @@ use core::any::Any; +use core::ptr::NonNull; use objc2::msg_send; use objc2::runtime::{Class, BOOL, NO}; @@ -31,7 +32,8 @@ pub trait INSObject: Any + Sized + Message { fn description(&self) -> ShareId { unsafe { let result: *mut NSString = msg_send![self, description]; - Id::from_ptr(result) + // TODO: Verify that description always returns a non-null string + Id::retain(NonNull::new_unchecked(result)) } } @@ -42,11 +44,7 @@ pub trait INSObject: Any + Sized + Message { fn new() -> Id { let cls = Self::class(); - unsafe { - let obj: *mut Self = msg_send![cls, alloc]; - let obj: *mut Self = msg_send![obj, init]; - Id::from_retained_ptr(obj) - } + unsafe { Id::new(msg_send![cls, new]) } } } diff --git a/objc2_foundation/src/string.rs b/objc2_foundation/src/string.rs index 2b67df022..535947007 100644 --- a/objc2_foundation/src/string.rs +++ b/objc2_foundation/src/string.rs @@ -1,5 +1,6 @@ use core::ffi::c_void; use core::fmt; +use core::ptr::NonNull; use core::slice; use core::str; use std::os::raw::c_char; @@ -15,7 +16,7 @@ pub trait INSCopying: INSObject { fn copy(&self) -> ShareId { unsafe { let obj: *mut Self::Output = msg_send![self, copy]; - Id::from_retained_ptr(obj) + Id::new(NonNull::new_unchecked(obj)) } } } @@ -26,7 +27,7 @@ pub trait INSMutableCopying: INSObject { fn mutable_copy(&self) -> Id { unsafe { let obj: *mut Self::Output = msg_send![self, mutableCopy]; - Id::from_retained_ptr(obj) + Id::new(NonNull::new_unchecked(obj)) } } } @@ -65,7 +66,7 @@ pub trait INSString: INSObject { let obj: *mut Self = msg_send![obj, initWithBytes:bytes length:string.len() encoding:UTF8_ENCODING]; - Id::from_retained_ptr(obj) + Id::new(NonNull::new_unchecked(obj)) } } } diff --git a/objc2_foundation/src/value.rs b/objc2_foundation/src/value.rs index 1cb37a8cc..233362f9b 100644 --- a/objc2_foundation/src/value.rs +++ b/objc2_foundation/src/value.rs @@ -3,6 +3,7 @@ use core::any::Any; use core::ffi::c_void; use core::marker::PhantomData; use core::mem::MaybeUninit; +use core::ptr::NonNull; use core::str; use std::ffi::{CStr, CString}; use std::os::raw::c_char; @@ -44,7 +45,7 @@ pub trait INSValue: INSObject { let obj: *mut Self = msg_send![cls, alloc]; let obj: *mut Self = msg_send![obj, initWithBytes:bytes objCType:encoding.as_ptr()]; - Id::from_retained_ptr(obj) + Id::new(NonNull::new_unchecked(obj)) } } } diff --git a/objc2_id/README.md b/objc2_id/README.md index 1ba8efc60..43ce55cff 100644 --- a/objc2_id/README.md +++ b/objc2_id/README.md @@ -23,7 +23,7 @@ use objc2_id::{Id, WeakId}; let cls = Class::get("NSObject").unwrap(); let obj: Id = unsafe { - Id::from_retained_ptr(msg_send![cls, new]) + Id::new(msg_send![cls, new]) }; // obj will be released when it goes out of scope diff --git a/objc2_id/src/id.rs b/objc2_id/src/id.rs index 97338e408..36d469d3a 100644 --- a/objc2_id/src/id.rs +++ b/objc2_id/src/id.rs @@ -71,7 +71,7 @@ impl Ownership for Shared {} /// # use objc2_id::{Id, Owned, Shared}; /// # type T = Object; /// let mut owned: Id; -/// # owned = unsafe { Id::from_retained_ptr(msg_send![class!(NSObject), new]) }; +/// # owned = unsafe { Id::new(msg_send![class!(NSObject), new]) }; /// let mut_ref: &mut T = &mut *owned; /// // Do something with `&mut T` here /// @@ -106,51 +106,101 @@ where T: Message, O: Ownership, { - unsafe fn new(ptr: NonNull) -> Id { - Id { - ptr, - item: PhantomData, - own: PhantomData, - } - } - - /// Constructs an [`Id`] from a pointer to an unretained object and - /// retains it. + /// 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 + /// from somewhere else, usually Objective-C methods like `init`, `alloc`, + /// `new`, `copy`, or methods with the `ns_returns_retained` attribute. /// - /// # Panics + /// Since most of the above methods create new objects, and you therefore + /// hold unique access to the object, you would often set the ownership to + /// be [`Owned`]. /// - /// Panics if the pointer is null. + /// But some immutable objects (like `NSString`) don't always return + /// unique references, so in those case you would use [`Shared`]. /// /// # Safety /// - /// The pointer must be to a valid object and the caller must ensure the - /// ownership is correct. - #[doc(alias = "objc_retain")] - pub unsafe fn from_ptr(ptr: *mut T) -> Id { - let nonnull = NonNull::new(ptr).expect("Attempted to construct an Id from a null pointer"); - let ptr: *mut T = objc2_sys::objc_retain(nonnull.as_ptr() as *mut _) as *mut _; - debug_assert_eq!( + /// The caller must ensure the given object has +1 retain count, and that + /// the object pointer otherwise follows the same safety requirements as + /// in [`Id::retain`]. + /// + /// # Example + /// + /// ```no_run + /// # use objc2::{class, msg_send}; + /// # use objc2::runtime::{Class, Object}; + /// # use objc2_id::{Id, Owned}; + /// let cls: &Class; + /// # let cls = class!(NSObject); + /// let obj: &mut Object = unsafe { msg_send![cls, alloc] }; + /// let obj: Id = unsafe { Id::new(msg_send![obj, init]) }; + /// // Or in this case simply just: + /// let obj: Id = unsafe { Id::new(msg_send![cls, new]) }; + /// ``` + /// + /// ```no_run + /// # use objc2::{class, msg_send}; + /// # use objc2::runtime::Object; + /// # use objc2_id::{Id, Shared}; + /// # type NSString = Object; + /// let cls = class!(NSString); + /// // NSString is immutable, so don't create an owned reference to it + /// let obj: Id = unsafe { Id::new(msg_send![cls, new]) }; + /// ``` + #[inline] + // Note: We don't take a reference as a parameter since it would be too + // easy to accidentally create two aliasing mutable references. + pub unsafe fn new(ptr: NonNull) -> Id { + // SAFETY: Upheld by the caller + Id { ptr, - nonnull.as_ptr(), - "objc_retain did not return the same pointer" - ); - Id::new(NonNull::new_unchecked(ptr)) + item: PhantomData, + own: PhantomData, + } } - /// Constructs an [`Id`] from a pointer to a retained object; this won't - /// retain the pointer, so the caller must ensure the object has a +1 - /// retain count. + /// Retains the given object pointer. /// - /// # Panics + /// This is useful when you have been given a pointer to an object from + /// some API, and you would like to ensure that the object stays around + /// so that you can work with it. /// - /// Panics if the pointer is null. + /// This is rarely used to construct owned [`Id`]s, see [`Id::new`] for + /// that. /// /// # Safety /// - /// The pointer must be to a valid object and the caller must ensure the - /// ownership is correct. - pub unsafe fn from_retained_ptr(ptr: *mut T) -> Id { - Id::new(NonNull::new(ptr).expect("Attempted to construct an Id from a null pointer")) + /// The caller must ensure that the ownership is correct; that is, there + /// must be no [`Owned`] pointers or mutable references to the same + /// object, and when creating owned [`Id`]s, there must be no other + /// pointers or references to the object. + /// + /// Additionally, the pointer must be valid as a reference (aligned, + /// dereferencable and initialized, see the [`std::ptr`] module for more + /// information). + // + // This would be illegal: + // ```no_run + // let owned: Id; + // // Lifetime information is discarded + // let retained: Id = unsafe { Id::retain(&*owned) }; + // // Which means we can still mutate `Owned`: + // let x: &mut T = &mut *owned; + // // While we have an immutable reference + // let y: &T = &*retained; + // ``` + #[doc(alias = "objc_retain")] + #[cfg_attr(debug_assertions, inline)] + pub unsafe fn retain(ptr: NonNull) -> Id { + let ptr = ptr.as_ptr() as *mut objc2_sys::objc_object; + // SAFETY: The caller upholds that the pointer is valid + let res = objc2_sys::objc_retain(ptr); + debug_assert_eq!(res, ptr, "objc_retain did not return the same pointer"); + // SAFETY: Non-null upheld by the caller, and `objc_retain` always + // returns the same pointer, see: + // https://clang.llvm.org/docs/AutomaticReferenceCounting.html#arc-runtime-objc-retain + Id::new(NonNull::new_unchecked(res as *mut T)) } } @@ -193,8 +243,8 @@ where #[doc(alias = "retain")] #[inline] fn clone(&self) -> ShareId { - // SAFETY: We already know the `ptr` is valid - unsafe { Id::from_ptr(self.ptr.as_ptr()) } + // SAFETY: The pointer is valid + unsafe { Id::retain(self.ptr) } } } @@ -343,6 +393,7 @@ unsafe impl Send for WeakId {} #[cfg(test)] mod tests { use core::mem::size_of; + use core::ptr::NonNull; use super::{Id, Owned, ShareId, Shared, WeakId}; use objc2::runtime::Object; @@ -382,7 +433,7 @@ mod tests { let obj: Id = unsafe { let obj: *mut Object = msg_send![cls, alloc]; let obj: *mut Object = msg_send![obj, init]; - Id::from_retained_ptr(obj) + Id::new(NonNull::new_unchecked(obj)) }; assert!(retain_count(&obj) == 1); @@ -403,7 +454,7 @@ mod tests { let obj: ShareId = unsafe { let obj: *mut Object = msg_send![cls, alloc]; let obj: *mut Object = msg_send![obj, init]; - Id::from_retained_ptr(obj) + Id::new(NonNull::new_unchecked(obj)) }; let weak = WeakId::new(&obj); diff --git a/objc2_id/src/lib.rs b/objc2_id/src/lib.rs index 08ac28807..af2bee862 100644 --- a/objc2_id/src/lib.rs +++ b/objc2_id/src/lib.rs @@ -18,7 +18,7 @@ use objc2_id::{Id, Shared, WeakId}; let cls = Class::get("NSObject").unwrap(); let obj: Id = unsafe { - Id::from_retained_ptr(msg_send![cls, new]) + Id::new(msg_send![cls, new]) }; // obj will be released when it goes out of scope From c0e70be4d969fc506aeb3a09ca05eb692666e648 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Wed, 8 Sep 2021 22:39:28 +0200 Subject: [PATCH 05/16] Add Id::autorelease (both mutable and immutable variants) --- objc2_id/src/id.rs | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/objc2_id/src/id.rs b/objc2_id/src/id.rs index 36d469d3a..1ba0f37d8 100644 --- a/objc2_id/src/id.rs +++ b/objc2_id/src/id.rs @@ -6,6 +6,7 @@ use core::mem::ManuallyDrop; use core::ops::{Deref, DerefMut}; use core::ptr::NonNull; +use objc2::rc::AutoreleasePool; use objc2::rc::WeakPtr; use objc2::runtime::Object; use objc2::Message; @@ -202,9 +203,36 @@ where // https://clang.llvm.org/docs/AutomaticReferenceCounting.html#arc-runtime-objc-retain Id::new(NonNull::new_unchecked(res as *mut T)) } + + #[cfg_attr(debug_assertions, inline)] + fn autorelease_inner(self) -> *mut T { + let ptr = ManuallyDrop::new(self).ptr.as_ptr() as *mut objc2_sys::objc_object; + // SAFETY: The `ptr` is guaranteed to be valid and have at least one + // retain count. + // And because of the ManuallyDrop, we don't call the Drop + // implementation, so the object won't also be released there. + let res = unsafe { objc2_sys::objc_autorelease(ptr) }; + debug_assert_eq!(res, ptr, "objc_autorelease did not return the same pointer"); + res as *mut T + } } impl Id { + /// Autoreleases the owned [`Id`], returning a mutable reference bound to + /// the pool. + /// + /// The object is not immediately released, but will be when the innermost + /// / current autorelease pool (given as a parameter) is drained. + #[doc(alias = "objc_autorelease")] + #[must_use = "If you don't intend to use the object any more, just drop it as usual"] + #[inline] + pub fn autorelease<'p>(self, pool: &'p AutoreleasePool) -> &'p mut T { + let ptr = self.autorelease_inner(); + // SAFETY: The pointer is valid as a reference, and we've consumed + // the unique access to the `Id` so mutability is safe. + unsafe { pool.ptr_as_mut(ptr) } + } + /// Promote a shared [`Id`] to an owned one, allowing it to be mutated. /// /// # Safety @@ -222,6 +250,22 @@ impl Id { } } +impl Id { + /// Autoreleases the shared [`Id`], returning an aliased reference bound + /// to the pool. + /// + /// The object is not immediately released, but will be when the innermost + /// / current autorelease pool (given as a parameter) is drained. + #[doc(alias = "objc_autorelease")] + #[must_use = "If you don't intend to use the object any more, just drop it as usual"] + #[inline] + pub fn autorelease<'p>(self, pool: &'p AutoreleasePool) -> &'p T { + let ptr = self.autorelease_inner(); + // SAFETY: The pointer is valid as a reference + unsafe { pool.ptr_as_ref(ptr) } + } +} + impl From> for Id { /// Downgrade from an owned to a shared [`Id`], allowing it to be cloned. #[inline] From 3183e75f9feec6fa684b8085068a832160ef6928 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Wed, 8 Sep 2021 23:48:28 +0200 Subject: [PATCH 06/16] Implement more common types on `Id` when `T` implement them - Eq - PartialOrd - Ord - Display - Iterator - DoubleEndedIterator - ExactSizeIterator - FusedIterator - Borrow - BorrowMut - AsRef - AsMut - Unpin --- objc2_id/Cargo.toml | 4 ++ objc2_id/src/id.rs | 168 +++++++++++++++++++++++++++++++++++--------- objc2_id/src/lib.rs | 3 + 3 files changed, 141 insertions(+), 34 deletions(-) diff --git a/objc2_id/Cargo.toml b/objc2_id/Cargo.toml index 3389bb6a4..160c3a221 100644 --- a/objc2_id/Cargo.toml +++ b/objc2_id/Cargo.toml @@ -19,3 +19,7 @@ license = "MIT" [dependencies] objc2 = { path = "../objc2" } objc2_sys = { path = "../objc2_sys" } + +[features] +default = [] # TODO: Add alloc? +alloc = [] diff --git a/objc2_id/src/id.rs b/objc2_id/src/id.rs index 1ba0f37d8..c8bfe97d1 100644 --- a/objc2_id/src/id.rs +++ b/objc2_id/src/id.rs @@ -1,6 +1,9 @@ +#[cfg(feature = "alloc")] +use alloc::borrow; use core::any::Any; use core::fmt; use core::hash; +use core::iter::FusedIterator; use core::marker::PhantomData; use core::mem::ManuallyDrop; use core::ops::{Deref, DerefMut}; @@ -102,11 +105,7 @@ pub struct Id { } // TODO: Maybe make most of these functions "associated" functions instead? -impl Id -where - T: Message, - O: Ownership, -{ +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 @@ -276,10 +275,7 @@ impl From> for Id { } } -impl Clone for Id -where - T: Message, -{ +impl Clone for Id { /// Makes a clone of the shared object. /// /// This increases the object's reference count. @@ -345,46 +341,82 @@ unsafe impl Sync for Id {} impl Deref for Id { type Target = T; + /// Obtain an immutable reference to the object. fn deref(&self) -> &T { + // SAFETY: The pointer's validity is verified when the type is created unsafe { self.ptr.as_ref() } } } impl DerefMut for Id { + /// Obtain a mutable reference to the object. fn deref_mut(&mut self) -> &mut T { + // SAFETY: The pointer's validity is verified when the type is created + // Additionally, the owned `Id` is the unique owner of the object, so + // mutability is safe. unsafe { self.ptr.as_mut() } } } -impl PartialEq for Id -where - T: PartialEq, -{ - fn eq(&self, other: &Id) -> bool { - self.deref() == other.deref() +impl PartialEq for Id { + #[inline] + fn eq(&self, other: &Self) -> bool { + (**self).eq(&**other) + } + + #[inline] + fn ne(&self, other: &Self) -> bool { + (**self).ne(&**other) + } +} + +impl Eq for Id {} + +impl PartialOrd for Id { + #[inline] + fn partial_cmp(&self, other: &Self) -> Option { + (**self).partial_cmp(&**other) + } + #[inline] + fn lt(&self, other: &Self) -> bool { + (**self).lt(&**other) + } + #[inline] + fn le(&self, other: &Self) -> bool { + (**self).le(&**other) + } + #[inline] + fn ge(&self, other: &Self) -> bool { + (**self).ge(&**other) + } + #[inline] + fn gt(&self, other: &Self) -> bool { + (**self).gt(&**other) } } -impl Eq for Id where T: Eq {} +impl Ord for Id { + #[inline] + fn cmp(&self, other: &Self) -> core::cmp::Ordering { + (**self).cmp(&**other) + } +} -impl hash::Hash for Id -where - T: hash::Hash, -{ - fn hash(&self, state: &mut H) - where - H: hash::Hasher, - { - self.deref().hash(state) +impl hash::Hash for Id { + fn hash(&self, state: &mut H) { + (**self).hash(state) } } -impl fmt::Debug for Id -where - T: fmt::Debug, -{ +impl fmt::Display for Id { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.deref().fmt(f) + (**self).fmt(f) + } +} + +impl fmt::Debug for Id { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + (**self).fmt(f) } } @@ -394,6 +426,77 @@ impl fmt::Pointer for Id { } } +impl Iterator for Id { + type Item = I::Item; + fn next(&mut self) -> Option { + (**self).next() + } + fn size_hint(&self) -> (usize, Option) { + (**self).size_hint() + } + fn nth(&mut self, n: usize) -> Option { + (**self).nth(n) + } +} + +impl DoubleEndedIterator for Id { + fn next_back(&mut self) -> Option { + (**self).next_back() + } + fn nth_back(&mut self, n: usize) -> Option { + (**self).nth_back(n) + } +} + +impl ExactSizeIterator for Id { + fn len(&self) -> usize { + (**self).len() + } +} + +impl FusedIterator for Id {} + +#[cfg(feature = "alloc")] +impl borrow::Borrow for Id { + fn borrow(&self) -> &T { + &**self + } +} + +#[cfg(feature = "alloc")] +impl borrow::BorrowMut for Id { + fn borrow_mut(&mut self) -> &mut T { + &mut **self + } +} + +impl AsRef for Id { + fn as_ref(&self) -> &T { + &**self + } +} + +impl AsMut for Id { + fn as_mut(&mut self) -> &mut T { + &mut **self + } +} + +// impl Future for Id { +// type Output = F::Output; +// fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { +// F::poll(Pin::new(&mut *self), cx) +// } +// } + +// This is valid without `T: Unpin` because we don't implement any projection. +// +// See https://doc.rust-lang.org/1.54.0/src/alloc/boxed.rs.html#1652-1675 +// and the `Arc` implementation. +impl Unpin for Id {} + +// TODO: When stabilized impl Fn traits & CoerceUnsized + /// A convenient alias for a shared [`Id`]. pub type ShareId = Id; @@ -404,10 +507,7 @@ pub struct WeakId { item: PhantomData, } -impl WeakId -where - T: Message, -{ +impl WeakId { /// Construct a new [`WeakId`] referencing the given [`ShareId`]. pub fn new(obj: &ShareId) -> WeakId { // SAFETY: The pointer is valid diff --git a/objc2_id/src/lib.rs b/objc2_id/src/lib.rs index af2bee862..36cf19843 100644 --- a/objc2_id/src/lib.rs +++ b/objc2_id/src/lib.rs @@ -41,6 +41,9 @@ assert!(weak.load().is_none()); // Update in Cargo.toml as well. #![doc(html_root_url = "https://docs.rs/objc2_id/0.1.1")] +#[cfg(alloc)] +extern crate alloc; + pub use id::{Id, Owned, Ownership, ShareId, Shared, WeakId}; mod id; From 47e4749d213aaf02d02048979e87fb0b12fd539e Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Thu, 9 Sep 2021 11:09:37 +0200 Subject: [PATCH 07/16] Construct WeakId manually Because I want to get rid of objc2::rc::WeakPtr. WeakId can now also use the null-pointer optimization, and it has a Default implementation. --- objc2_id/Cargo.toml | 4 -- objc2_id/src/id.rs | 121 +++++++++++++++++++++++++++++++++++++------- objc2_id/src/lib.rs | 1 - 3 files changed, 104 insertions(+), 22 deletions(-) diff --git a/objc2_id/Cargo.toml b/objc2_id/Cargo.toml index 160c3a221..3389bb6a4 100644 --- a/objc2_id/Cargo.toml +++ b/objc2_id/Cargo.toml @@ -19,7 +19,3 @@ license = "MIT" [dependencies] objc2 = { path = "../objc2" } objc2_sys = { path = "../objc2_sys" } - -[features] -default = [] # TODO: Add alloc? -alloc = [] diff --git a/objc2_id/src/id.rs b/objc2_id/src/id.rs index c8bfe97d1..e40adf124 100644 --- a/objc2_id/src/id.rs +++ b/objc2_id/src/id.rs @@ -1,17 +1,17 @@ -#[cfg(feature = "alloc")] use alloc::borrow; +use alloc::boxed::Box; use core::any::Any; +use core::cell::UnsafeCell; use core::fmt; use core::hash; use core::iter::FusedIterator; use core::marker::PhantomData; use core::mem::ManuallyDrop; use core::ops::{Deref, DerefMut}; +use core::ptr; use core::ptr::NonNull; use objc2::rc::AutoreleasePool; -use objc2::rc::WeakPtr; -use objc2::runtime::Object; use objc2::Message; /// A type used to mark that a struct owns the object(s) it contains, @@ -60,6 +60,9 @@ impl Ownership for Shared {} /// a reference-counting pointer that, when cloned, increases the reference /// count. /// +/// [`Box`]: alloc::boxed::Box +/// [`Arc`]: alloc::sync::Arc +/// /// # Caveats /// /// If the inner type implements [`Drop`], that implementation will not be @@ -179,6 +182,8 @@ impl Id { /// Additionally, the pointer must be valid as a reference (aligned, /// dereferencable and initialized, see the [`std::ptr`] module for more /// information). + /// + /// [`std::ptr`]: core::ptr // // This would be illegal: // ```no_run @@ -456,14 +461,12 @@ impl ExactSizeIterator for Id { impl FusedIterator for Id {} -#[cfg(feature = "alloc")] impl borrow::Borrow for Id { fn borrow(&self) -> &T { &**self } } -#[cfg(feature = "alloc")] impl borrow::BorrowMut for Id { fn borrow_mut(&mut self) -> &mut T { &mut **self @@ -502,29 +505,91 @@ pub type ShareId = Id; /// A pointer type for a weak reference to an Objective-C reference counted /// object. +/// +/// Allows breaking reference cycles and safely checking whether the object +/// has been deallocated. +#[repr(transparent)] pub struct WeakId { - ptr: WeakPtr, + /// We give the runtime the address to this box, so that it can modify it + /// even if the `WeakId` is moved. + /// + /// Loading may modify the pointer through a shared reference, so we use + /// an UnsafeCell to get a *mut without self being mutable. + inner: Box>, + /// TODO: Variance and dropck item: PhantomData, } impl WeakId { - /// Construct a new [`WeakId`] referencing the given [`ShareId`]. - pub fn new(obj: &ShareId) -> WeakId { - // SAFETY: The pointer is valid - let ptr = unsafe { WeakPtr::new(obj.ptr.as_ptr() as *mut Object) }; - WeakId { - ptr, + /// Construct a new [`WeakId`] referencing the given shared [`Id`]. + #[doc(alias = "objc_initWeak")] + pub fn new(obj: &Id) -> Self { + // Note that taking `&Id` would not be safe since that would + // allow loading an `Id` later on. + + // SAFETY: `obj` is valid + unsafe { Self::new_inner(&**obj as *const T as *mut T) } + } + + /// # Safety + /// + /// The object must be valid or null. + unsafe fn new_inner(obj: *mut T) -> Self { + let inner = Box::new(UnsafeCell::new(ptr::null_mut())); + // SAFETY: `ptr` will never move, and the caller verifies `obj` + objc2_sys::objc_initWeak(inner.get() as _, obj as _); + Self { + inner, item: PhantomData, } } - /// Load a [`ShareId`] from the [`WeakId`] if the object still exists. + /// Load a shared (and retained) [`Id`] if the object still exists. /// /// Returns [`None`] if the object has been deallocated. - pub fn load(&self) -> Option> { - let obj = self.ptr.load(); - let ptr = *ManuallyDrop::new(obj).deref().deref() as *mut T; - NonNull::new(ptr).map(|ptr| unsafe { Id::new(ptr) }) + #[doc(alias = "upgrade")] + #[doc(alias = "objc_loadWeak")] + #[doc(alias = "objc_loadWeakRetained")] + #[inline] + pub fn load(&self) -> Option> { + let ptr: *mut *mut objc2_sys::objc_object = self.inner.get() as _; + let obj = unsafe { objc2_sys::objc_loadWeakRetained(ptr) } as *mut T; + NonNull::new(obj).map(|obj| unsafe { Id::new(obj) }) + } +} + +impl Drop for WeakId { + /// Drops the `WeakId` pointer. + #[doc(alias = "objc_destroyWeak")] + fn drop(&mut self) { + unsafe { + objc2_sys::objc_destroyWeak(self.inner.get() as _); + } + } +} + +impl Clone for WeakId { + /// Makes a clone of the `WeakId` that points to the same object. + #[doc(alias = "objc_copyWeak")] + fn clone(&self) -> Self { + let ptr = Box::new(UnsafeCell::new(ptr::null_mut())); + unsafe { + objc2_sys::objc_copyWeak(ptr.get() as _, self.inner.get() as _); + } + Self { + inner: ptr, + item: PhantomData, + } + } +} + +impl Default for WeakId { + /// Constructs a new `WeakId` that doesn't reference any object. + /// + /// Calling [`Self::load`] on the return value always gives [`None`]. + fn default() -> Self { + // SAFETY: The pointer is null + unsafe { Self::new_inner(ptr::null_mut()) } } } @@ -534,6 +599,16 @@ unsafe impl Sync for WeakId {} /// This implementation follows the same reasoning as `Id`. unsafe impl Send for WeakId {} +// Unsure about the Debug bound on T, see std::sync::Weak +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 {} + #[cfg(test)] mod tests { use core::mem::size_of; @@ -569,6 +644,11 @@ mod tests { size_of::>>(), size_of::<&TestType>() ); + + assert_eq!( + size_of::>>(), + size_of::<*const ()>() + ); } #[test] @@ -611,4 +691,11 @@ mod tests { drop(obj); assert!(weak.load().is_none()); } + + #[test] + fn test_weak_default() { + let weak: WeakId = WeakId::default(); + assert!(weak.load().is_none()); + drop(weak); + } } diff --git a/objc2_id/src/lib.rs b/objc2_id/src/lib.rs index 36cf19843..5f97332b8 100644 --- a/objc2_id/src/lib.rs +++ b/objc2_id/src/lib.rs @@ -41,7 +41,6 @@ assert!(weak.load().is_none()); // Update in Cargo.toml as well. #![doc(html_root_url = "https://docs.rs/objc2_id/0.1.1")] -#[cfg(alloc)] extern crate alloc; pub use id::{Id, Owned, Ownership, ShareId, Shared, WeakId}; From e41d1c1ec82cbba446e6cbf180d7ba3324477048 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Thu, 9 Sep 2021 12:50:01 +0200 Subject: [PATCH 08/16] Make the Ownership trait sealed --- objc2_id/src/id.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/objc2_id/src/id.rs b/objc2_id/src/id.rs index e40adf124..01c1aaa46 100644 --- a/objc2_id/src/id.rs +++ b/objc2_id/src/id.rs @@ -1,6 +1,5 @@ use alloc::borrow; use alloc::boxed::Box; -use core::any::Any; use core::cell::UnsafeCell; use core::fmt; use core::hash; @@ -17,13 +16,25 @@ use objc2::Message; /// A type used to mark that a struct owns the object(s) it contains, /// so it has the sole references to them. pub enum Owned {} + /// A type used to mark that the object(s) a struct contains are shared, /// so there may be other references to them. pub enum Shared {} +mod private { + pub trait Sealed {} + + impl Sealed for super::Owned {} + impl Sealed for super::Shared {} +} + /// A type that marks what type of ownership a struct has over the object(s) /// it contains; specifically, either [`Owned`] or [`Shared`]. -pub trait Ownership: Any {} +/// +/// This trait is sealed and not meant to be implemented outside of the this +/// crate. +pub trait Ownership: private::Sealed + 'static {} + impl Ownership for Owned {} impl Ownership for Shared {} From f41363e808058b8f3b040218540d984893884535 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Thu, 9 Sep 2021 12:27:31 +0200 Subject: [PATCH 09/16] Require the Ownership bound on Id Will allow us to do some nice things when/if GATs stabilize --- objc2_foundation/src/array.rs | 8 ++++---- objc2_foundation/src/macros.rs | 10 +++++----- objc2_id/src/id.rs | 28 ++++++++++++++-------------- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/objc2_foundation/src/array.rs b/objc2_foundation/src/array.rs index b484ec65b..142288e31 100644 --- a/objc2_foundation/src/array.rs +++ b/objc2_foundation/src/array.rs @@ -190,11 +190,11 @@ pub trait INSArray: INSObject { } } -pub struct NSArray { +pub struct NSArray { item: PhantomData>, } -object_impl!(NSArray); +object_impl!(NSArray); impl INSObject for NSArray where @@ -340,11 +340,11 @@ pub trait INSMutableArray: INSArray { } } -pub struct NSMutableArray { +pub struct NSMutableArray { item: PhantomData>, } -object_impl!(NSMutableArray); +object_impl!(NSMutableArray); impl INSObject for NSMutableArray where diff --git a/objc2_foundation/src/macros.rs b/objc2_foundation/src/macros.rs index 09023c3de..8f41ee6c2 100644 --- a/objc2_foundation/src/macros.rs +++ b/objc2_foundation/src/macros.rs @@ -51,13 +51,13 @@ macro_rules! object_impl { ($name:ident) => ( object_impl!($name,); ); - ($name:ident<$($t:ident),+>) => ( - object_impl!($name, $($t),+); + ($name:ident<$($t:ident$(: $b:ident)?),+>) => ( + object_impl!($name, $($t$(: $b)?),+); ); - ($name:ident, $($t:ident),*) => ( - unsafe impl<$($t),*> ::objc2::Message for $name<$($t),*> { } + ($name:ident, $($t:ident$(: $b:ident)?),*) => ( + unsafe impl<$($t$(:($b))?),*> ::objc2::Message for $name<$($t),*> { } - unsafe impl<$($t),*> ::objc2::RefEncode for $name<$($t),*> { + unsafe impl<$($t$(: $b)?),*> ::objc2::RefEncode for $name<$($t),*> { const ENCODING_REF: ::objc2::Encoding<'static> = ::objc2::Encoding::Object; } ); diff --git a/objc2_id/src/id.rs b/objc2_id/src/id.rs index 01c1aaa46..c3aa59b32 100644 --- a/objc2_id/src/id.rs +++ b/objc2_id/src/id.rs @@ -99,7 +99,7 @@ impl Ownership for Shared {} /// ``` #[repr(transparent)] // TODO: Figure out if `Message` bound on `T` would be better here? -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 @@ -311,7 +311,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! @@ -354,7 +354,7 @@ unsafe impl Send for Id {} /// access as having a `T` directly. unsafe impl Sync for Id {} -impl Deref for Id { +impl Deref for Id { type Target = T; /// Obtain an immutable reference to the object. @@ -374,7 +374,7 @@ impl DerefMut for Id { } } -impl PartialEq for Id { +impl PartialEq for Id { #[inline] fn eq(&self, other: &Self) -> bool { (**self).eq(&**other) @@ -386,9 +386,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) @@ -411,32 +411,32 @@ impl PartialOrd for Id { } } -impl Ord for Id { +impl Ord for Id { #[inline] fn cmp(&self, other: &Self) -> core::cmp::Ordering { (**self).cmp(&**other) } } -impl hash::Hash for Id { +impl hash::Hash for Id { fn hash(&self, state: &mut H) { (**self).hash(state) } } -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 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) } @@ -472,7 +472,7 @@ impl ExactSizeIterator for Id { impl FusedIterator for Id {} -impl borrow::Borrow for Id { +impl borrow::Borrow for Id { fn borrow(&self) -> &T { &**self } @@ -484,7 +484,7 @@ impl borrow::BorrowMut for Id { } } -impl AsRef for Id { +impl AsRef for Id { fn as_ref(&self) -> &T { &**self } @@ -507,7 +507,7 @@ impl AsMut 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 {} // TODO: When stabilized impl Fn traits & CoerceUnsized From e6288e9ebd1884e8ab4a97847b96c408e6a802ba Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Thu, 9 Sep 2021 13:10:40 +0200 Subject: [PATCH 10/16] Move Id documentation to README only --- objc2_id/README.md | 13 +++++++------ objc2_id/src/lib.rs | 42 +++++------------------------------------- 2 files changed, 12 insertions(+), 43 deletions(-) diff --git a/objc2_id/README.md b/objc2_id/README.md index 43ce55cff..9e1a4b7b6 100644 --- a/objc2_id/README.md +++ b/objc2_id/README.md @@ -8,18 +8,19 @@ Rust smart pointers for Objective-C reference counting. To ensure that Objective-C objects are retained and released -at the proper times, we can use the Id struct. +at the proper times, we can use the `Id` struct. To enforce aliasing rules, an `Id` can be either owned or shared; if it is owned, meaning the `Id` is the only reference to the object, it can be mutably -dereferenced. An owned `Id` can be downgraded to a ShareId -which can be cloned to allow multiple references. +dereferenced. An owned `Id` can be downgraded to a shared `Id` which can be +cloned to allow multiple references. -Weak references may be created using the WeakId struct. +Weak references may be created using the `WeakId` struct. -``` rust +```rust , no_run +use objc2::msg_send; use objc2::runtime::{Class, Object}; -use objc2_id::{Id, WeakId}; +use objc2_id::{Id, WeakId, Shared}; let cls = Class::get("NSObject").unwrap(); let obj: Id = unsafe { diff --git a/objc2_id/src/lib.rs b/objc2_id/src/lib.rs index 5f97332b8..518ca7ea3 100644 --- a/objc2_id/src/lib.rs +++ b/objc2_id/src/lib.rs @@ -1,40 +1,4 @@ -/*! -Rust smart pointers for Objective-C reference counting. - -To ensure that Objective-C objects are retained and released -at the proper times, we can use the [`Id`] struct. - -To enforce aliasing rules, an [`Id`] can be either owned or shared; if it is -owned, meaning the [`Id`] is the only reference to the object, it can be -mutably dereferenced. An owned [`Id`] can be downgraded to a [`ShareId`] which -can be cloned to allow multiple references. - -Weak references may be created using the [`WeakId`] struct. - -```no_run -# use objc2::msg_send; -use objc2::runtime::{Class, Object}; -use objc2_id::{Id, Shared, WeakId}; - -let cls = Class::get("NSObject").unwrap(); -let obj: Id = unsafe { - Id::new(msg_send![cls, new]) -}; -// obj will be released when it goes out of scope - -// share the object so we can clone it -let obj: Id<_, Shared> = obj.into(); -let another_ref = obj.clone(); -// dropping our other reference will decrement the retain count -drop(another_ref); - -let weak = WeakId::new(&obj); -assert!(weak.load().is_some()); -// After the object is deallocated, our weak pointer returns none -drop(obj); -assert!(weak.load().is_none()); -``` -*/ +//! Smart pointers for Objective-C reference counting. // This crate is, but its dependencies are not #![no_std] @@ -45,6 +9,10 @@ extern crate alloc; pub use id::{Id, Owned, Ownership, ShareId, Shared, WeakId}; +#[cfg(doctest)] +#[doc = include_str!("../README.md")] +extern "C" {} + mod id; // TODO: Remove the need for this hack From b980a1fbbf629828c14b150dc69559cf06d66c7f Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Thu, 9 Sep 2021 14:24:51 +0200 Subject: [PATCH 11/16] Add a few more links to objc2::rc documentation --- objc2/src/rc/autorelease.rs | 7 ------- objc2/src/rc/mod.rs | 14 ++++++++++++-- objc2_id/src/id.rs | 1 + 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/objc2/src/rc/autorelease.rs b/objc2/src/rc/autorelease.rs index e29dd03a5..98a280100 100644 --- a/objc2/src/rc/autorelease.rs +++ b/objc2/src/rc/autorelease.rs @@ -13,13 +13,6 @@ use crate::runtime::{objc_autoreleasePoolPop, objc_autoreleasePoolPush}; /// /// And this is not [`Sync`], since you can only autorelease a reference to a /// pool on the current thread. -/// -/// See [the clang documentation][clang-arc] and [the apple article on memory -/// management][memory-mgmt] for more information on automatic reference -/// counting. -/// -/// [clang-arc]: https://clang.llvm.org/docs/AutomaticReferenceCounting.html -/// [memory-mgmt]: https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/MemoryMgmt/Articles/MemoryMgmt.html pub struct AutoreleasePool { context: *mut c_void, } diff --git a/objc2/src/rc/mod.rs b/objc2/src/rc/mod.rs index 317a30c42..bd5fde9e8 100644 --- a/objc2/src/rc/mod.rs +++ b/objc2/src/rc/mod.rs @@ -10,8 +10,18 @@ and safely fails if the object has been deallocated. These utilities are not intended to provide a fully safe interface, but can be useful when writing higher-level Rust wrappers for Objective-C code. -For more information on Objective-C's reference counting, see Apple's documentation: - +See [the clang documentation][clang-arc] and [the Apple article on memory +management][mem-mgmt] (similar document exists [for Core Foundation][mem-cf]) +for more information on automatic and manual reference counting. + +It can also be useful to [enable Malloc Debugging][mem-debug] if you're trying +to figure out if/where your application has memory errors and leaks. + + +[clang-arc]: https://clang.llvm.org/docs/AutomaticReferenceCounting.html +[mem-mgmt]: https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/MemoryMgmt/Articles/MemoryMgmt.html +[mem-cf]: https://developer.apple.com/library/archive/documentation/CoreFoundation/Conceptual/CFMemoryMgmt/CFMemoryMgmt.html +[mem-debug]: https://developer.apple.com/library/archive/documentation/Performance/Conceptual/ManagingMemory/Articles/MallocDebug.html # Example diff --git a/objc2_id/src/id.rs b/objc2_id/src/id.rs index c3aa59b32..b0898d207 100644 --- a/objc2_id/src/id.rs +++ b/objc2_id/src/id.rs @@ -99,6 +99,7 @@ impl Ownership for Shared {} /// ``` #[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 pub struct Id { /// A pointer to the contained object. The pointer is always retained. /// From 0a6e8b698b6c3c5a88e97526c811e656b225e67c Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Thu, 9 Sep 2021 14:44:52 +0200 Subject: [PATCH 12/16] Merge objc2_id into objc2 Id is such a vital requirement for applications that there's no need to make it a "second class citizen". If we really feel it's too much of an experiment, we can always feature-flag it later. Also, there's might be traits I'd like to implement for e.g. Option that is not possible if it lives in it's own crate! --- Cargo.toml | 1 - objc2/src/macros.rs | 4 +- {objc2_id/src => objc2/src/rc}/id.rs | 44 +++++++---- objc2/src/rc/mod.rs | 110 +++++++++++++++------------ objc2_foundation/Cargo.toml | 1 - objc2_foundation/src/array.rs | 4 +- objc2_foundation/src/data.rs | 2 +- objc2_foundation/src/dictionary.rs | 4 +- objc2_foundation/src/enumerator.rs | 2 +- objc2_foundation/src/object.rs | 2 +- objc2_foundation/src/string.rs | 2 +- objc2_foundation/src/value.rs | 2 +- objc2_id/Cargo.toml | 21 ----- objc2_id/README.md | 42 ---------- objc2_id/src/lib.rs | 36 --------- 15 files changed, 104 insertions(+), 173 deletions(-) rename {objc2_id/src => objc2/src/rc}/id.rs (96%) delete mode 100644 objc2_id/Cargo.toml delete mode 100644 objc2_id/README.md delete mode 100644 objc2_id/src/lib.rs diff --git a/Cargo.toml b/Cargo.toml index b958ec885..cdd936995 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,7 +6,6 @@ members = [ "objc2_exception", "objc2_foundation", "objc2_foundation_derive", - "objc2_id", "objc2_sys", "objc2_test_utils", ] diff --git a/objc2/src/macros.rs b/objc2/src/macros.rs index 3be5291cc..fb9214d44 100644 --- a/objc2/src/macros.rs +++ b/objc2/src/macros.rs @@ -66,13 +66,15 @@ macro_rules! sel { Sends a message to an object. The first argument can be any type that dereferences to a type that implements -[`Message`], like a reference, a pointer, or an `objc2_id::Id` to an object. +[`Message`], like a reference, a pointer, or an [`rc::Id`] to an +object. The syntax is similar to the message syntax in Objective-C. Variadic arguments are not currently supported. [`Message`]: crate::Message +[`rc::Id`]: crate::rc::Id # Panics diff --git a/objc2_id/src/id.rs b/objc2/src/rc/id.rs similarity index 96% rename from objc2_id/src/id.rs rename to objc2/src/rc/id.rs index b0898d207..242cb178a 100644 --- a/objc2_id/src/id.rs +++ b/objc2/src/rc/id.rs @@ -10,8 +10,8 @@ use core::ops::{Deref, DerefMut}; use core::ptr; use core::ptr::NonNull; -use objc2::rc::AutoreleasePool; -use objc2::Message; +use super::AutoreleasePool; +use crate::Message; /// A type used to mark that a struct owns the object(s) it contains, /// so it has the sole references to them. @@ -84,9 +84,33 @@ impl Ownership for Shared {} /// # Examples /// /// ```no_run +/// use objc2::msg_send; +/// use objc2::runtime::{Class, Object}; +/// use objc2::rc::{Id, WeakId, Shared}; +/// +/// let cls = Class::get("NSObject").unwrap(); +/// let obj: Id = unsafe { +/// Id::new(msg_send![cls, new]) +/// }; +/// // obj will be released when it goes out of scope +/// +/// // share the object so we can clone it +/// let obj: Id<_, Shared> = obj.into(); +/// let another_ref = obj.clone(); +/// // dropping our other reference will decrement the retain count +/// drop(another_ref); +/// +/// let weak = WeakId::new(&obj); +/// assert!(weak.load().is_some()); +/// // After the object is deallocated, our weak pointer returns none +/// drop(obj); +/// assert!(weak.load().is_none()); +/// ``` +/// +/// ```no_run /// # use objc2::{class, msg_send}; /// # use objc2::runtime::Object; -/// # use objc2_id::{Id, Owned, Shared}; +/// # use objc2::rc::{Id, Owned, Shared}; /// # type T = Object; /// let mut owned: Id; /// # owned = unsafe { Id::new(msg_send![class!(NSObject), new]) }; @@ -145,7 +169,7 @@ impl Id { /// ```no_run /// # use objc2::{class, msg_send}; /// # use objc2::runtime::{Class, Object}; - /// # use objc2_id::{Id, Owned}; + /// # use objc2::rc::{Id, Owned}; /// let cls: &Class; /// # let cls = class!(NSObject); /// let obj: &mut Object = unsafe { msg_send![cls, alloc] }; @@ -157,7 +181,7 @@ impl Id { /// ```no_run /// # use objc2::{class, msg_send}; /// # use objc2::runtime::Object; - /// # use objc2_id::{Id, Shared}; + /// # use objc2::rc::{Id, Shared}; /// # type NSString = Object; /// let cls = class!(NSString); /// // NSString is immutable, so don't create an owned reference to it @@ -627,14 +651,8 @@ mod tests { use core::ptr::NonNull; use super::{Id, Owned, ShareId, Shared, WeakId}; - use objc2::runtime::Object; - use objc2::{class, msg_send}; - - #[cfg(not(target_vendor = "apple"))] - #[test] - fn ensure_linkage() { - unsafe { crate::get_class_to_force_linkage() }; - } + use crate::runtime::Object; + use crate::{class, msg_send}; fn retain_count(obj: &Object) -> usize { unsafe { msg_send![obj, retainCount] } diff --git a/objc2/src/rc/mod.rs b/objc2/src/rc/mod.rs index bd5fde9e8..a6d8c1205 100644 --- a/objc2/src/rc/mod.rs +++ b/objc2/src/rc/mod.rs @@ -1,58 +1,70 @@ -/*! -Utilities for reference counting Objective-C objects. - -The utilities of the `rc` module provide ARC-like semantics for working with -Objective-C's reference counted objects in Rust. -A `StrongPtr` retains an object and releases the object when dropped. -A `WeakPtr` will not retain the object, but can be upgraded to a `StrongPtr` -and safely fails if the object has been deallocated. - -These utilities are not intended to provide a fully safe interface, but can be -useful when writing higher-level Rust wrappers for Objective-C code. - -See [the clang documentation][clang-arc] and [the Apple article on memory -management][mem-mgmt] (similar document exists [for Core Foundation][mem-cf]) -for more information on automatic and manual reference counting. - -It can also be useful to [enable Malloc Debugging][mem-debug] if you're trying -to figure out if/where your application has memory errors and leaks. - - -[clang-arc]: https://clang.llvm.org/docs/AutomaticReferenceCounting.html -[mem-mgmt]: https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/MemoryMgmt/Articles/MemoryMgmt.html -[mem-cf]: https://developer.apple.com/library/archive/documentation/CoreFoundation/Conceptual/CFMemoryMgmt/CFMemoryMgmt.html -[mem-debug]: https://developer.apple.com/library/archive/documentation/Performance/Conceptual/ManagingMemory/Articles/MallocDebug.html - -# Example - -``` no_run -# use objc2::{class, msg_send}; -# use objc2::rc::{autoreleasepool, StrongPtr}; -// StrongPtr will release the object when dropped -let obj = unsafe { - StrongPtr::new(msg_send![class!(NSObject), new]) -}; - -// Cloning retains the object an additional time -let cloned = obj.clone(); -autoreleasepool(|_| { - // Autorelease consumes the StrongPtr, but won't - // actually release until the end of an autoreleasepool - cloned.autorelease(); -}); - -// Weak references won't retain the object -let weak = obj.weak(); -drop(obj); -assert!(weak.load().is_null()); -``` -*/ +//! Utilities for reference counting Objective-C objects. +//! +//! The utilities of the `rc` module provide ARC-like semantics for working +//! with Objective-C's reference counted objects in Rust. +//! +//! A `StrongPtr` retains an object and releases the object when dropped. +//! A `WeakPtr` will not retain the object, but can be upgraded to a `StrongPtr` +//! and safely fails if the object has been deallocated. +//! +//! These utilities are not intended to provide a fully safe interface, but can be +//! useful when writing higher-level Rust wrappers for Objective-C code. +//! +//! A smart pointer version of this is provided with the `Id` struct. +//! To ensure that Objective-C objects are retained and released +//! at the proper times. +//! +//! To enforce aliasing rules, an `Id` can be either owned or shared; if it is +//! owned, meaning the `Id` is the only reference to the object, it can be +//! mutably dereferenced. An owned `Id` can be downgraded to a shared `Id` +//! which can be cloned to allow multiple references. +//! +//! Weak references may be created using the [`WeakId`] struct. +//! +//! See [the clang documentation][clang-arc] and [the Apple article on memory +//! management][mem-mgmt] (similar document exists [for Core Foundation][mem-cf]) +//! for more information on automatic and manual reference counting. +//! +//! It can also be useful to [enable Malloc Debugging][mem-debug] if you're trying +//! to figure out if/where your application has memory errors and leaks. +//! +//! +//! [clang-arc]: https://clang.llvm.org/docs/AutomaticReferenceCounting.html +//! [mem-mgmt]: https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/MemoryMgmt/Articles/MemoryMgmt.html +//! [mem-cf]: https://developer.apple.com/library/archive/documentation/CoreFoundation/Conceptual/CFMemoryMgmt/CFMemoryMgmt.html +//! [mem-debug]: https://developer.apple.com/library/archive/documentation/Performance/Conceptual/ManagingMemory/Articles/MallocDebug.html +//! +//! # Example +//! +//! ``` no_run +//! # use objc2::{class, msg_send}; +//! # use objc2::rc::{autoreleasepool, StrongPtr}; +//! // StrongPtr will release the object when dropped +//! let obj = unsafe { +//! StrongPtr::new(msg_send![class!(NSObject), new]) +//! }; +//! +//! // Cloning retains the object an additional time +//! let cloned = obj.clone(); +//! autoreleasepool(|_| { +//! // Autorelease consumes the StrongPtr, but won't +//! // actually release until the end of an autoreleasepool +//! cloned.autorelease(); +//! }); +//! +//! // Weak references won't retain the object +//! let weak = obj.weak(); +//! drop(obj); +//! assert!(weak.load().is_null()); +//! ``` mod autorelease; +mod id; mod strong; mod weak; pub use self::autorelease::{autoreleasepool, AutoreleasePool, AutoreleaseSafe}; +pub use self::id::{Id, Owned, Ownership, ShareId, Shared, WeakId}; pub use self::strong::StrongPtr; pub use self::weak::WeakPtr; diff --git a/objc2_foundation/Cargo.toml b/objc2_foundation/Cargo.toml index 939a32315..746a45933 100644 --- a/objc2_foundation/Cargo.toml +++ b/objc2_foundation/Cargo.toml @@ -24,4 +24,3 @@ block = ["objc2_block"] [dependencies] objc2_block = { path = "../objc2_block", optional = true } objc2 = { path = "../objc2" } -objc2_id = { path = "../objc2_id" } diff --git a/objc2_foundation/src/array.rs b/objc2_foundation/src/array.rs index 142288e31..8de60f0e2 100644 --- a/objc2_foundation/src/array.rs +++ b/objc2_foundation/src/array.rs @@ -5,10 +5,10 @@ use core::marker::PhantomData; use core::ops::{Index, Range}; use core::ptr::NonNull; +use objc2::rc::{Id, Owned, Ownership, ShareId, Shared}; use objc2::runtime::{Class, Object}; use objc2::{class, msg_send}; use objc2::{Encode, Encoding}; -use objc2_id::{Id, Owned, Ownership, ShareId, Shared}; use super::{INSCopying, INSFastEnumeration, INSMutableCopying, INSObject, NSEnumerator}; @@ -415,7 +415,7 @@ mod tests { use super::{INSArray, INSMutableArray, NSArray, NSMutableArray}; use crate::{INSObject, INSString, NSObject, NSString}; - use objc2_id::Id; + use objc2::rc::Id; fn sample_array(len: usize) -> Id> { let mut vec = Vec::with_capacity(len); diff --git a/objc2_foundation/src/data.rs b/objc2_foundation/src/data.rs index 75c66e104..14c03dfdc 100644 --- a/objc2_foundation/src/data.rs +++ b/objc2_foundation/src/data.rs @@ -6,9 +6,9 @@ use core::{ffi::c_void, ptr::NonNull}; use super::{INSCopying, INSMutableCopying, INSObject, NSRange}; use objc2::msg_send; +use objc2::rc::Id; #[cfg(feature = "block")] use objc2_block::{Block, ConcreteBlock}; -use objc2_id::Id; pub trait INSData: INSObject { fn len(&self) -> usize { diff --git a/objc2_foundation/src/dictionary.rs b/objc2_foundation/src/dictionary.rs index 48e2a31eb..26498327f 100644 --- a/objc2_foundation/src/dictionary.rs +++ b/objc2_foundation/src/dictionary.rs @@ -4,9 +4,9 @@ use core::marker::PhantomData; use core::ops::Index; use core::ptr::{self, NonNull}; +use objc2::rc::{Id, Owned, Ownership, ShareId}; use objc2::runtime::Class; use objc2::{class, msg_send}; -use objc2_id::{Id, Owned, Ownership, ShareId}; use super::{INSCopying, INSFastEnumeration, INSObject, NSArray, NSEnumerator, NSSharedArray}; @@ -166,7 +166,7 @@ where #[cfg(test)] mod tests { use alloc::vec; - use objc2_id::Id; + use objc2::rc::Id; use super::{INSDictionary, NSDictionary}; use crate::{INSArray, INSObject, INSString, NSObject, NSString}; diff --git a/objc2_foundation/src/enumerator.rs b/objc2_foundation/src/enumerator.rs index 9b21f447f..3cb91e07b 100644 --- a/objc2_foundation/src/enumerator.rs +++ b/objc2_foundation/src/enumerator.rs @@ -5,9 +5,9 @@ use core::ptr::NonNull; use core::slice; use std::os::raw::c_ulong; +use objc2::rc::Id; use objc2::runtime::Object; use objc2::{msg_send, Encode, Encoding, RefEncode}; -use objc2_id::Id; use super::INSObject; diff --git a/objc2_foundation/src/object.rs b/objc2_foundation/src/object.rs index a1a0bbb8d..637dced63 100644 --- a/objc2_foundation/src/object.rs +++ b/objc2_foundation/src/object.rs @@ -2,9 +2,9 @@ use core::any::Any; use core::ptr::NonNull; use objc2::msg_send; +use objc2::rc::{Id, ShareId}; use objc2::runtime::{Class, BOOL, NO}; use objc2::Message; -use objc2_id::{Id, ShareId}; use super::NSString; diff --git a/objc2_foundation/src/string.rs b/objc2_foundation/src/string.rs index 535947007..089e8405d 100644 --- a/objc2_foundation/src/string.rs +++ b/objc2_foundation/src/string.rs @@ -6,7 +6,7 @@ use core::str; use std::os::raw::c_char; use objc2::msg_send; -use objc2_id::{Id, ShareId}; +use objc2::rc::{Id, ShareId}; use super::INSObject; diff --git a/objc2_foundation/src/value.rs b/objc2_foundation/src/value.rs index 233362f9b..fc113543f 100644 --- a/objc2_foundation/src/value.rs +++ b/objc2_foundation/src/value.rs @@ -8,10 +8,10 @@ use core::str; use std::ffi::{CStr, CString}; use std::os::raw::c_char; +use objc2::rc::Id; use objc2::runtime::Class; use objc2::Encode; use objc2::{class, msg_send}; -use objc2_id::Id; use super::{INSCopying, INSObject}; diff --git a/objc2_id/Cargo.toml b/objc2_id/Cargo.toml deleted file mode 100644 index 3389bb6a4..000000000 --- a/objc2_id/Cargo.toml +++ /dev/null @@ -1,21 +0,0 @@ -[package] -name = "objc2_id" -version = "0.1.1" # Remember to update html_root_url in lib.rs -authors = ["Steven Sheldon", "Mads Marquart "] -edition = "2018" - -description = "Smart pointers for Objective-C reference counting." -keywords = ["objective-c", "macos", "ios", "retain", "release"] -categories = [ - "api-bindings", - "development-tools::ffi", - "os::macos-apis", -] -readme = "README.md" -repository = "https://github.com/madsmtm/objc2" -documentation = "https://docs.rs/objc2_id/" -license = "MIT" - -[dependencies] -objc2 = { path = "../objc2" } -objc2_sys = { path = "../objc2_sys" } diff --git a/objc2_id/README.md b/objc2_id/README.md deleted file mode 100644 index 9e1a4b7b6..000000000 --- a/objc2_id/README.md +++ /dev/null @@ -1,42 +0,0 @@ -# `objc2_id` - -[![Latest version](https://badgen.net/crates/v/objc2_id)](https://crates.io/crates/objc2_id) -[![License](https://badgen.net/badge/license/MIT/blue)](../LICENSE.txt) -[![Documentation](https://docs.rs/objc2_id/badge.svg)](https://docs.rs/objc2_id/) -[![CI Status](https://github.com/madsmtm/objc2/workflows/CI/badge.svg)](https://github.com/madsmtm/objc2/actions) - -Rust smart pointers for Objective-C reference counting. - -To ensure that Objective-C objects are retained and released -at the proper times, we can use the `Id` struct. - -To enforce aliasing rules, an `Id` can be either owned or shared; if it is -owned, meaning the `Id` is the only reference to the object, it can be mutably -dereferenced. An owned `Id` can be downgraded to a shared `Id` which can be -cloned to allow multiple references. - -Weak references may be created using the `WeakId` struct. - -```rust , no_run -use objc2::msg_send; -use objc2::runtime::{Class, Object}; -use objc2_id::{Id, WeakId, Shared}; - -let cls = Class::get("NSObject").unwrap(); -let obj: Id = unsafe { - Id::new(msg_send![cls, new]) -}; -// obj will be released when it goes out of scope - -// share the object so we can clone it -let obj: Id<_, Shared> = obj.into(); -let another_ref = obj.clone(); -// dropping our other reference will decrement the retain count -drop(another_ref); - -let weak = WeakId::new(&obj); -assert!(weak.load().is_some()); -// After the object is deallocated, our weak pointer returns none -drop(obj); -assert!(weak.load().is_none()); -``` diff --git a/objc2_id/src/lib.rs b/objc2_id/src/lib.rs deleted file mode 100644 index 518ca7ea3..000000000 --- a/objc2_id/src/lib.rs +++ /dev/null @@ -1,36 +0,0 @@ -//! Smart pointers for Objective-C reference counting. - -// This crate is, but its dependencies are not -#![no_std] -// Update in Cargo.toml as well. -#![doc(html_root_url = "https://docs.rs/objc2_id/0.1.1")] - -extern crate alloc; - -pub use id::{Id, Owned, Ownership, ShareId, Shared, WeakId}; - -#[cfg(doctest)] -#[doc = include_str!("../README.md")] -extern "C" {} - -mod id; - -// TODO: Remove the need for this hack - -#[cfg(not(target_vendor = "apple"))] -use objc2::runtime::Class; - -#[cfg(not(target_vendor = "apple"))] -#[link(name = "gnustep-base", kind = "dylib")] -extern "C" {} - -#[cfg(not(target_vendor = "apple"))] -extern "C" { - static _OBJC_CLASS_NSObject: Class; -} - -#[cfg(not(target_vendor = "apple"))] -#[allow(dead_code)] -unsafe fn get_class_to_force_linkage() -> &'static Class { - &_OBJC_CLASS_NSObject -} From 470a4b74ce44b93f5deefb62e157e206064f3838 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Thu, 9 Sep 2021 14:51:51 +0200 Subject: [PATCH 13/16] Refactor Ownership and WeakId into their own files --- objc2/src/rc/id.rs | 188 +------------------------------------- objc2/src/rc/mod.rs | 32 ++++++- objc2/src/rc/ownership.rs | 24 +++++ objc2/src/rc/weak_id.rs | 152 ++++++++++++++++++++++++++++++ 4 files changed, 209 insertions(+), 187 deletions(-) create mode 100644 objc2/src/rc/ownership.rs create mode 100644 objc2/src/rc/weak_id.rs diff --git a/objc2/src/rc/id.rs b/objc2/src/rc/id.rs index 242cb178a..8b39519c0 100644 --- a/objc2/src/rc/id.rs +++ b/objc2/src/rc/id.rs @@ -1,43 +1,16 @@ use alloc::borrow; -use alloc::boxed::Box; -use core::cell::UnsafeCell; use core::fmt; use core::hash; use core::iter::FusedIterator; use core::marker::PhantomData; use core::mem::ManuallyDrop; use core::ops::{Deref, DerefMut}; -use core::ptr; use core::ptr::NonNull; use super::AutoreleasePool; +use super::{Owned, Ownership, Shared}; use crate::Message; -/// A type used to mark that a struct owns the object(s) it contains, -/// so it has the sole references to them. -pub enum Owned {} - -/// A type used to mark that the object(s) a struct contains are shared, -/// so there may be other references to them. -pub enum Shared {} - -mod private { - pub trait Sealed {} - - impl Sealed for super::Owned {} - impl Sealed for super::Shared {} -} - -/// A type that marks what type of ownership a struct has over the object(s) -/// it contains; specifically, either [`Owned`] or [`Shared`]. -/// -/// This trait is sealed and not meant to be implemented outside of the this -/// crate. -pub trait Ownership: private::Sealed + 'static {} - -impl Ownership for Owned {} -impl Ownership for Shared {} - /// An pointer for Objective-C reference counted objects. /// /// [`Id`] strongly references or "retains" the given object `T`, and @@ -539,118 +512,11 @@ impl Unpin for Id {} /// A convenient alias for a shared [`Id`]. pub type ShareId = Id; -/// A pointer type for a weak reference to an Objective-C reference counted -/// object. -/// -/// Allows breaking reference cycles and safely checking whether the object -/// has been deallocated. -#[repr(transparent)] -pub struct WeakId { - /// We give the runtime the address to this box, so that it can modify it - /// even if the `WeakId` is moved. - /// - /// Loading may modify the pointer through a shared reference, so we use - /// an UnsafeCell to get a *mut without self being mutable. - inner: Box>, - /// TODO: Variance and dropck - item: PhantomData, -} - -impl WeakId { - /// Construct a new [`WeakId`] referencing the given shared [`Id`]. - #[doc(alias = "objc_initWeak")] - pub fn new(obj: &Id) -> Self { - // Note that taking `&Id` would not be safe since that would - // allow loading an `Id` later on. - - // SAFETY: `obj` is valid - unsafe { Self::new_inner(&**obj as *const T as *mut T) } - } - - /// # Safety - /// - /// The object must be valid or null. - unsafe fn new_inner(obj: *mut T) -> Self { - let inner = Box::new(UnsafeCell::new(ptr::null_mut())); - // SAFETY: `ptr` will never move, and the caller verifies `obj` - objc2_sys::objc_initWeak(inner.get() as _, obj as _); - Self { - inner, - item: PhantomData, - } - } - - /// Load a shared (and retained) [`Id`] if the object still exists. - /// - /// Returns [`None`] if the object has been deallocated. - #[doc(alias = "upgrade")] - #[doc(alias = "objc_loadWeak")] - #[doc(alias = "objc_loadWeakRetained")] - #[inline] - pub fn load(&self) -> Option> { - let ptr: *mut *mut objc2_sys::objc_object = self.inner.get() as _; - let obj = unsafe { objc2_sys::objc_loadWeakRetained(ptr) } as *mut T; - NonNull::new(obj).map(|obj| unsafe { Id::new(obj) }) - } -} - -impl Drop for WeakId { - /// Drops the `WeakId` pointer. - #[doc(alias = "objc_destroyWeak")] - fn drop(&mut self) { - unsafe { - objc2_sys::objc_destroyWeak(self.inner.get() as _); - } - } -} - -impl Clone for WeakId { - /// Makes a clone of the `WeakId` that points to the same object. - #[doc(alias = "objc_copyWeak")] - fn clone(&self) -> Self { - let ptr = Box::new(UnsafeCell::new(ptr::null_mut())); - unsafe { - objc2_sys::objc_copyWeak(ptr.get() as _, self.inner.get() as _); - } - Self { - inner: ptr, - item: PhantomData, - } - } -} - -impl Default for WeakId { - /// Constructs a new `WeakId` that doesn't reference any object. - /// - /// Calling [`Self::load`] on the return value always gives [`None`]. - fn default() -> Self { - // SAFETY: The pointer is null - unsafe { Self::new_inner(ptr::null_mut()) } - } -} - -/// This implementation follows the same reasoning as `Id`. -unsafe impl Sync for WeakId {} - -/// This implementation follows the same reasoning as `Id`. -unsafe impl Send for WeakId {} - -// Unsure about the Debug bound on T, see std::sync::Weak -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 {} - #[cfg(test)] mod tests { - use core::mem::size_of; use core::ptr::NonNull; - use super::{Id, Owned, ShareId, Shared, WeakId}; + use super::{Id, Shared}; use crate::runtime::Object; use crate::{class, msg_send}; @@ -658,29 +524,6 @@ mod tests { unsafe { msg_send![obj, retainCount] } } - pub struct TestType { - _data: [u8; 0], // TODO: `UnsafeCell`? - } - - #[test] - fn test_size_of() { - assert_eq!(size_of::>(), size_of::<&TestType>()); - assert_eq!(size_of::>(), size_of::<&TestType>()); - assert_eq!( - size_of::>>(), - size_of::<&TestType>() - ); - assert_eq!( - size_of::>>(), - size_of::<&TestType>() - ); - - assert_eq!( - size_of::>>(), - size_of::<*const ()>() - ); - } - #[test] fn test_clone() { let cls = class!(NSObject); @@ -701,31 +544,4 @@ mod tests { drop(obj); assert!(retain_count(&cloned) == 1); } - - #[test] - fn test_weak() { - let cls = class!(NSObject); - let obj: ShareId = unsafe { - let obj: *mut Object = msg_send![cls, alloc]; - let obj: *mut Object = msg_send![obj, init]; - Id::new(NonNull::new_unchecked(obj)) - }; - - let weak = WeakId::new(&obj); - let strong = weak.load().unwrap(); - let strong_ptr: *const Object = &*strong; - let obj_ptr: *const Object = &*obj; - assert!(strong_ptr == obj_ptr); - drop(strong); - - drop(obj); - assert!(weak.load().is_none()); - } - - #[test] - fn test_weak_default() { - let weak: WeakId = WeakId::default(); - assert!(weak.load().is_none()); - drop(weak); - } } diff --git a/objc2/src/rc/mod.rs b/objc2/src/rc/mod.rs index a6d8c1205..005f324e1 100644 --- a/objc2/src/rc/mod.rs +++ b/objc2/src/rc/mod.rs @@ -60,21 +60,51 @@ mod autorelease; mod id; +mod ownership; mod strong; mod weak; +mod weak_id; pub use self::autorelease::{autoreleasepool, AutoreleasePool, AutoreleaseSafe}; -pub use self::id::{Id, Owned, Ownership, ShareId, Shared, WeakId}; +pub use self::id::{Id, ShareId}; +pub use self::ownership::{Owned, Ownership, Shared}; pub use self::strong::StrongPtr; pub use self::weak::WeakPtr; +pub use self::weak_id::WeakId; // These tests use NSObject, which isn't present for GNUstep #[cfg(all(test, target_vendor = "apple"))] mod tests { + use core::mem::size_of; + use super::autoreleasepool; use super::StrongPtr; + use super::{Id, Owned, Shared, WeakId}; use crate::runtime::Object; + pub struct TestType { + _data: [u8; 0], // TODO: `UnsafeCell`? + } + + #[test] + fn test_size_of() { + assert_eq!(size_of::>(), size_of::<&TestType>()); + assert_eq!(size_of::>(), size_of::<&TestType>()); + assert_eq!( + size_of::>>(), + size_of::<&TestType>() + ); + assert_eq!( + size_of::>>(), + size_of::<&TestType>() + ); + + assert_eq!( + size_of::>>(), + size_of::<*const ()>() + ); + } + #[test] fn test_strong_clone() { fn retain_count(obj: *mut Object) -> usize { diff --git a/objc2/src/rc/ownership.rs b/objc2/src/rc/ownership.rs new file mode 100644 index 000000000..1cb0b8dd4 --- /dev/null +++ b/objc2/src/rc/ownership.rs @@ -0,0 +1,24 @@ +/// A type used to mark that a struct owns the object(s) it contains, +/// so it has the sole references to them. +pub enum Owned {} + +/// A type used to mark that the object(s) a struct contains are shared, +/// so there may be other references to them. +pub enum Shared {} + +mod private { + pub trait Sealed {} + + impl Sealed for super::Owned {} + impl Sealed for super::Shared {} +} + +/// A type that marks what type of ownership a struct has over the object(s) +/// it contains; specifically, either [`Owned`] or [`Shared`]. +/// +/// This trait is sealed and not meant to be implemented outside of the this +/// crate. +pub trait Ownership: private::Sealed + 'static {} + +impl Ownership for Owned {} +impl Ownership for Shared {} diff --git a/objc2/src/rc/weak_id.rs b/objc2/src/rc/weak_id.rs new file mode 100644 index 000000000..90d4cf0c1 --- /dev/null +++ b/objc2/src/rc/weak_id.rs @@ -0,0 +1,152 @@ +use alloc::boxed::Box; +use core::cell::UnsafeCell; +use core::fmt; +use core::marker::PhantomData; +use core::ptr; +use core::ptr::NonNull; + +use super::{Id, Shared}; +use crate::Message; + +/// A pointer type for a weak reference to an Objective-C reference counted +/// object. +/// +/// Allows breaking reference cycles and safely checking whether the object +/// has been deallocated. +#[repr(transparent)] +pub struct WeakId { + /// We give the runtime the address to this box, so that it can modify it + /// even if the `WeakId` is moved. + /// + /// Loading may modify the pointer through a shared reference, so we use + /// an UnsafeCell to get a *mut without self being mutable. + inner: Box>, + /// TODO: Variance and dropck + item: PhantomData, +} + +impl WeakId { + /// Construct a new [`WeakId`] referencing the given shared [`Id`]. + #[doc(alias = "objc_initWeak")] + pub fn new(obj: &Id) -> Self { + // Note that taking `&Id` would not be safe since that would + // allow loading an `Id` later on. + + // SAFETY: `obj` is valid + unsafe { Self::new_inner(&**obj as *const T as *mut T) } + } + + /// # Safety + /// + /// The object must be valid or null. + unsafe fn new_inner(obj: *mut T) -> Self { + let inner = Box::new(UnsafeCell::new(ptr::null_mut())); + // SAFETY: `ptr` will never move, and the caller verifies `obj` + objc2_sys::objc_initWeak(inner.get() as _, obj as _); + Self { + inner, + item: PhantomData, + } + } + + /// Load a shared (and retained) [`Id`] if the object still exists. + /// + /// Returns [`None`] if the object has been deallocated. + #[doc(alias = "upgrade")] + #[doc(alias = "objc_loadWeak")] + #[doc(alias = "objc_loadWeakRetained")] + #[inline] + pub fn load(&self) -> Option> { + let ptr: *mut *mut objc2_sys::objc_object = self.inner.get() as _; + let obj = unsafe { objc2_sys::objc_loadWeakRetained(ptr) } as *mut T; + NonNull::new(obj).map(|obj| unsafe { Id::new(obj) }) + } +} + +impl Drop for WeakId { + /// Drops the `WeakId` pointer. + #[doc(alias = "objc_destroyWeak")] + fn drop(&mut self) { + unsafe { + objc2_sys::objc_destroyWeak(self.inner.get() as _); + } + } +} + +impl Clone for WeakId { + /// Makes a clone of the `WeakId` that points to the same object. + #[doc(alias = "objc_copyWeak")] + fn clone(&self) -> Self { + let ptr = Box::new(UnsafeCell::new(ptr::null_mut())); + unsafe { + objc2_sys::objc_copyWeak(ptr.get() as _, self.inner.get() as _); + } + Self { + inner: ptr, + item: PhantomData, + } + } +} + +impl Default for WeakId { + /// Constructs a new `WeakId` that doesn't reference any object. + /// + /// Calling [`Self::load`] on the return value always gives [`None`]. + fn default() -> Self { + // SAFETY: The pointer is null + unsafe { Self::new_inner(ptr::null_mut()) } + } +} + +/// This implementation follows the same reasoning as `Id`. +unsafe impl Sync for WeakId {} + +/// This implementation follows the same reasoning as `Id`. +unsafe impl Send for WeakId {} + +// Unsure about the Debug bound on T, see std::sync::Weak +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 {} + +#[cfg(test)] +mod tests { + use core::ptr::NonNull; + + use super::WeakId; + use super::{Id, Shared}; + use crate::runtime::Object; + use crate::{class, msg_send}; + + #[test] + fn test_weak() { + let cls = class!(NSObject); + let obj: Id = unsafe { + let obj: *mut Object = msg_send![cls, alloc]; + let obj: *mut Object = msg_send![obj, init]; + Id::new(NonNull::new_unchecked(obj)) + }; + + let weak = WeakId::new(&obj); + let strong = weak.load().unwrap(); + let strong_ptr: *const Object = &*strong; + let obj_ptr: *const Object = &*obj; + assert!(strong_ptr == obj_ptr); + drop(strong); + + drop(obj); + assert!(weak.load().is_none()); + } + + #[test] + fn test_weak_default() { + let weak: WeakId = WeakId::default(); + assert!(weak.load().is_none()); + drop(weak); + } +} From e08e87d210dc8d1a7f77a0bffe7921076b0fc40d Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Thu, 9 Sep 2021 15:14:44 +0200 Subject: [PATCH 14/16] Remove StrongPtr and WeakPtr We want to encourage users to use Id and WeakId, because they provide much stronger guarantees. The other types can in most cases be emulated as: type StrongPtr = Option> type WeakPtr = WeakId And for the cases where they can't, their implementation is pretty simple anyhow, so advanced users can just use objc2_sys to implement the functionality they need themselves. --- objc2/README.md | 26 ++++---- objc2/examples/introspection.rs | 14 +++-- objc2/src/exception.rs | 9 ++- objc2/src/rc/id.rs | 16 +++++ objc2/src/rc/mod.rs | 107 +------------------------------- objc2/src/rc/strong.rs | 77 ----------------------- objc2/src/rc/weak.rs | 54 ---------------- objc2/src/rc/weak_id.rs | 18 +++++- 8 files changed, 64 insertions(+), 257 deletions(-) delete mode 100644 objc2/src/rc/strong.rs delete mode 100644 objc2/src/rc/weak.rs diff --git a/objc2/README.md b/objc2/README.md index 8e41f546c..2e3af7584 100644 --- a/objc2/README.md +++ b/objc2/README.md @@ -29,31 +29,33 @@ unsafe { The utilities of the `rc` module provide ARC-like semantics for working with Objective-C's reference counted objects in Rust. -A `StrongPtr` retains an object and releases the object when dropped. -A `WeakPtr` will not retain the object, but can be upgraded to a `StrongPtr` -and safely fails if the object has been deallocated. + +An `Id` retains an object and releases the object when dropped. +A `WeakId` will not retain the object, but can be upgraded to an `Id` and +safely fails if the object has been deallocated. ```rust , no_run use objc2::{class, msg_send}; -use objc2::rc::{autoreleasepool, StrongPtr}; +use objc2::rc::{autoreleasepool, Id, Shared, WeakId}; +use objc2::runtime::Object; -// StrongPtr will release the object when dropped -let obj = unsafe { - StrongPtr::new(msg_send![class!(NSObject), new]) +// Id will release the object when dropped +let obj: Id = unsafe { + Id::new(msg_send![class!(NSObject), new]) }; // Cloning retains the object an additional time let cloned = obj.clone(); -autoreleasepool(|_| { - // Autorelease consumes the StrongPtr, but won't +autoreleasepool(|pool| { + // Autorelease consumes the Id, but won't // actually release until the end of an autoreleasepool - cloned.autorelease(); + let obj_ref: &Object = cloned.autorelease(pool); }); // Weak references won't retain the object -let weak = obj.weak(); +let weak = WeakId::new(&obj); drop(obj); -assert!(weak.load().is_null()); +assert!(weak.load().is_none()); ``` ## Declaring classes diff --git a/objc2/examples/introspection.rs b/objc2/examples/introspection.rs index 189796928..d4225117e 100644 --- a/objc2/examples/introspection.rs +++ b/objc2/examples/introspection.rs @@ -1,4 +1,6 @@ -use objc2::rc::StrongPtr; +use core::ptr::NonNull; + +use objc2::rc::{Id, Owned}; use objc2::runtime::{Class, Object}; use objc2::{class, msg_send, sel, Encode}; @@ -14,15 +16,15 @@ fn main() { } // Allocate an instance - let obj = unsafe { + let obj: Id = unsafe { let obj: *mut Object = msg_send![cls, alloc]; - let obj: *mut Object = msg_send![obj, init]; - StrongPtr::new(obj) + let obj: NonNull = msg_send![obj, init]; + Id::new(obj) }; println!("NSObject address: {:p}", obj); // Access an ivar of the object - let isa: *const Class = unsafe { *(**obj).get_ivar("isa") }; + let isa: *const Class = unsafe { *obj.get_ivar("isa") }; println!("NSObject isa: {:?}", isa); // Inspect a method of the class @@ -33,6 +35,6 @@ fn main() { assert!(*hash_return == usize::ENCODING); // Invoke a method on the object - let hash: usize = unsafe { msg_send![*obj, hash] }; + let hash: usize = unsafe { msg_send![obj, hash] }; println!("NSObject hash: {}", hash); } diff --git a/objc2/src/exception.rs b/objc2/src/exception.rs index 487dd70dd..50013c70c 100644 --- a/objc2/src/exception.rs +++ b/objc2/src/exception.rs @@ -1,5 +1,8 @@ -use crate::rc::StrongPtr; +use core::ptr::NonNull; + +use crate::rc::Id; use crate::runtime::Object; +use objc2_exception::{r#try, Exception}; // Comment copied from `objc2_exception` @@ -18,6 +21,6 @@ use crate::runtime::Object; /// undefined behaviour until `C-unwind` is stabilized, see [RFC-2945]. /// /// [RFC-2945]: https://rust-lang.github.io/rfcs/2945-c-unwind-abi.html -pub unsafe fn catch_exception(closure: impl FnOnce() -> R) -> Result { - objc2_exception::r#try(closure).map_err(|exception| StrongPtr::new(exception as *mut Object)) +pub unsafe fn catch_exception(closure: impl FnOnce() -> R) -> Result> { + r#try(closure).map_err(|e| Id::new(NonNull::new(e).unwrap())) } diff --git a/objc2/src/rc/id.rs b/objc2/src/rc/id.rs index 8b39519c0..548ba124f 100644 --- a/objc2/src/rc/id.rs +++ b/objc2/src/rc/id.rs @@ -517,6 +517,7 @@ mod tests { use core::ptr::NonNull; use super::{Id, Shared}; + use crate::rc::autoreleasepool; use crate::runtime::Object; use crate::{class, msg_send}; @@ -524,6 +525,21 @@ mod tests { unsafe { msg_send![obj, retainCount] } } + #[test] + fn test_autorelease() { + let obj: Id = unsafe { Id::new(msg_send![class!(NSObject), new]) }; + + let cloned = obj.clone(); + + autoreleasepool(|pool| { + let _ref = obj.autorelease(pool); + assert_eq!(retain_count(&*cloned), 2); + }); + + // make sure that the autoreleased value has been released + assert_eq!(retain_count(&*cloned), 1); + } + #[test] fn test_clone() { let cls = class!(NSObject); diff --git a/objc2/src/rc/mod.rs b/objc2/src/rc/mod.rs index 005f324e1..6c18e7e7d 100644 --- a/objc2/src/rc/mod.rs +++ b/objc2/src/rc/mod.rs @@ -3,16 +3,8 @@ //! The utilities of the `rc` module provide ARC-like semantics for working //! with Objective-C's reference counted objects in Rust. //! -//! A `StrongPtr` retains an object and releases the object when dropped. -//! A `WeakPtr` will not retain the object, but can be upgraded to a `StrongPtr` -//! and safely fails if the object has been deallocated. -//! -//! These utilities are not intended to provide a fully safe interface, but can be -//! useful when writing higher-level Rust wrappers for Objective-C code. -//! -//! A smart pointer version of this is provided with the `Id` struct. -//! To ensure that Objective-C objects are retained and released -//! at the proper times. +//! A smart pointer [`Id`] is provided to ensure that Objective-C objects are +//! retained and released at the proper times. //! //! To enforce aliasing rules, an `Id` can be either owned or shared; if it is //! owned, meaning the `Id` is the only reference to the object, it can be @@ -33,54 +25,22 @@ //! [mem-mgmt]: https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/MemoryMgmt/Articles/MemoryMgmt.html //! [mem-cf]: https://developer.apple.com/library/archive/documentation/CoreFoundation/Conceptual/CFMemoryMgmt/CFMemoryMgmt.html //! [mem-debug]: https://developer.apple.com/library/archive/documentation/Performance/Conceptual/ManagingMemory/Articles/MallocDebug.html -//! -//! # Example -//! -//! ``` no_run -//! # use objc2::{class, msg_send}; -//! # use objc2::rc::{autoreleasepool, StrongPtr}; -//! // StrongPtr will release the object when dropped -//! let obj = unsafe { -//! StrongPtr::new(msg_send![class!(NSObject), new]) -//! }; -//! -//! // Cloning retains the object an additional time -//! let cloned = obj.clone(); -//! autoreleasepool(|_| { -//! // Autorelease consumes the StrongPtr, but won't -//! // actually release until the end of an autoreleasepool -//! cloned.autorelease(); -//! }); -//! -//! // Weak references won't retain the object -//! let weak = obj.weak(); -//! drop(obj); -//! assert!(weak.load().is_null()); -//! ``` mod autorelease; mod id; mod ownership; -mod strong; -mod weak; mod weak_id; pub use self::autorelease::{autoreleasepool, AutoreleasePool, AutoreleaseSafe}; pub use self::id::{Id, ShareId}; pub use self::ownership::{Owned, Ownership, Shared}; -pub use self::strong::StrongPtr; -pub use self::weak::WeakPtr; pub use self::weak_id::WeakId; -// These tests use NSObject, which isn't present for GNUstep -#[cfg(all(test, target_vendor = "apple"))] +#[cfg(test)] mod tests { use core::mem::size_of; - use super::autoreleasepool; - use super::StrongPtr; use super::{Id, Owned, Shared, WeakId}; - use crate::runtime::Object; pub struct TestType { _data: [u8; 0], // TODO: `UnsafeCell`? @@ -104,65 +64,4 @@ mod tests { size_of::<*const ()>() ); } - - #[test] - fn test_strong_clone() { - fn retain_count(obj: *mut Object) -> usize { - unsafe { msg_send![obj, retainCount] } - } - - let obj = unsafe { StrongPtr::new(msg_send![class!(NSObject), new]) }; - assert!(retain_count(*obj) == 1); - - let cloned = obj.clone(); - assert!(retain_count(*cloned) == 2); - assert!(retain_count(*obj) == 2); - - drop(obj); - assert!(retain_count(*cloned) == 1); - } - - #[test] - fn test_weak() { - let obj = unsafe { StrongPtr::new(msg_send![class!(NSObject), new]) }; - let weak = obj.weak(); - - let strong = weak.load(); - assert!(*strong == *obj); - drop(strong); - - drop(obj); - assert!(weak.load().is_null()); - } - - #[test] - fn test_weak_copy() { - let obj = unsafe { StrongPtr::new(msg_send![class!(NSObject), new]) }; - let weak = obj.weak(); - - let weak2 = weak.clone(); - - let strong = weak.load(); - let strong2 = weak2.load(); - assert!(*strong == *obj); - assert!(*strong2 == *obj); - } - - #[test] - fn test_autorelease() { - let obj = unsafe { StrongPtr::new(msg_send![class!(NSObject), new]) }; - - fn retain_count(obj: *mut Object) -> usize { - unsafe { msg_send![obj, retainCount] } - } - let cloned = obj.clone(); - - autoreleasepool(|_| { - obj.autorelease(); - assert!(retain_count(*cloned) == 2); - }); - - // make sure that the autoreleased value has been released - assert!(retain_count(*cloned) == 1); - } } diff --git a/objc2/src/rc/strong.rs b/objc2/src/rc/strong.rs deleted file mode 100644 index eb2264443..000000000 --- a/objc2/src/rc/strong.rs +++ /dev/null @@ -1,77 +0,0 @@ -use core::fmt; -use core::mem; -use core::ops::Deref; - -use super::WeakPtr; -use crate::runtime::{self, Object}; - -/// A pointer that strongly references an object, ensuring it won't be deallocated. -pub struct StrongPtr(*mut Object); - -impl StrongPtr { - /// Constructs a [`StrongPtr`] to a newly created object that already has a - /// +1 retain count. This will not retain the object. - /// When dropped, the object will be released. - /// - /// # Safety - /// - /// The caller must ensure the given object pointer is valid. - pub unsafe fn new(ptr: *mut Object) -> Self { - StrongPtr(ptr) - } - - /// Retains the given object and constructs a [`StrongPtr`] to it. - /// When dropped, the object will be released. - /// - /// # Safety - /// - /// The caller must ensure the given object pointer is valid. - pub unsafe fn retain(ptr: *mut Object) -> Self { - Self(runtime::objc_retain(ptr as _) as _) - } - - /// Autoreleases self, meaning that the object is not immediately released, - /// but will be when the autorelease pool is drained. A pointer to the - /// object is returned, but its validity is no longer ensured. - pub fn autorelease(self) -> *mut Object { - let ptr = self.0; - mem::forget(self); - unsafe { - runtime::objc_autorelease(ptr as _); - } - ptr - } - - /// Returns a [`WeakPtr`] to self. - pub fn weak(&self) -> WeakPtr { - unsafe { WeakPtr::new(self.0) } - } -} - -impl Drop for StrongPtr { - fn drop(&mut self) { - unsafe { - runtime::objc_release(self.0 as _); - } - } -} - -impl Clone for StrongPtr { - fn clone(&self) -> StrongPtr { - unsafe { StrongPtr::retain(self.0) } - } -} - -impl Deref for StrongPtr { - type Target = *mut Object; - - fn deref(&self) -> &*mut Object { - &self.0 - } -} - -impl fmt::Pointer for StrongPtr { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Pointer::fmt(&self.0, f) - } -} diff --git a/objc2/src/rc/weak.rs b/objc2/src/rc/weak.rs deleted file mode 100644 index 448810a2e..000000000 --- a/objc2/src/rc/weak.rs +++ /dev/null @@ -1,54 +0,0 @@ -use alloc::boxed::Box; -use core::cell::UnsafeCell; -use core::ptr; - -use super::StrongPtr; -use crate::runtime::{self, Object}; - -// Our pointer must have the same address even if we are moved, so Box it. -// Although loading the WeakPtr may modify the pointer, it is thread safe, -// so we must use an UnsafeCell to get a *mut without self being mutable. - -/// A pointer that weakly references an object, allowing to safely check -/// whether it has been deallocated. -pub struct WeakPtr(Box>); - -impl WeakPtr { - /// Constructs a [`WeakPtr`] to the given object. - /// - /// # Safety - /// - /// The caller must ensure the given object pointer is valid. - pub unsafe fn new(obj: *mut Object) -> Self { - let ptr = Box::new(UnsafeCell::new(ptr::null_mut())); - runtime::objc_initWeak(ptr.get() as _, obj as _); - WeakPtr(ptr) - } - - /// Loads the object self points to, returning a [`StrongPtr`]. - /// If the object has been deallocated, the returned pointer will be null. - pub fn load(&self) -> StrongPtr { - unsafe { - let ptr = runtime::objc_loadWeakRetained(self.0.get() as _); - StrongPtr::new(ptr as _) - } - } -} - -impl Drop for WeakPtr { - fn drop(&mut self) { - unsafe { - runtime::objc_destroyWeak(self.0.get() as _); - } - } -} - -impl Clone for WeakPtr { - fn clone(&self) -> Self { - let ptr = Box::new(UnsafeCell::new(ptr::null_mut())); - unsafe { - runtime::objc_copyWeak(ptr.get() as _, self.0.get() as _); - } - WeakPtr(ptr) - } -} diff --git a/objc2/src/rc/weak_id.rs b/objc2/src/rc/weak_id.rs index 90d4cf0c1..a115fd914 100644 --- a/objc2/src/rc/weak_id.rs +++ b/objc2/src/rc/weak_id.rs @@ -136,13 +136,29 @@ mod tests { let strong = weak.load().unwrap(); let strong_ptr: *const Object = &*strong; let obj_ptr: *const Object = &*obj; - assert!(strong_ptr == obj_ptr); + assert_eq!(strong_ptr, obj_ptr); drop(strong); drop(obj); assert!(weak.load().is_none()); } + #[test] + fn test_weak_clone() { + let obj: Id = unsafe { Id::new(msg_send![class!(NSObject), new]) }; + let weak = WeakId::new(&obj); + + let weak2 = weak.clone(); + + let strong = weak.load().unwrap(); + let strong2 = weak2.load().unwrap(); + let strong_ptr: *const Object = &*strong; + let strong2_ptr: *const Object = &*strong2; + let obj_ptr: *const Object = &*obj; + assert_eq!(strong_ptr, obj_ptr); + assert_eq!(strong2_ptr, obj_ptr); + } + #[test] fn test_weak_default() { let weak: WeakId = WeakId::default(); From f6b341e4f77f4c5c1aec3f17910e728b7c3b4cf4 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Thu, 9 Sep 2021 15:26:34 +0200 Subject: [PATCH 15/16] Remove ShareId and default ownership for Id This is a usability degradation, but before we know more about actual usage patterns this is the safer option; users will have to explicitly specify the ownership they're expecting. See https://github.com/SSheldon/rust-objc-id/issues/4 --- objc2/src/rc/id.rs | 15 ++++++--------- objc2/src/rc/mod.rs | 2 +- objc2_foundation/src/array.rs | 20 ++++++++++---------- objc2_foundation/src/data.rs | 6 +++--- objc2_foundation/src/dictionary.rs | 18 +++++++++--------- objc2_foundation/src/enumerator.rs | 4 ++-- objc2_foundation/src/object.rs | 6 +++--- objc2_foundation/src/string.rs | 8 ++++---- objc2_foundation/src/value.rs | 4 ++-- 9 files changed, 40 insertions(+), 43 deletions(-) diff --git a/objc2/src/rc/id.rs b/objc2/src/rc/id.rs index 548ba124f..109f69529 100644 --- a/objc2/src/rc/id.rs +++ b/objc2/src/rc/id.rs @@ -59,10 +59,10 @@ use crate::Message; /// ```no_run /// use objc2::msg_send; /// use objc2::runtime::{Class, Object}; -/// use objc2::rc::{Id, WeakId, Shared}; +/// use objc2::rc::{Id, Owned, Shared, WeakId}; /// /// let cls = Class::get("NSObject").unwrap(); -/// let obj: Id = unsafe { +/// let obj: Id = unsafe { /// Id::new(msg_send![cls, new]) /// }; /// // obj will be released when it goes out of scope @@ -97,7 +97,7 @@ 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 -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 @@ -296,7 +296,7 @@ impl Clone for Id { #[doc(alias = "objc_retain")] #[doc(alias = "retain")] #[inline] - fn clone(&self) -> ShareId { + fn clone(&self) -> Self { // SAFETY: The pointer is valid unsafe { Id::retain(self.ptr) } } @@ -509,14 +509,11 @@ impl Unpin for Id {} // TODO: When stabilized impl Fn traits & CoerceUnsized -/// A convenient alias for a shared [`Id`]. -pub type ShareId = Id; - #[cfg(test)] mod tests { use core::ptr::NonNull; - use super::{Id, Shared}; + use super::{Id, Owned, Shared}; use crate::rc::autoreleasepool; use crate::runtime::Object; use crate::{class, msg_send}; @@ -543,7 +540,7 @@ mod tests { #[test] fn test_clone() { let cls = class!(NSObject); - let obj: Id = unsafe { + let obj: Id = unsafe { let obj: *mut Object = msg_send![cls, alloc]; let obj: *mut Object = msg_send![obj, init]; Id::new(NonNull::new_unchecked(obj)) diff --git a/objc2/src/rc/mod.rs b/objc2/src/rc/mod.rs index 6c18e7e7d..da20300dc 100644 --- a/objc2/src/rc/mod.rs +++ b/objc2/src/rc/mod.rs @@ -32,7 +32,7 @@ mod ownership; mod weak_id; pub use self::autorelease::{autoreleasepool, AutoreleasePool, AutoreleaseSafe}; -pub use self::id::{Id, ShareId}; +pub use self::id::Id; pub use self::ownership::{Owned, Ownership, Shared}; pub use self::weak_id::WeakId; diff --git a/objc2_foundation/src/array.rs b/objc2_foundation/src/array.rs index 8de60f0e2..94c15bc21 100644 --- a/objc2_foundation/src/array.rs +++ b/objc2_foundation/src/array.rs @@ -5,7 +5,7 @@ use core::marker::PhantomData; use core::ops::{Index, Range}; use core::ptr::NonNull; -use objc2::rc::{Id, Owned, Ownership, ShareId, Shared}; +use objc2::rc::{Id, Owned, Ownership, Shared}; use objc2::runtime::{Class, Object}; use objc2::{class, msg_send}; use objc2::{Encode, Encoding}; @@ -71,7 +71,7 @@ unsafe impl Encode for NSRange { Encoding::Struct("_NSRange", &[usize::ENCODING, usize::ENCODING]); } -unsafe fn from_refs(refs: &[&A::Item]) -> Id +unsafe fn from_refs(refs: &[&A::Item]) -> Id where A: INSArray, { @@ -126,7 +126,7 @@ pub trait INSArray: INSObject { } } - fn from_vec(vec: Vec>) -> Id { + fn from_vec(vec: Vec>) -> Id { let refs: Vec<&Self::Item> = vec.iter().map(|obj| &**obj).collect(); unsafe { from_refs(&refs) } } @@ -145,7 +145,7 @@ pub trait INSArray: INSObject { self.objects_in_range(0..self.count()) } - fn into_vec(array: Id) -> Vec> { + fn into_vec(array: Id) -> Vec> { array .to_vec() .into_iter() @@ -163,7 +163,7 @@ pub trait INSArray: INSObject { } } - fn shared_object_at(&self, index: usize) -> ShareId + fn shared_object_at(&self, index: usize) -> Id where Self: INSArray, { @@ -171,7 +171,7 @@ pub trait INSArray: INSObject { unsafe { Id::retain(obj.into()) } } - fn from_slice(slice: &[ShareId]) -> Id + fn from_slice(slice: &[Id]) -> Id where Self: INSArray, { @@ -179,7 +179,7 @@ pub trait INSArray: INSObject { unsafe { from_refs(&refs) } } - fn to_shared_vec(&self) -> Vec> + fn to_shared_vec(&self) -> Vec> where Self: INSArray, { @@ -415,9 +415,9 @@ mod tests { use super::{INSArray, INSMutableArray, NSArray, NSMutableArray}; use crate::{INSObject, INSString, NSObject, NSString}; - use objc2::rc::Id; + use objc2::rc::{Id, Owned}; - fn sample_array(len: usize) -> Id> { + fn sample_array(len: usize) -> Id, Owned> { let mut vec = Vec::with_capacity(len); for _ in 0..len { vec.push(NSObject::new()); @@ -441,7 +441,7 @@ mod tests { assert!(array.first_object().unwrap() == array.object_at(0)); assert!(array.last_object().unwrap() == array.object_at(3)); - let empty_array: Id> = INSObject::new(); + let empty_array: Id, Owned> = INSObject::new(); assert!(empty_array.first_object().is_none()); assert!(empty_array.last_object().is_none()); } diff --git a/objc2_foundation/src/data.rs b/objc2_foundation/src/data.rs index 14c03dfdc..422d69f8e 100644 --- a/objc2_foundation/src/data.rs +++ b/objc2_foundation/src/data.rs @@ -6,7 +6,7 @@ use core::{ffi::c_void, ptr::NonNull}; use super::{INSCopying, INSMutableCopying, INSObject, NSRange}; use objc2::msg_send; -use objc2::rc::Id; +use objc2::rc::{Id, Owned}; #[cfg(feature = "block")] use objc2_block::{Block, ConcreteBlock}; @@ -30,7 +30,7 @@ pub trait INSData: INSObject { unsafe { slice::from_raw_parts(ptr, len) } } - fn with_bytes(bytes: &[u8]) -> Id { + fn with_bytes(bytes: &[u8]) -> Id { let cls = Self::class(); let bytes_ptr = bytes.as_ptr() as *const c_void; unsafe { @@ -42,7 +42,7 @@ pub trait INSData: INSObject { } #[cfg(feature = "block")] - fn from_vec(bytes: Vec) -> Id { + fn from_vec(bytes: Vec) -> Id { let capacity = bytes.capacity(); let dealloc = ConcreteBlock::new(move |bytes: *mut c_void, len: usize| unsafe { // Recreate the Vec and let it drop diff --git a/objc2_foundation/src/dictionary.rs b/objc2_foundation/src/dictionary.rs index 26498327f..180f2b27e 100644 --- a/objc2_foundation/src/dictionary.rs +++ b/objc2_foundation/src/dictionary.rs @@ -4,13 +4,13 @@ use core::marker::PhantomData; use core::ops::Index; use core::ptr::{self, NonNull}; -use objc2::rc::{Id, Owned, Ownership, ShareId}; +use objc2::rc::{Id, Owned, Ownership, Shared}; use objc2::runtime::Class; use objc2::{class, msg_send}; use super::{INSCopying, INSFastEnumeration, INSObject, NSArray, NSEnumerator, NSSharedArray}; -unsafe fn from_refs(keys: &[&T], vals: &[&D::Value]) -> Id +unsafe fn from_refs(keys: &[&T], vals: &[&D::Value]) -> Id where D: INSDictionary, T: INSCopying, @@ -93,14 +93,14 @@ pub trait INSDictionary: INSObject { } } - fn keys_array(&self) -> Id> { + fn keys_array(&self) -> Id, Owned> { unsafe { let keys = msg_send![self, allKeys]; Id::retain(NonNull::new_unchecked(keys)) } } - fn from_keys_and_objects(keys: &[&T], vals: Vec>) -> Id + fn from_keys_and_objects(keys: &[&T], vals: Vec>) -> Id where T: INSCopying, { @@ -108,7 +108,7 @@ pub trait INSDictionary: INSObject { unsafe { from_refs(keys, &vals_refs) } } - fn into_values_array(dict: Id) -> Id> { + fn into_values_array(dict: Id) -> Id, Owned> { unsafe { let vals = msg_send![dict, allValues]; Id::retain(NonNull::new_unchecked(vals)) @@ -117,8 +117,8 @@ pub trait INSDictionary: INSObject { } pub struct NSDictionary { - key: PhantomData>, - obj: PhantomData>, + key: PhantomData>, + obj: PhantomData>, } object_impl!(NSDictionary); @@ -166,12 +166,12 @@ where #[cfg(test)] mod tests { use alloc::vec; - use objc2::rc::Id; + use objc2::rc::{Owned, Id}; use super::{INSDictionary, NSDictionary}; use crate::{INSArray, INSObject, INSString, NSObject, NSString}; - fn sample_dict(key: &str) -> Id> { + fn sample_dict(key: &str) -> Id, Owned> { let string = NSString::from_str(key); let obj = NSObject::new(); NSDictionary::from_keys_and_objects(&[&*string], vec![obj]) diff --git a/objc2_foundation/src/enumerator.rs b/objc2_foundation/src/enumerator.rs index 3cb91e07b..8bd3fe9c9 100644 --- a/objc2_foundation/src/enumerator.rs +++ b/objc2_foundation/src/enumerator.rs @@ -5,7 +5,7 @@ use core::ptr::NonNull; use core::slice; use std::os::raw::c_ulong; -use objc2::rc::Id; +use objc2::rc::{Id, Owned}; use objc2::runtime::Object; use objc2::{msg_send, Encode, Encoding, RefEncode}; @@ -15,7 +15,7 @@ pub struct NSEnumerator<'a, T> where T: INSObject, { - id: Id, + id: Id, item: PhantomData<&'a T>, } diff --git a/objc2_foundation/src/object.rs b/objc2_foundation/src/object.rs index 637dced63..850a00019 100644 --- a/objc2_foundation/src/object.rs +++ b/objc2_foundation/src/object.rs @@ -2,7 +2,7 @@ use core::any::Any; use core::ptr::NonNull; use objc2::msg_send; -use objc2::rc::{Id, ShareId}; +use objc2::rc::{Id, Owned, Shared}; use objc2::runtime::{Class, BOOL, NO}; use objc2::Message; @@ -29,7 +29,7 @@ pub trait INSObject: Any + Sized + Message { result != NO } - fn description(&self) -> ShareId { + fn description(&self) -> Id { unsafe { let result: *mut NSString = msg_send![self, description]; // TODO: Verify that description always returns a non-null string @@ -42,7 +42,7 @@ pub trait INSObject: Any + Sized + Message { result != NO } - fn new() -> Id { + fn new() -> Id { let cls = Self::class(); unsafe { Id::new(msg_send![cls, new]) } } diff --git a/objc2_foundation/src/string.rs b/objc2_foundation/src/string.rs index 089e8405d..5e98a8b9d 100644 --- a/objc2_foundation/src/string.rs +++ b/objc2_foundation/src/string.rs @@ -6,14 +6,14 @@ use core::str; use std::os::raw::c_char; use objc2::msg_send; -use objc2::rc::{Id, ShareId}; +use objc2::rc::{Id, Owned, Shared}; use super::INSObject; pub trait INSCopying: INSObject { type Output: INSObject; - fn copy(&self) -> ShareId { + fn copy(&self) -> Id { unsafe { let obj: *mut Self::Output = msg_send![self, copy]; Id::new(NonNull::new_unchecked(obj)) @@ -24,7 +24,7 @@ pub trait INSCopying: INSObject { pub trait INSMutableCopying: INSObject { type Output: INSObject; - fn mutable_copy(&self) -> Id { + fn mutable_copy(&self) -> Id { unsafe { let obj: *mut Self::Output = msg_send![self, mutableCopy]; Id::new(NonNull::new_unchecked(obj)) @@ -58,7 +58,7 @@ pub trait INSString: INSObject { } } - fn from_str(string: &str) -> Id { + fn from_str(string: &str) -> Id { let cls = Self::class(); let bytes = string.as_ptr() as *const c_void; unsafe { diff --git a/objc2_foundation/src/value.rs b/objc2_foundation/src/value.rs index fc113543f..9f844aa7e 100644 --- a/objc2_foundation/src/value.rs +++ b/objc2_foundation/src/value.rs @@ -8,7 +8,7 @@ use core::str; use std::ffi::{CStr, CString}; use std::os::raw::c_char; -use objc2::rc::Id; +use objc2::rc::{Id, Owned}; use objc2::runtime::Class; use objc2::Encode; use objc2::{class, msg_send}; @@ -36,7 +36,7 @@ pub trait INSValue: INSObject { } } - fn from_value(value: Self::Value) -> Id { + fn from_value(value: Self::Value) -> Id { let cls = Self::class(); let value_ptr: *const Self::Value = &value; let bytes = value_ptr as *const c_void; From baff6f96d03ffd807ee5fd184402e691b1e1ea24 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Thu, 9 Sep 2021 15:32:47 +0200 Subject: [PATCH 16/16] Add a few more TODOs and docs to Id --- objc2/src/rc/id.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/objc2/src/rc/id.rs b/objc2/src/rc/id.rs index 109f69529..7d275384d 100644 --- a/objc2/src/rc/id.rs +++ b/objc2/src/rc/id.rs @@ -97,6 +97,7 @@ 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: Consider changing the name of Id -> Retain pub struct Id { /// A pointer to the contained object. The pointer is always retained. /// @@ -219,6 +220,12 @@ impl Id { #[cfg_attr(debug_assertions, inline)] fn autorelease_inner(self) -> *mut T { + // Note that this (and the actual `autorelease`) is not an associated + // function. This breaks the guideline that smart pointers shouldn't + // add inherent methods, but since autoreleasing only works on already + // retained objects it is hard to imagine a case where the inner type + // has a method with the same name. + let ptr = ManuallyDrop::new(self).ptr.as_ptr() as *mut objc2_sys::objc_object; // SAFETY: The `ptr` is guaranteed to be valid and have at least one // retain count. @@ -228,8 +235,24 @@ impl Id { debug_assert_eq!(res, ptr, "objc_autorelease did not return the same pointer"); res as *mut T } + + // TODO: objc_retainAutoreleasedReturnValue + // TODO: objc_autoreleaseReturnValue + // TODO: objc_retainAutorelease + // TODO: objc_retainAutoreleaseReturnValue + // TODO: objc_autoreleaseReturnValue + // TODO: objc_autoreleaseReturnValue } +// TODO: Consider something like this +// #[cfg(block)] +// impl Id { +// #[doc(alias = "objc_retainBlock")] +// pub unsafe fn retain_block(block: NonNull) -> Self { +// todo!() +// } +// } + impl Id { /// Autoreleases the owned [`Id`], returning a mutable reference bound to /// the pool.