From 4d2591ee0094ae1031e17d45daaac5574f2b6a4f Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Fri, 28 May 2021 15:48:17 +0200 Subject: [PATCH 1/9] Add initial `Retained` pointer, the smart pointer version of StrongPtr --- src/rc/mod.rs | 21 +--- src/rc/retained.rs | 245 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 247 insertions(+), 19 deletions(-) create mode 100644 src/rc/retained.rs diff --git a/src/rc/mod.rs b/src/rc/mod.rs index c703f585d..a16a56731 100644 --- a/src/rc/mod.rs +++ b/src/rc/mod.rs @@ -40,10 +40,12 @@ assert!(weak.load().is_null()); ``` */ +mod retained; mod strong; mod weak; mod autorelease; +pub use self::retained::Retained; pub use self::strong::StrongPtr; pub use self::weak::WeakPtr; pub use self::autorelease::autoreleasepool; @@ -55,25 +57,6 @@ mod tests { use super::StrongPtr; use super::autoreleasepool; - #[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 { diff --git a/src/rc/retained.rs b/src/rc/retained.rs new file mode 100644 index 000000000..f4e22c424 --- /dev/null +++ b/src/rc/retained.rs @@ -0,0 +1,245 @@ +use core::borrow; +use core::fmt; +use core::hash; +use core::marker::{PhantomData, Unpin}; +use core::mem; +use core::ops::Deref; +use core::ptr::NonNull; + +use crate::runtime::{self, Object}; + +/// A smart pointer that strongly references an object, ensuring it won't be +/// deallocated. +/// +/// This is guaranteed to have the same size as the underlying pointer. +/// +/// ## 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` selector instead. +/// +/// TODO: Explain similarities with `Arc` and `RefCell`. +#[repr(transparent)] +pub struct Retained { + /// A pointer to the contained object. + /// + /// It is important that this is `NonNull`, since we want to dereference + /// it later. + /// + /// Usually the contained object would be an [extern type][extern-type-rfc] + /// (when that gets stabilized), or a type such as: + /// ``` + /// pub struct MyType { + /// _data: [u8; 0], // TODO: `UnsafeCell`? + /// } + /// ``` + /// + /// DSTs that carry metadata cannot be used here, so unsure if we should + /// have a `?Sized` bound? + /// + /// TODO: + /// https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait + /// https://doc.rust-lang.org/nomicon/exotic-sizes.html + /// https://doc.rust-lang.org/core/ptr/trait.Pointee.html + /// https://doc.rust-lang.org/core/ptr/traitalias.Thin.html + /// + /// [extern-type-rfc]: https://github.com/rust-lang/rfcs/blob/master/text/1861-extern-types.md + ptr: NonNull, // Covariant + phantom: PhantomData, +} + +impl Retained { + /// Constructs a `Retained` to an 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, and has +1 + /// retain count. + /// + /// TODO: Something about there not being any mutable references. + #[inline] + pub const unsafe fn new(ptr: NonNull) -> Self { + Retained { + ptr, + phantom: PhantomData, + } + } + + #[inline] + pub const fn as_ptr(&self) -> *mut T { + self.ptr.as_ptr() + } + + /// Retains the given object pointer. + /// + /// When dropped, the object will be released. + /// + /// # Safety + /// + /// The caller must ensure the given object pointer is valid. + #[doc(alias = "objc_retain")] + #[inline] + pub unsafe fn retain(ptr: NonNull) -> Self { + // SAFETY: The caller upholds that the pointer is valid + let rtn = runtime::objc_retain(ptr.as_ptr() as *mut Object); + debug_assert_eq!(rtn, ptr.as_ptr() as *mut Object); + Retained { + ptr, + phantom: PhantomData, + } + } + + /// Autoreleases the retained pointer, meaning that the object is not + /// immediately released, but will be when the innermost / current + /// autorelease pool is drained. + /// + /// A pointer to the object is returned, but it's validity is only until + /// guaranteed until the innermost pool 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(self) -> NonNull { + let ptr = mem::ManuallyDrop::new(self).ptr; + // 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. + unsafe { runtime::objc_autorelease(ptr.as_ptr() as *mut Object) }; + ptr + } + + #[cfg(test)] + #[doc(alias = "retainCount")] + pub fn retain_count(&self) -> usize { + unsafe { msg_send![self.ptr.as_ptr() as *mut Object, retainCount] } + } +} + +// TODO: #[may_dangle] +// https://doc.rust-lang.org/nightly/nomicon/dropck.html +impl Drop for Retained { + /// Releases the retained object + #[doc(alias = "objc_release")] + #[doc(alias = "release")] + #[inline] + fn drop(&mut self) { + // SAFETY: The `ptr` is guaranteed to be valid and have at least one + // retain count + unsafe { runtime::objc_release(self.ptr.as_ptr() as *mut Object) }; + } +} + +impl Clone for Retained { + /// Makes a clone of the `Retained` object. + /// + /// This increases the object's reference count. + #[doc(alias = "objc_retain")] + #[doc(alias = "retain")] + #[inline] + fn clone(&self) -> Self { + // SAFETY: The `ptr` is guaranteed to be valid + unsafe { Self::retain(self.ptr) } + } +} + +impl Deref for Retained { + type Target = T; + + #[inline] + fn deref(&self) -> &Self::Target { + // SAFETY: TODO + unsafe { self.ptr.as_ref() } + } +} + +impl PartialEq for Retained { + #[inline] + fn eq(&self, other: &Retained) -> bool { + &**self == &**other + } + + #[inline] + fn ne(&self, other: &Retained) -> bool { + &**self != &**other + } +} + +// TODO: impl PartialOrd, Ord and Eq + +impl fmt::Display for Retained { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(&**self, f) + } +} + +impl fmt::Debug for Retained { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Debug::fmt(&**self, f) + } +} + +impl fmt::Pointer for Retained { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + fmt::Pointer::fmt(&self.as_ptr(), f) + } +} + +impl hash::Hash for Retained { + fn hash(&self, state: &mut H) { + (&**self).hash(state) + } +} + +impl borrow::Borrow for Retained { + fn borrow(&self) -> &T { + &**self + } +} + +impl AsRef for Retained { + fn as_ref(&self) -> &T { + &**self + } +} + +impl Unpin for Retained {} + +#[cfg(test)] +mod tests { + use std::mem::size_of; + + use super::Retained; + 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>() + ); + } + + #[cfg(any(target_os = "macos", target_os = "ios"))] + #[test] + fn test_clone() { + let obj: Retained = unsafe { Retained::new(msg_send![class!(NSObject), new]) }; + assert!(obj.retain_count() == 1); + + let cloned = obj.clone(); + assert!(cloned.retain_count() == 2); + assert!(obj.retain_count() == 2); + + drop(obj); + assert!(cloned.retain_count() == 1); + } +} From b9f18496afd66d9fc14460bb84362bc8f227f43c Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Fri, 28 May 2021 17:33:28 +0200 Subject: [PATCH 2/9] Fix creation of Retained in test when verify_message feature is enabled --- src/rc/retained.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/rc/retained.rs b/src/rc/retained.rs index f4e22c424..a25f2cade 100644 --- a/src/rc/retained.rs +++ b/src/rc/retained.rs @@ -211,7 +211,8 @@ impl Unpin for Retained {} #[cfg(test)] mod tests { - use std::mem::size_of; + use core::mem::size_of; + use core::ptr::NonNull; use super::Retained; use crate::runtime::Object; @@ -232,7 +233,9 @@ mod tests { #[cfg(any(target_os = "macos", target_os = "ios"))] #[test] fn test_clone() { - let obj: Retained = unsafe { Retained::new(msg_send![class!(NSObject), new]) }; + // TODO: Maybe make a way to return `Retained` directly? + let obj: *mut Object = unsafe { msg_send![class!(NSObject), new] }; + let obj: Retained = unsafe { Retained::new(NonNull::new(obj).unwrap()) }; assert!(obj.retain_count() == 1); let cloned = obj.clone(); From 2cebfce66573155339a3fb741a390cd78eab5e63 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 29 May 2021 02:02:40 +0200 Subject: [PATCH 3/9] Add initial `Owned` pointer, the mutable version of Retained --- src/rc/mod.rs | 2 + src/rc/owned.rs | 160 +++++++++++++++++++++++++++++++++++++++++++++ src/rc/retained.rs | 8 +++ 3 files changed, 170 insertions(+) create mode 100644 src/rc/owned.rs diff --git a/src/rc/mod.rs b/src/rc/mod.rs index a16a56731..57d81ef50 100644 --- a/src/rc/mod.rs +++ b/src/rc/mod.rs @@ -40,12 +40,14 @@ assert!(weak.load().is_null()); ``` */ +mod owned; mod retained; mod strong; mod weak; mod autorelease; pub use self::retained::Retained; +pub use self::owned::Owned; pub use self::strong::StrongPtr; pub use self::weak::WeakPtr; pub use self::autorelease::autoreleasepool; diff --git a/src/rc/owned.rs b/src/rc/owned.rs new file mode 100644 index 000000000..e3cffe8fa --- /dev/null +++ b/src/rc/owned.rs @@ -0,0 +1,160 @@ +use core::marker::PhantomData; +use core::ptr::{NonNull, drop_in_place}; +use core::mem; +use core::ops::{DerefMut, Deref}; +use core::fmt; +use core::hash; +use core::borrow; + +use super::Retained; +use crate::runtime::{self, Object}; + +/// A smart pointer that strongly references and owns an Objective-C object. +/// +/// The fact that we own the pointer means that we're safe to mutate it, hence +/// why this implements [`DerefMut`]. +/// +/// This is guaranteed to have the same size as the underlying pointer. +/// +/// TODO: Explain similarities to [`Box`]. +/// +/// TODO: Explain this vs. [`Retained`] +#[repr(transparent)] +pub struct Owned { + /// The pointer is always retained. + pub(super) ptr: NonNull, // Covariant + phantom: PhantomData, // Necessary for dropcheck +} + +// SAFETY: TODO +unsafe impl Send for Owned {} + +// SAFETY: TODO +unsafe impl Sync for Owned {} + +impl Owned { + #[inline] + pub unsafe fn new(ptr: NonNull) -> Self { + Self { + ptr, + phantom: PhantomData, + } + } + + // TODO: Unsure how the API should look... + #[inline] + pub unsafe fn retain(ptr: NonNull) -> Self { + Self::from_retained(Retained::retain(ptr)) + } + + /// TODO + /// + /// # Safety + /// + /// The given [`Retained`] must be the only reference to the object + /// anywhere in the program - even in other Objective-C code. + #[inline] + pub unsafe fn from_retained(obj: Retained) -> Self { + let ptr = mem::ManuallyDrop::new(obj).ptr; + Self { + ptr, + phantom: PhantomData, + } + } +} + +// TODO: #[may_dangle] +// https://doc.rust-lang.org/nightly/nomicon/dropck.html +impl Drop for Owned { + /// Releases the retained object + /// + /// This is guaranteed to be the last destructor that runs, in contrast to + /// [`Retained`], which means that we can run the [`Drop`] implementation + /// on the contained object as well. + #[inline] + fn drop(&mut self) { + let ptr = self.ptr; + unsafe { + drop_in_place(ptr.as_ptr()); + // Construct a new `Retained`, which will be dropped immediately + Retained::new(ptr); + }; + } +} + +// Note: `Clone` is not implemented for this! + +impl Deref for Owned { + type Target = T; + + #[inline] + fn deref(&self) -> &Self::Target { + // SAFETY: TODO + unsafe { self.ptr.as_ref() } + } +} + +impl DerefMut for Owned { + #[inline] + fn deref_mut(&mut self) -> &mut Self::Target { + // SAFETY: TODO + unsafe { self.ptr.as_mut() } + } +} + +// TODO: impl PartialEq, PartialOrd, Ord and Eq + +impl fmt::Display for Owned { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(&**self, f) + } +} + +impl fmt::Debug for Owned { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Debug::fmt(&**self, f) + } +} + +impl fmt::Pointer for Owned { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + fmt::Pointer::fmt(&self.ptr.as_ptr(), f) + } +} + +impl hash::Hash for Owned { + fn hash(&self, state: &mut H) { + (&**self).hash(state) + } +} + +// TODO: impl Fn traits? See `boxed_closure_impls` + +// TODO: CoerceUnsized + +impl borrow::Borrow for Owned { + fn borrow(&self) -> &T { + &**self + } +} + +impl borrow::BorrowMut for Owned { + fn borrow_mut(&mut self) -> &mut T { + &mut **self + } +} + +impl AsRef for Owned { + fn as_ref(&self) -> &T { + &**self + } +} + +impl AsMut for Owned { + fn as_mut(&mut self) -> &mut T { + &mut **self + } +} + +// TODO: Comment on impl Unpin for Box +impl Unpin for Owned {} diff --git a/src/rc/retained.rs b/src/rc/retained.rs index a25f2cade..d1bda9986 100644 --- a/src/rc/retained.rs +++ b/src/rc/retained.rs @@ -7,6 +7,7 @@ use core::ops::Deref; use core::ptr::NonNull; use crate::runtime::{self, Object}; +use super::Owned; /// A smart pointer that strongly references an object, ensuring it won't be /// deallocated. @@ -209,6 +210,13 @@ impl AsRef for Retained { impl Unpin for Retained {} +impl From> for Retained { + fn from(obj: Owned) -> Self { + // SAFETY: TODO + unsafe { Self::new(obj.ptr) } + } +} + #[cfg(test)] mod tests { use core::mem::size_of; From a0760018ccefc73f459e7f8167a149983dcce51a Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 29 May 2021 02:03:02 +0200 Subject: [PATCH 4/9] Add Send and Sync impls for Retained --- src/rc/retained.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/rc/retained.rs b/src/rc/retained.rs index d1bda9986..ef61775bb 100644 --- a/src/rc/retained.rs +++ b/src/rc/retained.rs @@ -51,6 +51,13 @@ pub struct Retained { phantom: PhantomData, } +// SAFETY: TODO +unsafe impl Send for Retained {} + +// SAFETY: TODO +unsafe impl Sync for Retained {} + + impl Retained { /// Constructs a `Retained` to an object that already has a +1 retain /// count. This will not retain the object. From 869449dc4aae0a2d08adb04d579852613a049aa2 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 29 May 2021 02:03:41 +0200 Subject: [PATCH 5/9] Clean up Retained code a bit --- src/rc/retained.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/rc/retained.rs b/src/rc/retained.rs index ef61775bb..e8ad25b09 100644 --- a/src/rc/retained.rs +++ b/src/rc/retained.rs @@ -71,15 +71,15 @@ impl Retained { /// /// TODO: Something about there not being any mutable references. #[inline] - pub const unsafe fn new(ptr: NonNull) -> Self { - Retained { + pub unsafe fn new(ptr: NonNull) -> Self { + Self { ptr, phantom: PhantomData, } } #[inline] - pub const fn as_ptr(&self) -> *mut T { + pub fn as_ptr(&self) -> *mut T { self.ptr.as_ptr() } @@ -96,7 +96,7 @@ impl Retained { // SAFETY: The caller upholds that the pointer is valid let rtn = runtime::objc_retain(ptr.as_ptr() as *mut Object); debug_assert_eq!(rtn, ptr.as_ptr() as *mut Object); - Retained { + Self { ptr, phantom: PhantomData, } @@ -167,12 +167,12 @@ impl Deref for Retained { impl PartialEq for Retained { #[inline] - fn eq(&self, other: &Retained) -> bool { + fn eq(&self, other: &Self) -> bool { &**self == &**other } #[inline] - fn ne(&self, other: &Retained) -> bool { + fn ne(&self, other: &Self) -> bool { &**self != &**other } } @@ -193,7 +193,7 @@ impl fmt::Debug for Retained { impl fmt::Pointer for Retained { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - fmt::Pointer::fmt(&self.as_ptr(), f) + fmt::Pointer::fmt(&self.ptr.as_ptr(), f) } } From 684159dd8fd7f67d8d2e3217035894822e8a09ba Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 29 May 2021 02:04:10 +0200 Subject: [PATCH 6/9] Add some extra TODOs to Retained --- src/rc/retained.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/rc/retained.rs b/src/rc/retained.rs index e8ad25b09..5dcdcf925 100644 --- a/src/rc/retained.rs +++ b/src/rc/retained.rs @@ -14,6 +14,9 @@ use super::Owned; /// /// This is guaranteed to have the same size as the underlying pointer. /// +/// TODO: Something about the fact that we haven't made the methods associated +/// for [reasons]??? +/// /// ## Caveats /// /// If the inner type implements [`Drop`], that implementation will not be @@ -21,6 +24,8 @@ use super::Owned; /// do so. If you need to run some code when the object is destroyed, /// implement the `dealloc` selector instead. /// +/// TODO: Restrict the possible types with some kind of unsafe marker trait? +/// /// TODO: Explain similarities with `Arc` and `RefCell`. #[repr(transparent)] pub struct Retained { @@ -47,7 +52,9 @@ pub struct Retained { /// https://doc.rust-lang.org/core/ptr/traitalias.Thin.html /// /// [extern-type-rfc]: https://github.com/rust-lang/rfcs/blob/master/text/1861-extern-types.md - ptr: NonNull, // Covariant + pub(super) ptr: NonNull, // Covariant - but should probably be invariant? + /// TODO: + /// https://github.com/rust-lang/rfcs/blob/master/text/0769-sound-generic-drop.md#phantom-data phantom: PhantomData, } @@ -92,6 +99,7 @@ impl Retained { /// The caller must ensure the given object pointer is valid. #[doc(alias = "objc_retain")] #[inline] + // TODO: Maybe just take a normal reference, and then this can be safe? pub unsafe fn retain(ptr: NonNull) -> Self { // SAFETY: The caller upholds that the pointer is valid let rtn = runtime::objc_retain(ptr.as_ptr() as *mut Object); @@ -215,6 +223,8 @@ impl AsRef for Retained { } } +// TODO: CoerceUnsized? + impl Unpin for Retained {} impl From> for Retained { From f52bc56fd6e36ef54dbf50d39e664d8b5e7d6038 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 29 May 2021 03:13:06 +0200 Subject: [PATCH 7/9] Create Owned and Retained using references instead of NonNull pointers Makes the API easier to use, and means there are less safety requirements that we have to document. --- src/rc/owned.rs | 55 ++++++++++++++++++++++++++++++---------------- src/rc/retained.rs | 54 ++++++++++++++++++++++++++++++--------------- 2 files changed, 72 insertions(+), 37 deletions(-) diff --git a/src/rc/owned.rs b/src/rc/owned.rs index e3cffe8fa..009168398 100644 --- a/src/rc/owned.rs +++ b/src/rc/owned.rs @@ -1,18 +1,18 @@ -use core::marker::PhantomData; -use core::ptr::{NonNull, drop_in_place}; -use core::mem; -use core::ops::{DerefMut, Deref}; +use core::borrow; use core::fmt; use core::hash; -use core::borrow; +use core::marker::PhantomData; +use core::mem; +use core::ops::{Deref, DerefMut}; +use core::ptr::{drop_in_place, NonNull}; use super::Retained; use crate::runtime::{self, Object}; /// A smart pointer that strongly references and owns an Objective-C object. /// -/// The fact that we own the pointer means that we're safe to mutate it, hence -/// why this implements [`DerefMut`]. +/// The fact that we own the pointer means that it's safe to mutate it. As +/// such, this implements [`DerefMut`]. /// /// This is guaranteed to have the same size as the underlying pointer. /// @@ -32,27 +32,44 @@ unsafe impl Send for Owned {} // SAFETY: TODO unsafe impl Sync for Owned {} +// TODO: Unsure how the API should look... impl Owned { + /// TODO + /// + /// # Safety + /// + /// The caller must ensure the given object reference has exactly 1 retain + /// count (that is, a retain count that has been handed off from somewhere + /// else, usually Objective-C methods like `init`, `alloc`, `new`, or + /// `copy`). + /// + /// Additionally, there must be no other pointers to the same object. + /// + /// # Example + /// + /// ```rust + /// let obj: &mut Object = unsafe { msg_send![cls, alloc] }; + /// let obj: Owned = unsafe { Owned::new(msg_send![obj, init]) }; + /// // Or in this case simply just: + /// let obj: Owned = unsafe { Owned::new(msg_send![cls, new]) }; + /// ``` + /// + /// TODO: Something about there not being other references. #[inline] - pub unsafe fn new(ptr: NonNull) -> Self { + pub unsafe fn new(obj: &mut T) -> Self { Self { - ptr, + ptr: obj.into(), phantom: PhantomData, } } - // TODO: Unsure how the API should look... - #[inline] - pub unsafe fn retain(ptr: NonNull) -> Self { - Self::from_retained(Retained::retain(ptr)) - } - - /// TODO + /// Construct an `Owned` pointer /// /// # Safety /// - /// The given [`Retained`] must be the only reference to the object - /// anywhere in the program - even in other Objective-C code. + /// The caller must ensure that there are no other pointers to the same + /// object (which also means that the given [`Retained`] should have a + /// retain count of exactly 1). #[inline] pub unsafe fn from_retained(obj: Retained) -> Self { let ptr = mem::ManuallyDrop::new(obj).ptr; @@ -77,7 +94,7 @@ impl Drop for Owned { unsafe { drop_in_place(ptr.as_ptr()); // Construct a new `Retained`, which will be dropped immediately - Retained::new(ptr); + Retained::new(ptr.as_ref()); }; } } diff --git a/src/rc/retained.rs b/src/rc/retained.rs index 5dcdcf925..acd58df3f 100644 --- a/src/rc/retained.rs +++ b/src/rc/retained.rs @@ -6,8 +6,8 @@ use core::mem; use core::ops::Deref; use core::ptr::NonNull; -use crate::runtime::{self, Object}; use super::Owned; +use crate::runtime::{self, Object}; /// A smart pointer that strongly references an object, ensuring it won't be /// deallocated. @@ -64,29 +64,33 @@ unsafe impl Send for Retained {} // SAFETY: TODO unsafe impl Sync for Retained {} - impl Retained { /// Constructs a `Retained` to an object that already has a +1 retain /// count. This will not retain the object. /// /// When dropped, the object will be released. /// + /// See also [`Owned::new`] for the common case of creating objects. + /// /// # Safety /// - /// The caller must ensure the given object pointer is valid, and has +1 - /// retain count. + /// The caller must ensure the given object reference has +1 retain count + /// (that is, a retain count that has been handed off from somewhere else, + /// usually Objective-C methods with the `ns_returns_retained` attribute). + /// + /// Additionally, there must be no [`Owned`] pointers to the same object. /// /// TODO: Something about there not being any mutable references. #[inline] - pub unsafe fn new(ptr: NonNull) -> Self { + pub unsafe fn new(obj: &T) -> Self { Self { - ptr, + ptr: obj.into(), phantom: PhantomData, } } #[inline] - pub fn as_ptr(&self) -> *mut T { + pub fn as_ptr(&self) -> *const T { self.ptr.as_ptr() } @@ -96,16 +100,28 @@ impl Retained { /// /// # Safety /// - /// The caller must ensure the given object pointer is valid. + /// The caller must ensure that there are no [`Owned`] pointers to the + /// same object. + // + // So this would be illegal: + // ```rust + // let owned: Owned = ...; + // // Lifetime information is discarded + // let retained = Retained::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")] - #[inline] - // TODO: Maybe just take a normal reference, and then this can be safe? - pub unsafe fn retain(ptr: NonNull) -> Self { + // Inlined since it's `objc_retain` that does the work. + #[cfg_attr(debug_assertions, inline)] + pub unsafe fn retain(obj: &T) -> Self { // SAFETY: The caller upholds that the pointer is valid - let rtn = runtime::objc_retain(ptr.as_ptr() as *mut Object); - debug_assert_eq!(rtn, ptr.as_ptr() as *mut Object); + let rtn = runtime::objc_retain(obj as *const T as *mut Object); + debug_assert_eq!(rtn, obj as *const T as *mut Object); Self { - ptr, + ptr: obj.into(), phantom: PhantomData, } } @@ -119,6 +135,8 @@ impl Retained { #[doc(alias = "objc_autorelease")] #[must_use = "If you don't intend to use the object any more, just drop it as usual"] #[inline] + // TODO: Get a lifetime relating to the pool, so that we can return a + // reference instead of a pointer. pub fn autorelease(self) -> NonNull { let ptr = mem::ManuallyDrop::new(self).ptr; // SAFETY: The `ptr` is guaranteed to be valid and have at least one @@ -159,7 +177,7 @@ impl Clone for Retained { #[inline] fn clone(&self) -> Self { // SAFETY: The `ptr` is guaranteed to be valid - unsafe { Self::retain(self.ptr) } + unsafe { Self::retain(&*self) } } } @@ -230,7 +248,7 @@ impl Unpin for Retained {} impl From> for Retained { fn from(obj: Owned) -> Self { // SAFETY: TODO - unsafe { Self::new(obj.ptr) } + unsafe { Self::new(&*obj) } } } @@ -259,8 +277,8 @@ mod tests { #[test] fn test_clone() { // TODO: Maybe make a way to return `Retained` directly? - let obj: *mut Object = unsafe { msg_send![class!(NSObject), new] }; - let obj: Retained = unsafe { Retained::new(NonNull::new(obj).unwrap()) }; + let obj: &Object = unsafe { msg_send![class!(NSObject), new] }; + let obj: Retained = unsafe { Retained::new(obj) }; assert!(obj.retain_count() == 1); let cloned = obj.clone(); From fecb2e7ced6c2203d6472e58ef15d7d4df0b0d46 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 29 May 2021 03:28:29 +0200 Subject: [PATCH 8/9] Add some more documentation to Owned and Retained --- src/rc/owned.rs | 17 ++++++++++++++++- src/rc/retained.rs | 5 ++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/rc/owned.rs b/src/rc/owned.rs index 009168398..ac6c60a51 100644 --- a/src/rc/owned.rs +++ b/src/rc/owned.rs @@ -16,6 +16,18 @@ use crate::runtime::{self, Object}; /// /// This is guaranteed to have the same size as the underlying pointer. /// +/// # Cloning and [`Retained`] +/// +/// This does not implement [`Clone`], but [`Retained`] has a [`From`] +/// implementation to convert from this, so you can easily reliquish ownership +/// and work with a normal [`Retained`] pointer. +/// +/// ```no_run +/// let obj: Owned = ...; +/// let retained: Retained = obj.into(); +/// let cloned: Retained = retained.clone(); +/// ``` +/// /// TODO: Explain similarities to [`Box`]. /// /// TODO: Explain this vs. [`Retained`] @@ -43,7 +55,8 @@ impl Owned { /// else, usually Objective-C methods like `init`, `alloc`, `new`, or /// `copy`). /// - /// Additionally, there must be no other pointers to the same object. + /// Additionally, there must be no other pointers or references to the same + /// object, and the given reference must not be used afterwards. /// /// # Example /// @@ -55,6 +68,8 @@ impl Owned { /// ``` /// /// TODO: Something about there not being other references. + // Note: The fact that we take a `&mut` here is more of a lint; the lifetime + // information is lost, so whatever produced the reference can still be #[inline] pub unsafe fn new(obj: &mut T) -> Self { Self { diff --git a/src/rc/retained.rs b/src/rc/retained.rs index acd58df3f..2b9917841 100644 --- a/src/rc/retained.rs +++ b/src/rc/retained.rs @@ -9,9 +9,12 @@ use core::ptr::NonNull; use super::Owned; use crate::runtime::{self, Object}; -/// A smart pointer that strongly references an object, ensuring it won't be +/// An smart pointer that strongly references an object, ensuring it won't be /// deallocated. /// +/// This doesn't own the object, so it is not safe to obtain a mutable +/// reference from this. For that, see [`Owned`]. +/// /// This is guaranteed to have the same size as the underlying pointer. /// /// TODO: Something about the fact that we haven't made the methods associated From 2ec0185ba1395ec7413b1a2ebedd57728b15f678 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 29 May 2021 03:47:27 +0200 Subject: [PATCH 9/9] Add stubs for a few extra objc reference counting methods --- src/rc/retained.rs | 41 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/src/rc/retained.rs b/src/rc/retained.rs index 2b9917841..ebf6574f1 100644 --- a/src/rc/retained.rs +++ b/src/rc/retained.rs @@ -129,6 +129,12 @@ impl Retained { } } + /// TODO + #[doc(alias = "objc_retainAutoreleasedReturnValue")] + pub unsafe fn retain_autoreleased_return(obj: &T) -> Self { + todo!() + } + /// Autoreleases the retained pointer, meaning that the object is not /// immediately released, but will be when the innermost / current /// autorelease pool is drained. @@ -150,13 +156,46 @@ impl Retained { ptr } - #[cfg(test)] + /// TODO + #[doc(alias = "objc_autoreleaseReturnValue")] + pub fn autorelease_return(self) -> *const T { + todo!() + } + + /// TODO + /// + /// Equivalent to `Retained::retain(&obj).autorelease()`, but slightly + /// more efficient. + #[doc(alias = "objc_retainAutorelease")] + pub unsafe fn retain_and_autorelease(obj: &T) -> *const T { + todo!() + } + + /// TODO + /// + /// Equivalent to `Retained::retain(&obj).autorelease_return()`, but + /// slightly more efficient. + #[doc(alias = "objc_retainAutoreleaseReturnValue")] + pub unsafe fn retain_and_autorelease_return(obj: &T) -> *const T { + todo!() + } + + #[cfg(test)] // TODO #[doc(alias = "retainCount")] pub fn retain_count(&self) -> usize { unsafe { msg_send![self.ptr.as_ptr() as *mut Object, retainCount] } } } +// TODO: Consider something like this +// #[cfg(block)] +// impl Retained { +// #[doc(alias = "objc_retainBlock")] +// pub unsafe fn retain_block(block: &T) -> Self { +// todo!() +// } +// } + // TODO: #[may_dangle] // https://doc.rust-lang.org/nightly/nomicon/dropck.html impl Drop for Retained {