Merge pull request #520 from madsmtm/option-allocated

Change `Allocated<T>` in preparation for arbitrary self types

crates/header-translator/src/method.rs

@@ -686,7 +686,7 @@ impl fmt::Display for Method {
 
         // Receiver
         if let MemoryManagement::IdInit = self.memory_management {
-            write!(f, "this: Option<Allocated<Self>>, ")?;
+            write!(f, "this: Allocated<Self>, ")?;
         } else if self.is_class {
             // Insert nothing; a class method is assumed
         } else if self.mutating {

crates/icrate/src/additions/Foundation/data.rs

@@ -267,7 +267,7 @@ impl IdFromIterator<u8> for NSMutableData {
 }
 
 #[cfg(feature = "block")]
-unsafe fn with_vec<T: Message>(obj: Option<Allocated<T>>, bytes: Vec<u8>) -> Id<T> {
+unsafe fn with_vec<T: Message>(obj: Allocated<T>, bytes: Vec<u8>) -> Id<T> {
     use core::mem::ManuallyDrop;
 
     use block2::{Block, ConcreteBlock};

crates/icrate/src/additions/Foundation/string.rs

@@ -144,7 +144,7 @@ impl NSMutableString {
     }
 }
 
-unsafe fn init_with_str<T: Message>(obj: Option<Allocated<T>>, string: &str) -> Id<T> {
+unsafe fn init_with_str<T: Message>(obj: Allocated<T>, string: &str) -> Id<T> {
     let bytes: *const c_void = string.as_ptr().cast();
     // We use `msg_send_id` instead of the generated method from `icrate`,
     // since that assumes the encoding is `usize`, whereas GNUStep assumes

crates/icrate/src/additions/Foundation/thread.rs

@@ -157,7 +157,7 @@ impl MainThreadMarker {
     /// let obj: Id<SomeClass> = unsafe { msg_send_id![obj, init] };
     /// ```
     #[inline]
-    pub fn alloc<T: ClassType>(self) -> Option<Allocated<T>> {
+    pub fn alloc<T: ClassType>(self) -> Allocated<T> {
         // SAFETY: Same as `ClassType::alloc`, with the addition that since we
         // take `self: MainThreadMarker`, the `IsAllocableAnyThread` bound is
         // not required.

crates/icrate/src/additions/Metal/private.rs

@@ -27,7 +27,7 @@ extern_methods!(
         #[cfg(feature = "Metal_MTLDevice")]
         #[method_id(initWithVertexData:fragmentData:serializedVertexDescriptor:device:options:flags:)]
         pub unsafe fn initWithVertexData(
-            this: Option<Allocated<Self>>,
+            this: Allocated<Self>,
             vertex_data: *mut c_void,
             fragment_data: *mut c_void,
             vertex_desc: *mut c_void,

crates/icrate/src/fixes/Foundation/NSUUID.rs

@@ -19,10 +19,7 @@ unsafe impl RefEncode for UuidBytes {
 extern_methods!(
     unsafe impl Foundation::NSUUID {
         #[method_id(initWithUUIDBytes:)]
-        pub(crate) fn initWithUUIDBytes(
-            this: Option<Allocated<Self>>,
-            bytes: &UuidBytes,
-        ) -> Id<Self>;
+        pub(crate) fn initWithUUIDBytes(this: Allocated<Self>, bytes: &UuidBytes) -> Id<Self>;
 
         #[method(getUUIDBytes:)]
         pub(crate) fn getUUIDBytes(&self, bytes: &mut UuidBytes);

crates/icrate/tests/mutable_array.rs

@@ -1,5 +1,5 @@
 #![cfg(feature = "Foundation_NSMutableArray")]
-use objc2::rc::{__RcTestObject, __ThreadTestData, autoreleasepool};
+use objc2::rc::{__RcTestObject, __ThreadTestData, autoreleasepool, Allocated};
 use objc2::{msg_send, ClassType};
 
 #[cfg(feature = "Foundation_NSNumber")]
@@ -123,7 +123,8 @@ fn test_threaded() {
 
         s.spawn(|| {
             let array = <NSMutableArray<NSObject>>::alloc();
-            assert!(array.is_some());
+            let ptr = Allocated::as_ptr(&array);
+            assert!(!ptr.is_null());
         });
     });
 }

crates/objc2/CHANGELOG.md

@@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   - `CounterpartOrSelf`.
 * Added new `encode` traits `EncodeReturn`, `EncodeArgument` and
   `EncodeArguments`.
+* Added methods `as_ptr` and `as_mut_ptr` to `Allocated`.
 
 ### Changed
 * **BREAKING**: `AnyClass::verify_sel` now take more well-defined types
@@ -47,6 +48,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   a method.
 * **BREAKING**: Renamed the associated types `Ret` and `Args` on
   `MethodImplementation` to `Return` and `Arguments`.
+* **BREAKING**: Make `rc::Allocated` allowed to be `NULL` internally, such
+  that uses of `Option<Allocated<T>>` is now simply `Allocated<T>`.
 
 ### Deprecated
 * Soft deprecated using `msg_send!` without a comma between arguments (i.e.

crates/objc2/src/__macro_helpers/declare_class.rs

@@ -46,7 +46,7 @@ where
 }
 
 // Explicitly left unimplemented for now!
-// impl MessageRecieveId<impl MessageReceiver, Option<Allocated<T>>> for Alloc {}
+// impl MessageRecieveId<impl MessageReceiver, Allocated<T>> for Alloc {}
 
 // Note: `MethodImplementation` allows for `Allocated` as the receiver, so we
 // restrict it here to only be when the selector is `init`.
@@ -56,7 +56,7 @@ where
 impl<Ret, T> MessageRecieveId<Allocated<T>, Ret> for Init
 where
     T: Message,
-    Ret: MaybeOptionId<Input = Id<T>>,
+    Ret: MaybeOptionId<Input = Option<Id<T>>>,
 {
     #[inline]
     fn into_return(obj: Ret) -> IdReturnValue {

crates/objc2/src/__macro_helpers/method_family.rs

@@ -13,8 +13,8 @@
 ///     retainable object pointer type - we ensure this by making
 ///     `message_send_id` return `Id`.
 ///   - `init`: The method must be an instance method and must return an
-///     Objective-C pointer type - We ensure this by taking
-///     `Option<Allocated<T>>`, which means it can't be a class method!
+///     Objective-C pointer type - We ensure this by taking `Allocated<T>`,
+///     which means it can't be a class method!
 ///
 /// <https://clang.llvm.org/docs/AutomaticReferenceCounting.html#retainable-object-pointers-as-operands-and-arguments>
 // TODO: Use an enum instead of u8 here when stable

crates/objc2/src/__macro_helpers/msg_send_id.rs

@@ -19,17 +19,17 @@ pub trait MsgSendId<T, U> {
     /// `send_message_id` with that, and return an error if one occurred.
     #[inline]
     #[track_caller]
-    unsafe fn send_message_id_error<A, E>(obj: T, sel: Sel, args: A) -> Result<U, Id<E>>
+    unsafe fn send_message_id_error<A, E, R>(obj: T, sel: Sel, args: A) -> Result<R, Id<E>>
     where
         *mut *mut E: Encode,
         A: TupleExtender<*mut *mut E>,
         <A as TupleExtender<*mut *mut E>>::PlusOneArgument: ConvertArguments,
         E: Message,
-        Option<U>: MaybeUnwrap<Input = U>,
+        Option<R>: MaybeUnwrap<Input = U>,
     {
         let mut err: *mut E = ptr::null_mut();
         let args = args.add_argument(&mut err);
-        let res: Option<U> = unsafe { Self::send_message_id(obj, sel, args) };
+        let res: Option<R> = unsafe { Self::send_message_id(obj, sel, args) };
         // As per the Cocoa documentation:
         // > Success or failure is indicated by the return value of the
         // > method. Although Cocoa methods that indirectly return error
@@ -66,9 +66,9 @@ unsafe fn encountered_error<E: Message>(err: *mut E) -> Id<E> {
     unsafe { Id::retain(err) }.expect("error parameter should be set if the method returns NULL")
 }
 
-impl<T: MsgSend, U: ?Sized + Message> MsgSendId<T, Id<U>> for New {
+impl<T: MsgSend, U: ?Sized + Message> MsgSendId<T, Option<Id<U>>> for New {
     #[inline]
-    unsafe fn send_message_id<A: ConvertArguments, R: MaybeUnwrap<Input = Id<U>>>(
+    unsafe fn send_message_id<A: ConvertArguments, R: MaybeUnwrap<Input = Option<Id<U>>>>(
         obj: T,
         sel: Sel,
         args: A,
@@ -96,18 +96,18 @@ impl<T: ?Sized + Message> MsgSendId<&'_ AnyClass, Allocated<T>> for Alloc {
         let obj = unsafe { MsgSend::send_message(cls, sel, args) };
         // SAFETY: The selector is `alloc`, so this has +1 retain count
         let obj = unsafe { Allocated::new(obj) };
-        R::maybe_unwrap::<Self>(obj, (cls, sel))
+        R::maybe_unwrap::<Self>(obj, ())
     }
 }
 
-impl<T: ?Sized + Message> MsgSendId<Option<Allocated<T>>, Id<T>> for Init {
+impl<T: ?Sized + Message> MsgSendId<Allocated<T>, Option<Id<T>>> for Init {
     #[inline]
-    unsafe fn send_message_id<A: ConvertArguments, R: MaybeUnwrap<Input = Id<T>>>(
-        obj: Option<Allocated<T>>,
+    unsafe fn send_message_id<A: ConvertArguments, R: MaybeUnwrap<Input = Option<Id<T>>>>(
+        obj: Allocated<T>,
         sel: Sel,
         args: A,
     ) -> R {
-        let ptr = Allocated::option_into_ptr(obj);
+        let ptr = Allocated::into_ptr(obj);
         // SAFETY: `ptr` may be null here, but that's fine since the return
         // is `*mut T`, which is one of the few types where messages to nil is
         // allowed.
@@ -121,9 +121,9 @@ impl<T: ?Sized + Message> MsgSendId<Option<Allocated<T>>, Id<T>> for Init {
     }
 }
 
-impl<T: MsgSend, U: ?Sized + Message> MsgSendId<T, Id<U>> for CopyOrMutCopy {
+impl<T: MsgSend, U: ?Sized + Message> MsgSendId<T, Option<Id<U>>> for CopyOrMutCopy {
     #[inline]
-    unsafe fn send_message_id<A: ConvertArguments, R: MaybeUnwrap<Input = Id<U>>>(
+    unsafe fn send_message_id<A: ConvertArguments, R: MaybeUnwrap<Input = Option<Id<U>>>>(
         obj: T,
         sel: Sel,
         args: A,
@@ -137,9 +137,9 @@ impl<T: MsgSend, U: ?Sized + Message> MsgSendId<T, Id<U>> for CopyOrMutCopy {
     }
 }
 
-impl<T: MsgSend, U: Message> MsgSendId<T, Id<U>> for Other {
+impl<T: MsgSend, U: Message> MsgSendId<T, Option<Id<U>>> for Other {
     #[inline]
-    unsafe fn send_message_id<A: ConvertArguments, R: MaybeUnwrap<Input = Id<U>>>(
+    unsafe fn send_message_id<A: ConvertArguments, R: MaybeUnwrap<Input = Option<Id<U>>>>(
         obj: T,
         sel: Sel,
         args: A,
@@ -163,11 +163,11 @@ impl<T: MsgSend, U: Message> MsgSendId<T, Id<U>> for Other {
 pub trait MaybeUnwrap {
     type Input;
     #[track_caller]
-    fn maybe_unwrap<'a, F: MsgSendIdFailed<'a>>(obj: Option<Self::Input>, args: F::Args) -> Self;
+    fn maybe_unwrap<'a, F: MsgSendIdFailed<'a>>(obj: Self::Input, args: F::Args) -> Self;
 }
 
 impl<T: ?Sized> MaybeUnwrap for Option<Id<T>> {
-    type Input = Id<T>;
+    type Input = Option<Id<T>>;
 
     #[inline]
     fn maybe_unwrap<'a, F: MsgSendIdFailed<'a>>(obj: Option<Id<T>>, _args: F::Args) -> Self {
@@ -176,7 +176,7 @@ impl<T: ?Sized> MaybeUnwrap for Option<Id<T>> {
 }
 
 impl<T: ?Sized> MaybeUnwrap for Id<T> {
-    type Input = Id<T>;
+    type Input = Option<Id<T>>;
 
     #[inline]
     fn maybe_unwrap<'a, F: MsgSendIdFailed<'a>>(obj: Option<Id<T>>, args: F::Args) -> Self {
@@ -187,24 +187,12 @@ impl<T: ?Sized> MaybeUnwrap for Id<T> {
     }
 }
 
-impl<T: ?Sized> MaybeUnwrap for Option<Allocated<T>> {
-    type Input = Allocated<T>;
-
-    #[inline]
-    fn maybe_unwrap<'a, F: MsgSendIdFailed<'a>>(obj: Option<Allocated<T>>, _args: F::Args) -> Self {
-        obj
-    }
-}
-
 impl<T: ?Sized> MaybeUnwrap for Allocated<T> {
     type Input = Allocated<T>;
 
     #[inline]
-    fn maybe_unwrap<'a, F: MsgSendIdFailed<'a>>(obj: Option<Allocated<T>>, args: F::Args) -> Self {
-        match obj {
-            Some(obj) => obj,
-            None => F::failed(args),
-        }
+    fn maybe_unwrap<'a, F: MsgSendIdFailed<'a>>(obj: Allocated<T>, _args: F::Args) -> Self {
+        obj
     }
 }
 
@@ -244,15 +232,11 @@ impl<'a> MsgSendIdFailed<'a> for New {
 }
 
 impl<'a> MsgSendIdFailed<'a> for Alloc {
-    type Args = (&'a AnyClass, Sel);
+    type Args = ();
 
     #[cold]
-    fn failed((cls, sel): Self::Args) -> ! {
-        if sel == sel!(alloc) {
-            panic!("failed allocating {cls}")
-        } else {
-            panic!("failed allocating with +[{cls} {sel}]")
-        }
+    fn failed(_: Self::Args) -> ! {
+        unreachable!()
     }
 }
 
@@ -385,18 +369,22 @@ mod tests {
         let mut expected = __ThreadTestData::current();
         let cls = __RcTestObject::class();
 
-        let obj: Option<Allocated<__RcTestObject>> = unsafe { msg_send_id![cls, alloc] };
+        let obj: Allocated<__RcTestObject> = unsafe { msg_send_id![cls, alloc] };
         expected.alloc += 1;
+        expected.assert_current();
+
         // Don't check allocation error
         let _obj: Id<__RcTestObject> = unsafe { msg_send_id![obj, init] };
         expected.init += 1;
         expected.assert_current();
 
-        let obj: Option<Allocated<__RcTestObject>> = unsafe { msg_send_id![cls, alloc] };
+        let obj: Allocated<__RcTestObject> = unsafe { msg_send_id![cls, alloc] };
         expected.alloc += 1;
+        expected.assert_current();
+
         // Check allocation error before init
-        let obj = obj.unwrap();
-        let _obj: Id<__RcTestObject> = unsafe { msg_send_id![Some(obj), init] };
+        assert!(!Allocated::as_ptr(&obj).is_null());
+        let _obj: Id<__RcTestObject> = unsafe { msg_send_id![obj, init] };
         expected.init += 1;
         expected.assert_current();
     }
@@ -474,7 +462,6 @@ mod tests {
     }
 
     #[test]
-    #[should_panic = "failed allocating with +[__RcTestObject allocReturningNull]"]
     fn test_alloc_with_null() {
         let _obj: Allocated<__RcTestObject> =
             unsafe { msg_send_id![__RcTestObject::class(), allocReturningNull] };
@@ -483,7 +470,7 @@ mod tests {
     #[test]
     #[should_panic = "failed initializing object with -initReturningNull"]
     fn test_init_with_null() {
-        let obj: Option<Allocated<__RcTestObject>> =
+        let obj: Allocated<__RcTestObject> =
             unsafe { msg_send_id![__RcTestObject::class(), alloc] };
         let _obj: Id<__RcTestObject> = unsafe { msg_send_id![obj, initReturningNull] };
     }
@@ -492,7 +479,8 @@ mod tests {
     #[should_panic = "failed allocating object"]
     #[cfg(not(debug_assertions))] // Does NULL receiver checks
     fn test_init_with_null_receiver() {
-        let obj: Option<Allocated<__RcTestObject>> = None;
+        let obj: Allocated<__RcTestObject> =
+            unsafe { msg_send_id![__RcTestObject::class(), allocReturningNull] };
         let _obj: Id<__RcTestObject> = unsafe { msg_send_id![obj, init] };
     }
 

crates/objc2/src/macros/extern_methods.rs

@@ -97,7 +97,7 @@
 ///         #[method_id(initWithVal:)]
 ///         // Arbitary self types are not stable, but we can work around it
 ///         // with the special name `this`.
-///         pub fn init(this: Option<Allocated<Self>>, val: usize) -> Id<Self>;
+///         pub fn init(this: Allocated<Self>, val: usize) -> Id<Self>;
 ///     }
 ///
 ///     /// Instance accessor methods.
@@ -150,7 +150,7 @@
 ///         unsafe { msg_send_id![Self::class(), new] }
 ///     }
 ///
-///     pub fn init(this: Option<Allocated<Self>>, val: usize) -> Id<Self> {
+///     pub fn init(this: Allocated<Self>, val: usize) -> Id<Self> {
 ///         unsafe { msg_send_id![this, initWithVal: val] }
 ///     }
 /// }

crates/objc2/src/macros/mod.rs

@@ -1081,12 +1081,11 @@ macro_rules! msg_send_bool {
 ///   return type is a generic `Id<T>` or `Option<Id<T>>`.
 ///
 /// - The `alloc` family: The receiver must be `&AnyClass`, and the return
-///   type is a generic `Allocated<T>` or `Option<Allocated<T>>`.
+///   type is a generic `Allocated<T>`.
 ///
-/// - The `init` family: The receiver must be `Option<Allocated<T>>` as
-///   returned from `alloc`. The receiver is consumed, and a the
-///   now-initialized `Id<T>` or `Option<Id<T>>` (with the same `T`) is
-///   returned.
+/// - The `init` family: The receiver must be `Allocated<T>` as returned from
+///   `alloc`. The receiver is consumed, and a the now-initialized `Id<T>` or
+///   `Option<Id<T>>` (with the same `T`) is returned.
 ///
 /// - The `copy` family: The receiver may be anything that implements
 ///   [`MessageReceiver`] and the return type is a generic `Id<T>` or
@@ -1276,7 +1275,7 @@ macro_rules! __msg_send_id_helper {
         ($($selector:tt)*)
         ($($argument:expr,)*)
     } => ({
-        <$crate::__macro_helpers::$retain_semantics as $crate::__macro_helpers::MsgSendId<_, _>>::$fn::<_, _>(
+        <$crate::__macro_helpers::$retain_semantics as $crate::__macro_helpers::MsgSendId<_, _>>::$fn(
             $obj,
             $crate::sel!($($selector)*),
             ($($argument,)*),
@@ -1296,7 +1295,7 @@ macro_rules! __msg_send_id_helper {
         let result;
         result = <$crate::__macro_helpers::RetainSemantics<{
             $crate::__macro_helpers::retain_semantics(__SELECTOR_DATA)
-        }> as $crate::__macro_helpers::MsgSendId<_, _>>::$fn::<_, _>(
+        }> as $crate::__macro_helpers::MsgSendId<_, _>>::$fn(
             $obj,
             $crate::__sel_inner!(
                 __SELECTOR_DATA,