Manual block encodings

crates/block2/src/rc_block.rs

@@ -101,8 +101,7 @@ impl<F: ?Sized> RcBlock<F> {
         R: EncodeReturn,
         Closure: IntoBlock<'f, A, R, Dyn = F>,
     {
-        // SAFETY: no encoding is given.
-        unsafe { Self::maybe_encoded::<_, _, _, NoBlockEncoding<A, R>>(closure) }
+        Self::maybe_encoded::<_, _, _, NoBlockEncoding<A, R>>(closure)
     }
 
     /// Constructs a new [`RcBlock`] with the given function and encoding
@@ -111,12 +110,6 @@ impl<F: ?Sized> RcBlock<F> {
     /// See [`StackBlock::with_encoding`] as to why and how this could be
     /// useful. The same requirements as [`Self::new`] apply here as well.
     ///
-    /// # Safety
-    ///
-    /// The raw encoding string given through `E` must be correct with respect
-    /// to the given closure's argument and return types: see
-    /// [`ManualBlockEncoding`].
-    ///
     /// # Example
     ///
     /// ```
@@ -125,32 +118,34 @@ impl<F: ?Sized> RcBlock<F> {
     /// # use objc2_foundation::NSError;
     /// #
     /// struct MyBlockEncoding;
+    /// // SAFETY: The encoding is correct.
     /// unsafe impl ManualBlockEncoding for MyBlockEncoding {
     ///     type Arguments = (*mut NSError,);
     ///     type Return = i32;
-    ///     const ENCODING_CSTR: &'static CStr = cr#"i16@?0@"NSError"8"#;
+    ///     const ENCODING_CSTR: &'static CStr = if cfg!(target_pointer_width = "64") {
+    ///         cr#"i16@?0@"NSError"8"#
+    ///     } else {
+    ///         cr#"i8@?0@"NSError"4"#
+    ///     };
     /// }
     ///
-    /// let my_block = unsafe {
-    ///     RcBlock::with_encoding::<_, _, _, MyBlockEncoding>(|_err: *mut NSError| {
-    ///         42i32
-    ///     })
-    /// };
+    /// let my_block = RcBlock::with_encoding::<_, _, _, MyBlockEncoding>(|_err: *mut NSError| {
+    ///     42i32
+    /// });
     /// assert_eq!(my_block.call((std::ptr::null_mut(),)), 42);
     /// ```
     #[inline]
-    pub unsafe fn with_encoding<'f, A, R, Closure, E>(closure: Closure) -> Self
+    pub fn with_encoding<'f, A, R, Closure, E>(closure: Closure) -> Self
     where
         A: EncodeArguments,
         R: EncodeReturn,
         Closure: IntoBlock<'f, A, R, Dyn = F>,
         E: ManualBlockEncoding<Arguments = A, Return = R>,
     {
-        // SAFETY: supposed to be upheld by the caller.
-        unsafe { Self::maybe_encoded::<_, _, _, UserSpecified<E>>(closure) }
+        Self::maybe_encoded::<_, _, _, UserSpecified<E>>(closure)
     }
 
-    unsafe fn maybe_encoded<'f, A, R, Closure, E>(closure: Closure) -> Self
+    fn maybe_encoded<'f, A, R, Closure, E>(closure: Closure) -> Self
     where
         A: EncodeArguments,
         R: EncodeReturn,
@@ -165,8 +160,6 @@ impl<F: ?Sized> RcBlock<F> {
         //
         // Clang doesn't do this optimization either.
         // <https://github.com/llvm/llvm-project/blob/llvmorg-17.0.6/clang/lib/CodeGen/CGBlocks.cpp#L281-L284>
-        //
-        // Encoding safety is supposed to be upheld by the caller.
         let block = unsafe { StackBlock::new_no_clone::<E>(closure) };
 
         // Transfer ownership from the stack to the heap.

crates/block2/src/stack.rs

@@ -162,8 +162,7 @@ where
     /// [`RcBlock::new`]: crate::RcBlock::new
     #[inline]
     pub fn new(closure: Closure) -> Self {
-        // SAFETY: no encoding is given.
-        unsafe { Self::maybe_encoded::<NoBlockEncoding<A, R>>(closure) }
+        Self::maybe_encoded::<NoBlockEncoding<A, R>>(closure)
     }
 
     /// Constructs a new [`StackBlock`] with the given function and encoding
@@ -190,6 +189,9 @@ where
     /// encoding information string themselves, thus obtaining a block
     /// containing it and working with these APIs.
     ///
+    /// You provide the encoding through the `E` type parameter, which shoul
+    /// implement [`ManualBlockEncoding`].
+    ///
     /// The same requirements as [`Self::new`] apply here as well.
     ///
     /// [`FileProvider`]: https://developer.apple.com/documentation/fileprovider?language=objc
@@ -200,12 +202,6 @@ where
     /// [`NEFilterDataProvider`]: https://developer.apple.com/documentation/networkextension/nefilterdataprovider?language=objc
     /// [`applySettings:completionHandler`]: https://developer.apple.com/documentation/networkextension/nefilterdataprovider/3181998-applysettings?language=objc
     ///
-    /// # Safety
-    ///
-    /// The raw encoding string given through `E` must be correct with respect
-    /// to the given closure's argument and return types: see
-    /// [`ManualBlockEncoding`].
-    ///
     /// # Example
     ///
     /// ```
@@ -214,30 +210,32 @@ where
     /// # use objc2_foundation::NSError;
     /// #
     /// struct MyBlockEncoding;
+    /// // SAFETY: The encoding is correct.
     /// unsafe impl ManualBlockEncoding for MyBlockEncoding {
     ///     type Arguments = (*mut NSError,);
     ///     type Return = i32;
-    ///     const ENCODING_CSTR: &'static CStr = cr#"i16@?0@"NSError"8"#;
+    ///     const ENCODING_CSTR: &'static CStr = if cfg!(target_pointer_width = "64") {
+    ///         cr#"i16@?0@"NSError"8"#
+    ///     } else {
+    ///         cr#"i8@?0@"NSError"4"#
+    ///     };
     /// }
     ///
-    /// let my_block = unsafe {
-    ///     StackBlock::with_encoding::<MyBlockEncoding>(|_err: *mut NSError| {
-    ///         42i32
-    ///     })
-    /// };
+    /// let my_block = StackBlock::with_encoding::<MyBlockEncoding>(|_err: *mut NSError| {
+    ///     42i32
+    /// });
     /// assert_eq!(my_block.call((std::ptr::null_mut(),)), 42);
     /// ```
     #[inline]
-    pub unsafe fn with_encoding<E>(closure: Closure) -> Self
+    pub fn with_encoding<E>(closure: Closure) -> Self
     where
         E: ManualBlockEncoding<Arguments = A, Return = R>,
     {
-        // SAFETY: supposed to be upheld by the caller.
-        unsafe { Self::maybe_encoded::<UserSpecified<E>>(closure) }
+        Self::maybe_encoded::<UserSpecified<E>>(closure)
     }
 
     #[inline]
-    unsafe fn maybe_encoded<E>(closure: Closure) -> Self
+    fn maybe_encoded<E>(closure: Closure) -> Self
     where
         E: ManualBlockEncodingExt<Arguments = A, Return = R>,
     {
@@ -310,9 +308,7 @@ impl<'f, A, R, Closure> StackBlock<'f, A, R, Closure> {
 
     /// # Safety
     ///
-    ///  * `_Block_copy` must be called on the resulting stack block only once.
-    ///  * Encoding must be correct with respect to the given function's input
-    ///    and output types: see [`ManualBlockEncoding`].
+    ///  `_Block_copy` must be called on the resulting stack block only once.
     #[inline]
     pub(crate) unsafe fn new_no_clone<E>(closure: Closure) -> Self
     where
@@ -335,13 +331,20 @@ impl<'f, A, R, Closure> StackBlock<'f, A, R, Closure> {
         };
         // See discussion in `new` above with regards to the safety of the
         // pointer to the descriptor.
-        let descriptor = if mem::needs_drop::<Self>() {
-            if E::IS_NONE {
+        let descriptor = match (mem::needs_drop::<Self>(), E::IS_NONE) {
+            (true, true) => {
                 // SAFETY: see above.
                 BlockDescriptorPtr {
                     with_copy_dispose: &Self::DESCRIPTOR_WITH_DROP,
                 }
-            } else {
+            }
+            (false, true) => {
+                // SAFETY: see above.
+                BlockDescriptorPtr {
+                    basic: &Self::DESCRIPTOR_BASIC,
+                }
+            }
+            (true, false) => {
                 // SAFETY: see above; the value is already a similar constant,
                 // so promotion can be guaranteed as well here.
                 BlockDescriptorPtr {
@@ -352,19 +355,16 @@ impl<'f, A, R, Closure> StackBlock<'f, A, R, Closure> {
                         &<Self as EncodedDescriptors<E>>::DESCRIPTOR_WITH_DROP_AND_ENCODING,
                 }
             }
-        } else if E::IS_NONE {
-            // SAFETY: see above.
-            BlockDescriptorPtr {
-                basic: &Self::DESCRIPTOR_BASIC,
-            }
-        } else {
-            // SAFETY: see above; the value is already a similar constant,
-            // so promotion can be guaranteed as well here.
-            BlockDescriptorPtr {
-                // TODO: move to a `const fn` defined next to the partially-
-                // copied constant and called here in an inline `const` when
-                // the MSRV is at least 1.79.
-                with_signature: &<Self as EncodedDescriptors<E>>::DESCRIPTOR_BASIC_WITH_ENCODING,
+            (false, false) => {
+                // SAFETY: see above; the value is already a similar constant,
+                // so promotion can be guaranteed as well here.
+                BlockDescriptorPtr {
+                    // TODO: move to a `const fn` defined next to the partially-
+                    // copied constant and called here in an inline `const` when
+                    // the MSRV is at least 1.79.
+                    with_signature:
+                        &<Self as EncodedDescriptors<E>>::DESCRIPTOR_BASIC_WITH_ENCODING,
+                }
             }
         };
 

crates/block2/src/traits.rs

@@ -158,6 +158,8 @@ impl_traits!(t0: T0, t1: T1, t2: T2, t3: T3, t4: T4, t5: T5, t6: T6, t7: T7, t8:
 ///    is put to the forefront for them;
 ///  * reading a block encoding string is tough when not initiated, so these
 ///    also serve as self-documentation;
+///  * the safety validation can be moved to the trait implementation, so that
+///    the use can be marked safe.
 ///
 /// [`RcBlock::with_encoding`]: crate::RcBlock::with_encoding
 ///
@@ -256,6 +258,8 @@ where
     _r: PhantomData<R>,
 }
 
+// SAFETY: The encoding here is incorrect, but it will never be used because
+// we specify `IS_NONE = true` in `ManualBlockEncodingExt`.
 unsafe impl<A, R> ManualBlockEncoding for NoBlockEncoding<A, R>
 where
     A: EncodeArguments,

crates/test-ui/ui/block_lifetimes_independent.rs

@@ -34,7 +34,7 @@ fn args_with_encoding<'a, 'b>(
         type Return = ();
         const ENCODING_CSTR: &'static CStr = c"v24@?0^i8^i16";
     }
-    unsafe { RcBlock::with_encoding::<_, _, _, Enc<'a, 'b>>(f) }
+    RcBlock::with_encoding::<_, _, _, Enc<'a, 'b>>(f)
 }
 
 fn args_return_with_encoding<'a, 'b>(
@@ -46,7 +46,7 @@ fn args_return_with_encoding<'a, 'b>(
         type Return = &'b i32;
         const ENCODING_CSTR: &'static CStr = c"^i816@?0^i8";
     }
-    unsafe { RcBlock::with_encoding::<_, _, _, Enc<'a, 'b>>(f) }
+    RcBlock::with_encoding::<_, _, _, Enc<'a, 'b>>(f)
 }
 
 fn args_entire_with_encoding<'a, 'b>(f: impl Fn(&'a i32) + 'b) -> RcBlock<dyn Fn(&'b i32) + 'a> {
@@ -56,7 +56,7 @@ fn args_entire_with_encoding<'a, 'b>(f: impl Fn(&'a i32) + 'b) -> RcBlock<dyn Fn
         type Return = ();
         const ENCODING_CSTR: &'static CStr = c"v16@?0^i8";
     }
-    unsafe { RcBlock::with_encoding::<_, _, _, Enc<'a>>(f) }
+    RcBlock::with_encoding::<_, _, _, Enc<'a>>(f)
 }
 
 fn return_entire_with_encoding<'a, 'b>(
@@ -68,7 +68,7 @@ fn return_entire_with_encoding<'a, 'b>(
         type Return = &'a i32;
         const ENCODING_CSTR: &'static CStr = c"^i8@?0";
     }
-    unsafe { RcBlock::with_encoding::<_, _, _, Enc<'a>>(f) }
+    RcBlock::with_encoding::<_, _, _, Enc<'a>>(f)
 }
 
 fn main() {}

crates/test-ui/ui/lifetime_of_closure_tied_to_block.rs

@@ -21,12 +21,12 @@ fn main() {
 
     let _ = {
         let x = 2;
-        unsafe { RcBlock::with_encoding::<_, _, _, VoidToI32>(|| x + 2) }
+        RcBlock::with_encoding::<_, _, _, VoidToI32>(|| x + 2)
     };
 
     let _ = {
         let x = 2;
-        unsafe { RcBlock::with_encoding::<_, _, _, VoidToI32>(|| x + 2).clone() }
+        RcBlock::with_encoding::<_, _, _, VoidToI32>(|| x + 2).clone()
     };
 
     let _ = {
@@ -41,11 +41,11 @@ fn main() {
 
     let _ = {
         let x = 2;
-        unsafe { StackBlock::with_encoding::<VoidToI32>(|| x + 2) }
+        StackBlock::with_encoding::<VoidToI32>(|| x + 2)
     };
 
     let _ = {
         let x = 2;
-        unsafe { StackBlock::with_encoding::<VoidToI32>(|| x + 2).copy() }
+        StackBlock::with_encoding::<VoidToI32>(|| x + 2).copy()
     };
 }