Make MessageReceiver simpler, and add internal trait MsgSend

There is value in having a simpler `MessageReceiver` which is understandable by end users, and that doesn't possibly incur extra costs depending on which arguments/return type you pass it.

Author: madsmtm

LAYERED_SAFETY.md

@@ -52,14 +52,14 @@ Doing the Rust equivalent of Objective-C's `NSUInteger hash_code = [obj hash];`.
 
 ```rust
 let obj: *const c_void = ...;
-let sel = unsafe { sel_registerName(b"hash\0".as_ptr() as *const c_char) };
-let fnptr = unsafe {
+let sel = unsafe { ffi::sel_registerName(b"hash\0".as_ptr() as *const c_char) };
+let msg_send_fn = unsafe {
     mem::transmute::<
-        extern "C" fn(*const c_void, SEL) -> NSUInteger,
-        extern "C" fn(),
-    >(objc_msgSend)
+        unsafe extern "C" fn(),
+        unsafe extern "C" fn(*const c_void, SEL) -> NSUInteger,
+    >(ffi::objc_msgSend)
 };
-let hash_code = unsafe { fnptr(obj, sel) };
+let hash_code = unsafe { msg_send_fn(obj, sel) };
 ```
 
 
@@ -68,8 +68,7 @@ let hash_code = unsafe { fnptr(obj, sel) };
 We can improve on this using [`MessageReceiver::send_message`], which
 abstracts away the calling convention details, as well as adding an `Encode`
 bound on all the involved types. This ensures that we don't accidentally try
-to pass e.g. a `Vec<T>`, which does not have a stable memory layout. It also
-handles details surrounding Objective-C's `BOOL` type.
+to pass e.g. a `Vec<T>`, which does not have a stable memory layout.
 
 Additionally, when `debug_assertions` are enabled, the types involved in the
 message send are compared to the types exposed in the Objective-C runtime.
@@ -97,7 +96,8 @@ let hash_code: NSUInteger = unsafe {
 
 Introducing macros: [`msg_send!`] can abstract away the tediousness of writing
 the selector expression, as well as ensuring that the number of arguments to
-the method is correct.
+the method is correct. It also handles details surrounding Objective-C's
+`BOOL` type.
 
 [`msg_send!`]: https://docs.rs/objc2/0.3.0-beta.4/objc2/macro.msg_send.html
 

crates/objc2/CHANGELOG.md

@@ -29,6 +29,19 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 * **BREAKING**: Moved the `MethodImplementation` trait from the `declare`
   module to the `runtime` module.
 * **BREAKING**: Moved the `MessageReceiver` trait to the `runtime` module.
+* **BREAKING**: Make the `MessageReceiver` trait no longer implemented for
+  references to `Id`. Dereference the `Id` yourself.
+
+  Note: Passing `&Id` in `msg_send!` is still supported.
+* **BREAKING**: `MessageReceiver::send_message` and
+  `MessageReceiver::send_super_message` now take `EncodeArguments` and return
+  `EncodeReturn`, instead of internal traits.
+
+  This is done to make `MessageReceiver` more straightforward to understand,
+  although it now also has slightly less functionality than `msg_send!`.
+
+  In particular automatic conversion of `bool` is not supported in
+  `MessageReceiver`.
 
 ### Deprecated
 * Soft deprecated using `msg_send!` without a comma between arguments (i.e.

crates/objc2/src/__macro_helpers/mod.rs

@@ -27,6 +27,7 @@ mod common_selectors;
 mod convert;
 mod declare_class;
 mod method_family;
+mod msg_send;
 mod msg_send_id;
 mod writeback;
 
@@ -40,6 +41,7 @@ pub use self::declare_class::{
 pub use self::method_family::{
     retain_semantics, Alloc, CopyOrMutCopy, Init, New, Other, RetainSemantics,
 };
+pub use self::msg_send::MsgSend;
 pub use self::msg_send_id::{MaybeUnwrap, MsgSendId};
 
 /// Helper struct for emitting the module info that macOS 32-bit requires.

crates/objc2/src/__macro_helpers/msg_send.rs

@@ -0,0 +1,203 @@
+use core::mem::ManuallyDrop;
+use core::ptr;
+
+use crate::encode::RefEncode;
+use crate::mutability::IsMutable;
+use crate::rc::Id;
+use crate::runtime::{AnyClass, AnyObject, MessageReceiver, Sel};
+use crate::{ClassType, Encode, Message};
+
+use super::{ConvertArguments, ConvertReturn, TupleExtender};
+
+pub trait MsgSend: Sized {
+    type Inner: ?Sized + RefEncode;
+
+    fn into_raw_receiver(self) -> *mut AnyObject;
+
+    #[inline]
+    #[track_caller]
+    unsafe fn send_message<A, R>(self, sel: Sel, args: A) -> R
+    where
+        A: ConvertArguments,
+        R: ConvertReturn,
+    {
+        let (args, stored) = A::__into_arguments(args);
+
+        // SAFETY: Upheld by caller
+        let result = unsafe { MessageReceiver::send_message(self.into_raw_receiver(), sel, args) };
+
+        // TODO: If we want `objc_retainAutoreleasedReturnValue` to
+        // work, we must not do any work before it has been run; so
+        // somehow, we should do that _before_ this call!
+        //
+        // SAFETY: The argument was passed to the message sending
+        // function, and the stored values are only processed this
+        // once. See `src/__macro_helpers/writeback.rs` for
+        // details.
+        unsafe { A::__process_after_message_send(stored) };
+
+        R::__from_return(result)
+    }
+
+    #[inline]
+    #[track_caller]
+    unsafe fn send_super_message<A, R>(self, superclass: &AnyClass, sel: Sel, args: A) -> R
+    where
+        A: ConvertArguments,
+        R: ConvertReturn,
+    {
+        let (args, stored) = A::__into_arguments(args);
+
+        // SAFETY: Upheld by caller
+        let result = unsafe {
+            MessageReceiver::send_super_message(self.into_raw_receiver(), superclass, sel, args)
+        };
+
+        // SAFETY: Same as in send_message above.
+        unsafe { A::__process_after_message_send(stored) };
+
+        R::__from_return(result)
+    }
+
+    #[inline]
+    #[track_caller]
+    unsafe fn send_super_message_static<A, R>(self, sel: Sel, args: A) -> R
+    where
+        Self::Inner: ClassType,
+        <Self::Inner as ClassType>::Super: ClassType,
+        A: ConvertArguments,
+        R: ConvertReturn,
+    {
+        unsafe { self.send_super_message(<Self::Inner as ClassType>::Super::class(), sel, args) }
+    }
+
+    // Error functions below. See MsgSendId::send_message_id_error for further
+    // details.
+    //
+    // Some of this could be abstracted away using closures, but that would
+    // interfere with `#[track_caller]`, so we avoid doing that.
+
+    #[inline]
+    #[track_caller]
+    unsafe fn send_message_error<A, E>(self, sel: Sel, args: A) -> Result<(), Id<E>>
+    where
+        *mut *mut E: Encode,
+        A: TupleExtender<*mut *mut E>,
+        <A as TupleExtender<*mut *mut E>>::PlusOneArgument: ConvertArguments,
+        E: Message,
+    {
+        let mut err: *mut E = ptr::null_mut();
+        let args = args.add_argument(&mut err);
+        let res: bool = unsafe { self.send_message(sel, args) };
+        if res {
+            Ok(())
+        } else {
+            Err(unsafe { encountered_error(err) })
+        }
+    }
+
+    #[inline]
+    #[track_caller]
+    unsafe fn send_super_message_error<A, E>(
+        self,
+        superclass: &AnyClass,
+        sel: Sel,
+        args: A,
+    ) -> Result<(), Id<E>>
+    where
+        *mut *mut E: Encode,
+        A: TupleExtender<*mut *mut E>,
+        <A as TupleExtender<*mut *mut E>>::PlusOneArgument: ConvertArguments,
+        E: Message,
+    {
+        let mut err: *mut E = ptr::null_mut();
+        let args = args.add_argument(&mut err);
+        let res: bool = unsafe { self.send_super_message(superclass, sel, args) };
+        if res {
+            Ok(())
+        } else {
+            Err(unsafe { encountered_error(err) })
+        }
+    }
+
+    #[inline]
+    #[track_caller]
+    unsafe fn send_super_message_static_error<A, E>(self, sel: Sel, args: A) -> Result<(), Id<E>>
+    where
+        Self::Inner: ClassType,
+        <Self::Inner as ClassType>::Super: ClassType,
+        *mut *mut E: Encode,
+        A: TupleExtender<*mut *mut E>,
+        <A as TupleExtender<*mut *mut E>>::PlusOneArgument: ConvertArguments,
+        E: Message,
+    {
+        let mut err: *mut E = ptr::null_mut();
+        let args = args.add_argument(&mut err);
+        let res: bool = unsafe { self.send_super_message_static(sel, args) };
+        if res {
+            Ok(())
+        } else {
+            Err(unsafe { encountered_error(err) })
+        }
+    }
+}
+
+#[cold]
+#[track_caller]
+unsafe fn encountered_error<E: Message>(err: *mut E) -> Id<E> {
+    // SAFETY: Ensured by caller
+    unsafe { Id::retain(err) }.expect("error parameter should be set if the method returns NO")
+}
+
+impl<T: ?Sized + MessageReceiver> MsgSend for T {
+    type Inner = T::__Inner;
+
+    #[inline]
+    fn into_raw_receiver(self) -> *mut AnyObject {
+        MessageReceiver::__as_raw_receiver(self)
+    }
+}
+
+impl<'a, T: ?Sized + Message> MsgSend for &'a Id<T> {
+    type Inner = T;
+
+    #[inline]
+    fn into_raw_receiver(self) -> *mut AnyObject {
+        (Id::as_ptr(self) as *mut T).cast()
+    }
+}
+
+impl<'a, T: ?Sized + Message + IsMutable> MsgSend for &'a mut Id<T> {
+    type Inner = T;
+
+    #[inline]
+    fn into_raw_receiver(self) -> *mut AnyObject {
+        Id::as_mut_ptr(self).cast()
+    }
+}
+
+impl<T: ?Sized + Message> MsgSend for ManuallyDrop<Id<T>> {
+    type Inner = T;
+
+    #[inline]
+    fn into_raw_receiver(self) -> *mut AnyObject {
+        Id::consume_as_ptr(ManuallyDrop::into_inner(self)).cast()
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::msg_send;
+    use crate::test_utils;
+
+    use super::*;
+
+    #[test]
+    fn test_send_message_manuallydrop() {
+        let obj = ManuallyDrop::new(test_utils::custom_object());
+        unsafe {
+            let _: () = msg_send![obj, release];
+        };
+        // `obj` is consumed, can't use here
+    }
+}

crates/objc2/src/__macro_helpers/msg_send_id.rs

@@ -2,10 +2,10 @@ use core::ptr;
 
 use crate::encode::Encode;
 use crate::rc::{Allocated, Id};
-use crate::runtime::{AnyClass, AnyObject, MessageReceiver, Sel};
+use crate::runtime::{AnyClass, AnyObject, Sel};
 use crate::{sel, Message};
 
-use super::{Alloc, ConvertArguments, CopyOrMutCopy, Init, New, Other, TupleExtender};
+use super::{Alloc, ConvertArguments, CopyOrMutCopy, Init, MsgSend, New, Other, TupleExtender};
 
 pub trait MsgSendId<T, U> {
     #[track_caller]
@@ -66,16 +66,16 @@ 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: MessageReceiver, U: ?Sized + Message> MsgSendId<T, Id<U>> for New {
+impl<T: MsgSend, U: ?Sized + Message> MsgSendId<T, Id<U>> for New {
     #[inline]
     unsafe fn send_message_id<A: ConvertArguments, R: MaybeUnwrap<Input = Id<U>>>(
         obj: T,
         sel: Sel,
         args: A,
     ) -> R {
-        let ptr = obj.__as_raw_receiver();
+        let ptr = obj.into_raw_receiver();
         // SAFETY: Checked by caller
-        let obj = unsafe { MessageReceiver::send_message(ptr, sel, args) };
+        let obj = unsafe { MsgSend::send_message(ptr, sel, args) };
         // SAFETY: The selector is `new`, so this has +1 retain count
         let obj = unsafe { Id::new(obj) };
 
@@ -93,7 +93,7 @@ impl<T: ?Sized + Message> MsgSendId<&'_ AnyClass, Allocated<T>> for Alloc {
         args: A,
     ) -> R {
         // SAFETY: Checked by caller
-        let obj = unsafe { MessageReceiver::send_message(cls, sel, args) };
+        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))
@@ -114,39 +114,39 @@ impl<T: ?Sized + Message> MsgSendId<Option<Allocated<T>>, Id<T>> for Init {
         //
         // We do this for efficiency, to avoid having a branch that the user
         // did not intend after every `alloc`.
-        let obj = unsafe { MessageReceiver::send_message(ptr, sel, args) };
+        let obj = unsafe { MsgSend::send_message(ptr, sel, args) };
         // SAFETY: The selector is `init`, so this has +1 retain count
         let obj = unsafe { Id::new(obj) };
         R::maybe_unwrap::<Self>(obj, (ptr.cast(), sel))
     }
 }
 
-impl<T: MessageReceiver, U: ?Sized + Message> MsgSendId<T, Id<U>> for CopyOrMutCopy {
+impl<T: MsgSend, U: ?Sized + Message> MsgSendId<T, Id<U>> for CopyOrMutCopy {
     #[inline]
     unsafe fn send_message_id<A: ConvertArguments, R: MaybeUnwrap<Input = Id<U>>>(
         obj: T,
         sel: Sel,
         args: A,
     ) -> R {
         // SAFETY: Checked by caller
-        let obj = unsafe { MessageReceiver::send_message(obj, sel, args) };
+        let obj = unsafe { MsgSend::send_message(obj, sel, args) };
         // SAFETY: The selector is `copy` or `mutableCopy`, so this has +1
         // retain count
         let obj = unsafe { Id::new(obj) };
         R::maybe_unwrap::<Self>(obj, ())
     }
 }
 
-impl<T: MessageReceiver, U: Message> MsgSendId<T, Id<U>> for Other {
+impl<T: MsgSend, U: Message> MsgSendId<T, Id<U>> for Other {
     #[inline]
     unsafe fn send_message_id<A: ConvertArguments, R: MaybeUnwrap<Input = Id<U>>>(
         obj: T,
         sel: Sel,
         args: A,
     ) -> R {
-        let ptr = obj.__as_raw_receiver();
+        let ptr = obj.into_raw_receiver();
         // SAFETY: Checked by caller
-        let obj = unsafe { MessageReceiver::send_message(ptr, sel, args) };
+        let obj = unsafe { MsgSend::send_message(ptr, sel, args) };
         // All code between the message send and the `retain_autoreleased`
         // must be able to be optimized away for this to work.
 

crates/objc2/src/macros/__method_msg_send.rs

@@ -1,4 +1,4 @@
-/// Forward selector and arguments to `MessageReceiver::send_message[_error]`.
+/// Forward selector and arguments to `MsgSend::send_message[_error]`.
 ///
 /// Note: We can't forward to `msg_send!` since that doesn't support selectors
 /// with space between.
@@ -115,7 +115,7 @@ macro_rules! __method_msg_send {
         $crate::__msg_send_helper! {
             ($receiver)
             // Use error method
-            (__send_message_error)
+            (send_message_error)
             ($($sel_parsed)* $sel :)
             ($($arg_parsed)*)
         }

crates/objc2/src/macros/__msg_send_parse.rs

@@ -140,7 +140,7 @@ macro_rules! __comma_between_args {
 #[cfg(feature = "unstable-msg-send-always-comma")]
 macro_rules! __comma_between_args {
     (
-        (__send_super_message_static)
+        (send_super_message_static)
         ($($args:tt)*)
         ($obj:expr)
     ) => {