Merge pull request #241 from madsmtm/block-traits

Clean up block traits

block2/CHANGELOG.md

@@ -6,6 +6,11 @@ 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`.
+* **BREAKING**: Cleaned up `IntoConcreteBlock` trait, it is now sealed and the
+  associated output type has been renamed to `Output`.
 
 ## 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,31 +5,49 @@ 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 {
+mod private {
+    pub trait Sealed<A> {}
+}
+
+/// Types that may be converted into a [`ConcreteBlock`].
+///
+/// This is implemented for [`Fn`] closures of up to 12 arguments, where each
+/// argument and the return type 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 IntoConcreteBlock<A: BlockArguments>: private::Sealed<A> + Sized {
     /// The return type of the resulting `ConcreteBlock`.
-    type Ret: Encode;
+    type Output: Encode;
 
-    /// Consumes self to create a `ConcreteBlock`.
-    fn into_concrete_block(self) -> ConcreteBlock<A, Self::Ret, Self>;
+    #[doc(hidden)]
+    fn __into_concrete_block(self) -> ConcreteBlock<A, Self::Output, Self>;
 }
 
 macro_rules! concrete_block_impl {
     ($f:ident) => (
         concrete_block_impl!($f,);
     );
     ($f:ident, $($a:ident : $t:ident),*) => (
-        impl<$($t: Encode,)* R: Encode, X> IntoConcreteBlock<($($t,)*)> for X
+        impl<$($t: Encode,)* R: Encode, X> private::Sealed<($($t,)*)> for X
+        where
+            X: Fn($($t,)*) -> R,
+        {}
+
+        unsafe impl<$($t: Encode,)* R: Encode, X> IntoConcreteBlock<($($t,)*)> for X
         where
             X: Fn($($t,)*) -> R,
         {
-            type Ret = R;
+            type Output = R;
 
-            fn into_concrete_block(self) -> ConcreteBlock<($($t,)*), R, X> {
+            fn __into_concrete_block(self) -> ConcreteBlock<($($t,)*), R, X> {
                 extern "C" fn $f<$($t,)* R, X>(
                     block: &ConcreteBlock<($($t,)*), R, X>,
                     $($a: $t,)*
@@ -148,23 +166,21 @@ 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>,
+    F: IntoConcreteBlock<A, Output = R>,
 {
     /// Constructs a `ConcreteBlock` with the given closure.
     /// When the block is called, it will return the value that results from
     /// calling the closure.
     pub fn new(closure: F) -> Self {
-        closure.into_concrete_block()
+        closure.__into_concrete_block()
     }
 }
 

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)