Clean up BlockArguments trait

block2/CHANGELOG.md

@@ -6,6 +6,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## Unreleased - YYYY-MM-DD
 
+### Fixed
+* **BREAKING**: Cleaned up `BlockArguments` trait, it is now sealed and a
+  subtrait of `EncodeArguments`.
 
 ## 0.2.0-alpha.5 - 2022-07-19
 

block2/src/block.rs

@@ -5,30 +5,37 @@ use objc2_encode::{Encode, EncodeArguments, Encoding, RefEncode};
 
 use crate::ffi;
 
-/// Types that may be used as the arguments to an Objective-C block.
-pub trait BlockArguments: Sized {
-    /// Calls the given `Block` with self as the arguments.
-    ///
-    /// # Safety
-    ///
-    /// The given block must point to a valid `Block`.
-    ///
-    /// This invokes foreign code whose safety the user must guarantee.
-    unsafe fn call_block<R>(self, block: *mut Block<Self, R>) -> R;
+/// Types that may be used as the arguments of an Objective-C block.
+///
+/// This is implemented for tuples of up to 12 arguments, where each argument
+/// implements [`Encode`].
+///
+///
+/// # Safety
+///
+/// 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 {
+    /// Calls the given method the block and arguments.
+    #[doc(hidden)]
+    unsafe fn __call_block<R: Encode>(
+        invoke: unsafe extern "C" fn(),
+        block: *mut Block<Self, R>,
+        args: Self,
+    ) -> R;
 }
 
 macro_rules! block_args_impl {
-    ($($a:ident : $t:ident),*) => (
-        impl<$($t),*> BlockArguments for ($($t,)*) {
-            unsafe fn call_block<R>(self, block: *mut Block<Self, R>) -> R {
-                let layout = unsafe { block.cast::<ffi::Block_layout>().as_ref().unwrap_unchecked() };
-                // TODO: Can `invoke` actually be null?
-                let invoke: unsafe extern "C" fn() = layout.invoke.unwrap();
-                let invoke: unsafe extern "C" fn(*mut Block<Self, R>, $($t),*) -> R =
-                    unsafe { mem::transmute(invoke) }
-                ;
-                let ($($a,)*) = self;
-                unsafe { invoke(block, $($a),*) }
+    ($($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 {
+                // Very similar to `MessageArguments::__invoke`
+                let invoke: unsafe extern "C" fn(*mut Block<Self, R> $(, $t)*) -> R = unsafe {
+                    mem::transmute(invoke)
+                };
+
+                unsafe { invoke(block $(, $a)*) }
             }
         }
     );
@@ -86,11 +93,11 @@ pub struct Block<A, R> {
     _p: PhantomData<fn(A) -> R>,
 }
 
-unsafe impl<A: BlockArguments + EncodeArguments, R: Encode> RefEncode for Block<A, R> {
+unsafe impl<A: BlockArguments, R: Encode> RefEncode for Block<A, R> {
     const ENCODING_REF: Encoding<'static> = Encoding::Block;
 }
 
-impl<A: BlockArguments + EncodeArguments, R: Encode> Block<A, R> {
+impl<A: BlockArguments, R: Encode> Block<A, R> {
     /// Call self with the given arguments.
     ///
     /// # Safety
@@ -101,6 +108,11 @@ impl<A: BlockArguments + EncodeArguments, R: Encode> Block<A, R> {
     /// For example, if this block is shared with multiple references, the
     /// caller must ensure that calling it will not cause a data race.
     pub unsafe fn call(&self, args: A) -> R {
-        unsafe { args.call_block(self as *const Self as *mut Self) }
+        let ptr: *const Self = self;
+        let layout = unsafe { ptr.cast::<ffi::Block_layout>().as_ref().unwrap_unchecked() };
+        // TODO: Is `invoke` actually ever null?
+        let invoke = layout.invoke.unwrap();
+
+        unsafe { A::__call_block(invoke, ptr as *mut Self, args) }
     }
 }

block2/src/concrete_block.rs

@@ -5,12 +5,12 @@ use core::ops::Deref;
 use core::ptr;
 use std::os::raw::c_ulong;
 
-use objc2_encode::{Encode, EncodeArguments, Encoding, RefEncode};
+use objc2_encode::{Encode, Encoding, RefEncode};
 
 use crate::{ffi, Block, BlockArguments, RcBlock};
 
 /// Types that may be converted into a `ConcreteBlock`.
-pub trait IntoConcreteBlock<A: BlockArguments + EncodeArguments>: Sized {
+pub trait IntoConcreteBlock<A: BlockArguments>: Sized {
     /// The return type of the resulting `ConcreteBlock`.
     type Ret: Encode;
 
@@ -148,15 +148,13 @@ pub struct ConcreteBlock<A, R, F> {
     pub(crate) closure: F,
 }
 
-unsafe impl<A: BlockArguments + EncodeArguments, R: Encode, F> RefEncode
-    for ConcreteBlock<A, R, F>
-{
+unsafe impl<A: BlockArguments, R: Encode, F> RefEncode for ConcreteBlock<A, R, F> {
     const ENCODING_REF: Encoding<'static> = Encoding::Block;
 }
 
 impl<A, R, F> ConcreteBlock<A, R, F>
 where
-    A: BlockArguments + EncodeArguments,
+    A: BlockArguments,
     R: Encode,
     F: IntoConcreteBlock<A, Ret = R>,
 {

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, EncodeArguments};
+use objc2_encode::Encode;
 
 use super::{ffi, Block};
 use crate::BlockArguments;
@@ -34,13 +34,13 @@ pub struct GlobalBlock<A, R = ()> {
 
 unsafe impl<A, R> Sync for GlobalBlock<A, R>
 where
-    A: BlockArguments + EncodeArguments,
+    A: BlockArguments,
     R: Encode,
 {
 }
 unsafe impl<A, R> Send for GlobalBlock<A, R>
 where
-    A: BlockArguments + EncodeArguments,
+    A: BlockArguments,
     R: Encode,
 {
 }
@@ -77,7 +77,7 @@ impl<A, R> GlobalBlock<A, R> {
 
 impl<A, R> Deref for GlobalBlock<A, R>
 where
-    A: BlockArguments + EncodeArguments,
+    A: BlockArguments,
     R: Encode,
 {
     type Target = Block<A, R>;

test-ui/ui/global_block_not_encode.stderr

@@ -16,7 +16,7 @@ error[E0277]: the trait bound `Box<i32>: objc2_encode::encode::Encode` is not sa
             *mut c_void
             AtomicBool
           and 152 others
-  = note: required because of the requirements on the impl of `objc2_encode::encode::EncodeArguments` for `(Box<i32>,)`
+  = note: required because of the requirements on the impl of `BlockArguments` for `(Box<i32>,)`
   = note: required because of the requirements on the impl of `Sync` for `GlobalBlock<(Box<i32>,)>`
   = note: shared static variables must have a type that implements `Sync`
   = note: this error originates in the macro `global_block` (in Nightly builds, run with -Z macro-backtrace for more info)