Merge pull request #277 from madsmtm/out-parameters

Handling objects passed as "out parameters"

Cargo.toml

@@ -9,3 +9,6 @@ resolver = "2"
 inherits = "release"
 # Enable LTO to allow testing the `unstable-static-sel-inlined` feature
 lto = true
+# Don't emit unwind info; while important to get right, the control flow is
+# very hard to glean from assembly output.
+panic = "abort"

crates/block2/src/block.rs

@@ -1,7 +1,8 @@
 use core::marker::PhantomData;
 use core::mem;
 
-use objc2::encode::{Encode, EncodeArguments, Encoding, RefEncode};
+use objc2::encode::__unstable::EncodeReturn;
+use objc2::encode::{Encode, Encoding, RefEncode};
 
 use crate::ffi;
 
@@ -15,10 +16,10 @@ use crate::ffi;
 ///
 /// This is a sealed trait, and should not need to be implemented. Open an
 /// issue if you know a use-case where this restrition should be lifted!
-pub unsafe trait BlockArguments: EncodeArguments + Sized {
+pub unsafe trait BlockArguments: Sized {
     /// Calls the given method the block and arguments.
     #[doc(hidden)]
-    unsafe fn __call_block<R: Encode>(
+    unsafe fn __call_block<R: EncodeReturn>(
         invoke: unsafe extern "C" fn(),
         block: *mut Block<Self, R>,
         args: Self,
@@ -29,7 +30,11 @@ macro_rules! block_args_impl {
     ($($a:ident: $t:ident),*) => (
         unsafe impl<$($t: Encode),*> BlockArguments for ($($t,)*) {
             #[inline]
-            unsafe fn __call_block<R: Encode>(invoke: unsafe extern "C" fn(), block: *mut Block<Self, R>, ($($a,)*): Self) -> R {
+            unsafe fn __call_block<R: EncodeReturn>(
+                invoke: unsafe extern "C" fn(),
+                block: *mut Block<Self, R>,
+                ($($a,)*): Self,
+            ) -> R {
                 // Very similar to `MessageArguments::__invoke`
                 let invoke: unsafe extern "C" fn(*mut Block<Self, R> $(, $t)*) -> R = unsafe {
                     mem::transmute(invoke)
@@ -93,11 +98,11 @@ pub struct Block<A, R> {
     _p: PhantomData<fn(A) -> R>,
 }
 
-unsafe impl<A: BlockArguments, R: Encode> RefEncode for Block<A, R> {
+unsafe impl<A: BlockArguments, R: EncodeReturn> RefEncode for Block<A, R> {
     const ENCODING_REF: Encoding = Encoding::Block;
 }
 
-impl<A: BlockArguments, R: Encode> Block<A, R> {
+impl<A: BlockArguments, R: EncodeReturn> Block<A, R> {
     /// Call self with the given arguments.
     ///
     /// # Safety

crates/block2/src/concrete_block.rs

@@ -5,6 +5,7 @@ use core::ops::Deref;
 use core::ptr;
 use std::os::raw::c_ulong;
 
+use objc2::encode::__unstable::EncodeReturn;
 use objc2::encode::{Encode, Encoding, RefEncode};
 
 use crate::{ffi, Block, BlockArguments, RcBlock};
@@ -25,7 +26,7 @@ mod private {
 /// issue if you know a use-case where this restrition should be lifted!
 pub unsafe trait IntoConcreteBlock<A: BlockArguments>: private::Sealed<A> + Sized {
     /// The return type of the resulting `ConcreteBlock`.
-    type Output: Encode;
+    type Output: EncodeReturn;
 
     #[doc(hidden)]
     fn __into_concrete_block(self) -> ConcreteBlock<A, Self::Output, Self>;
@@ -36,12 +37,12 @@ macro_rules! concrete_block_impl {
         concrete_block_impl!($f,);
     );
     ($f:ident, $($a:ident : $t:ident),*) => (
-        impl<$($t: Encode,)* R: Encode, X> private::Sealed<($($t,)*)> for X
+        impl<$($t: Encode,)* R: EncodeReturn, X> private::Sealed<($($t,)*)> for X
         where
             X: Fn($($t,)*) -> R,
         {}
 
-        unsafe impl<$($t: Encode,)* R: Encode, X> IntoConcreteBlock<($($t,)*)> for X
+        unsafe impl<$($t: Encode,)* R: EncodeReturn, X> IntoConcreteBlock<($($t,)*)> for X
         where
             X: Fn($($t,)*) -> R,
         {
@@ -166,14 +167,14 @@ pub struct ConcreteBlock<A, R, F> {
     pub(crate) closure: F,
 }
 
-unsafe impl<A: BlockArguments, R: Encode, F> RefEncode for ConcreteBlock<A, R, F> {
+unsafe impl<A: BlockArguments, R: EncodeReturn, F> RefEncode for ConcreteBlock<A, R, F> {
     const ENCODING_REF: Encoding = Encoding::Block;
 }
 
 impl<A, R, F> ConcreteBlock<A, R, F>
 where
     A: BlockArguments,
-    R: Encode,
+    R: EncodeReturn,
     F: IntoConcreteBlock<A, Output = R>,
 {
     /// Constructs a `ConcreteBlock` with the given closure.

crates/block2/src/global.rs

@@ -5,7 +5,7 @@ use core::ops::Deref;
 use core::ptr;
 use std::os::raw::c_ulong;
 
-use objc2::encode::Encode;
+use objc2::encode::__unstable::EncodeReturn;
 
 use super::{ffi, Block};
 use crate::BlockArguments;
@@ -35,13 +35,13 @@ pub struct GlobalBlock<A, R = ()> {
 unsafe impl<A, R> Sync for GlobalBlock<A, R>
 where
     A: BlockArguments,
-    R: Encode,
+    R: EncodeReturn,
 {
 }
 unsafe impl<A, R> Send for GlobalBlock<A, R>
 where
     A: BlockArguments,
-    R: Encode,
+    R: EncodeReturn,
 {
 }
 
@@ -78,7 +78,7 @@ impl<A, R> GlobalBlock<A, R> {
 impl<A, R> Deref for GlobalBlock<A, R>
 where
     A: BlockArguments,
-    R: Encode,
+    R: EncodeReturn,
 {
     type Target = Block<A, R>;
 
@@ -94,7 +94,7 @@ where
 ///
 /// The syntax is similar to a static closure (except that all types have to
 /// be specified). Note that the block cannot capture its environment, and
-/// its argument types and return type must be [`Encode`].
+/// its argument types and return type must be [`EncodeReturn`].
 ///
 /// # Examples
 ///
@@ -130,7 +130,7 @@ where
 /// assert_eq!(x, 47);
 /// ```
 ///
-/// The following does not compile because [`Box`] is not [`Encode`]:
+/// The following does not compile because [`Box`] is not [`EncodeReturn`]:
 ///
 /// ```compile_fail
 /// use block2::global_block;

crates/header-translator/src/rust_type.rs

@@ -1722,27 +1722,27 @@ impl fmt::Display for Ty {
                     is_const: false,
                     pointee,
                 } => match &**pointee {
-                    // TODO: Re-enable once we can support it
-                    // Inner::Id {
-                    //     ty,
-                    //     is_const: false,
-                    //     lifetime: Lifetime::Autoreleasing,
-                    //     nullability: inner_nullability,
-                    // } if self.kind == TyKind::MethodArgument => {
-                    //     let tokens = if *inner_nullability == Nullability::NonNull {
-                    //         format!("Id<{ty}, Shared>")
-                    //     } else {
-                    //         format!("Option<Id<{ty}, Shared>>")
-                    //     };
-                    //     if *nullability == Nullability::NonNull {
-                    //         write!(f, "&mut {tokens}")
-                    //     } else {
-                    //         write!(f, "Option<&mut {tokens}>")
-                    //     }
-                    // }
-                    // Inner::Id { .. } => {
-                    //     unreachable!("there should be no id with other values: {self:?}")
-                    // }
+                    Inner::Id {
+                        ty,
+                        // Don't care about the const-ness of the id.
+                        is_const: _,
+                        lifetime: Lifetime::Autoreleasing,
+                        nullability: inner_nullability,
+                    } if self.kind == TyKind::MethodArgument => {
+                        let tokens = if *inner_nullability == Nullability::NonNull {
+                            format!("Id<{ty}, Shared>")
+                        } else {
+                            format!("Option<Id<{ty}, Shared>>")
+                        };
+                        if *nullability == Nullability::NonNull {
+                            write!(f, "&mut {tokens}")
+                        } else {
+                            write!(f, "Option<&mut {tokens}>")
+                        }
+                    }
+                    Inner::Id { .. } if self.kind == TyKind::MethodArgument => {
+                        unreachable!("invalid out-pointer id {self:?}")
+                    }
                     block @ Inner::Block { .. } => {
                         if *nullability == Nullability::NonNull {
                             write!(f, "&{block}")

crates/icrate/CHANGELOG.md

@@ -88,6 +88,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 * Removed a few instances of `TodoProtocols`.
 * Fixed type-encoding of a few `struct`s.
 * Fixed `NSProxy` trait methods.
+* **BREAKING**: Fixed type in methods that worked with out-parameters.
 
 
 ## icrate 0.0.1 - 2022-12-24

crates/objc2/CHANGELOG.md

@@ -17,8 +17,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   `Debug`, `Hash`, `PartialEq` and `Eq`.
 * Support running `Drop` impls on `dealloc` in `declare_class!`.
 * Added `declare::IvarEncode` and `declare::IvarBool` types.
-* Moved the `objc2_encode` traits to `objc2::encode` (API surface is
-  unchanged, since they were already exposed from that).
+* **BREAKING**: Moved the `objc2_encode` traits to `objc2::encode`.
+
+  This includes removing the `EncodeConvert` and `EncodeArguments` traits.
+* Added support for out-parameters like `&mut Id<_, _>` in `msg_send!`,
+  `msg_send_id!` and `extern_methods!`.
 
 ### Changed
 * **BREAKING**: Using the automatic `NSError**`-to-`Result` functionality in
@@ -113,6 +116,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 * Allow empty structs in `declare_class!` macro.
 * Allow using `extern_methods!` without the `ClassType` trait in scope.
 * Fixed a few small issues with `declare_class!`.
+* Fixed `()` being possible in argument position in `msg_send!`.
 
 
 ## 0.3.0-beta.4 - 2022-12-24

crates/objc2/src/__macro_helpers.rs

@@ -138,15 +138,15 @@ pub trait MsgSendId<T, U> {
         if let Some(res) = res {
             // In this case, the error is likely not created. If it is, it is
             // autoreleased anyhow, so it would be a waste to retain and
-            // release it.
+            // release it here.
             Ok(res)
         } else {
             // In this case, the error has very likely been created, but has
-            // been autoreleased (as is common for "out parameters"). Hence we
-            // need to retain it if we want it to live across autorelease
-            // pools.
+            // been autoreleased (as is common for "out parameters", see
+            // `src/rc/writeback.rs`). Hence we need to retain it if we want
+            // it to live across autorelease pools.
             //
-            // SAFETY: The closure `f` is guaranteed to populate the error
+            // SAFETY: The message send is guaranteed to populate the error
             // object, or leave it as NULL. The error is shared, and all
             // holders of the error know this, so is safe to retain.
             Err(unsafe { encountered_error(err) })

crates/objc2/src/declare.rs

@@ -124,7 +124,8 @@ use core::ptr;
 use core::ptr::NonNull;
 use std::ffi::CString;
 
-use crate::encode::{Encode, EncodeArguments, Encoding, RefEncode};
+use crate::encode::__unstable::{EncodeArguments, EncodeReturn};
+use crate::encode::{Encode, Encoding, RefEncode};
 use crate::ffi;
 use crate::rc::Allocated;
 use crate::runtime::{Bool, Class, Imp, Object, Protocol, Sel};
@@ -148,7 +149,7 @@ pub trait MethodImplementation: private::Sealed {
     /// The callee type of the method.
     type Callee: RefEncode + ?Sized;
     /// The return type of the method.
-    type Ret: Encode;
+    type Ret: EncodeReturn;
     /// The argument types of the method.
     type Args: EncodeArguments;
 
@@ -161,14 +162,14 @@ macro_rules! method_decl_impl {
         impl<$($l,)* T, $r, $($t),*> private::Sealed for $f
         where
             T: Message + ?Sized,
-            $r: Encode,
+            $r: EncodeReturn,
             $($t: Encode,)*
         {}
 
         impl<$($l,)* T, $r, $($t),*> MethodImplementation for $f
         where
             T: Message + ?Sized,
-            $r: Encode,
+            $r: EncodeReturn,
             $($t: Encode,)*
         {
             type Callee = T;
@@ -183,13 +184,13 @@ macro_rules! method_decl_impl {
     (@<$($l:lifetime),*> Class, $r:ident, $f:ty, $($t:ident),*) => {
         impl<$($l,)* $r, $($t),*> private::Sealed for $f
         where
-            $r: Encode,
+            $r: EncodeReturn,
             $($t: Encode,)*
         {}
 
         impl<$($l,)* $r, $($t),*> MethodImplementation for $f
         where
-            $r: Encode,
+            $r: EncodeReturn,
             $($t: Encode,)*
         {
             type Callee = Class;
@@ -408,7 +409,7 @@ impl ClassBuilder {
         F: MethodImplementation<Callee = T>,
     {
         let enc_args = F::Args::ENCODINGS;
-        let enc_ret = F::Ret::ENCODING;
+        let enc_ret = F::Ret::ENCODING_RETURN;
 
         let sel_args = sel.number_of_arguments();
         assert_eq!(
@@ -465,7 +466,7 @@ impl ClassBuilder {
         F: MethodImplementation<Callee = Class>,
     {
         let enc_args = F::Args::ENCODINGS;
-        let enc_ret = F::Ret::ENCODING;
+        let enc_ret = F::Ret::ENCODING_RETURN;
 
         let sel_args = sel.number_of_arguments();
         assert_eq!(
@@ -629,7 +630,7 @@ impl ProtocolBuilder {
         is_instance_method: bool,
     ) where
         Args: EncodeArguments,
-        Ret: Encode,
+        Ret: EncodeReturn,
     {
         let encs = Args::ENCODINGS;
         let sel_args = sel.number_of_arguments();
@@ -641,7 +642,7 @@ impl ProtocolBuilder {
             sel_args,
             encs.len(),
         );
-        let types = method_type_encoding(&Ret::ENCODING, encs);
+        let types = method_type_encoding(&Ret::ENCODING_RETURN, encs);
         unsafe {
             ffi::protocol_addMethodDescription(
                 self.as_mut_ptr(),
@@ -657,7 +658,7 @@ impl ProtocolBuilder {
     pub fn add_method_description<Args, Ret>(&mut self, sel: Sel, is_required: bool)
     where
         Args: EncodeArguments,
-        Ret: Encode,
+        Ret: EncodeReturn,
     {
         self.add_method_description_common::<Args, Ret>(sel, is_required, true)
     }
@@ -666,7 +667,7 @@ impl ProtocolBuilder {
     pub fn add_class_method_description<Args, Ret>(&mut self, sel: Sel, is_required: bool)
     where
         Args: EncodeArguments,
-        Ret: Encode,
+        Ret: EncodeReturn,
     {
         self.add_method_description_common::<Args, Ret>(sel, is_required, false)
     }