From 5387cab953b72781e74422ab239d302d5bb57c4c Mon Sep 17 00:00:00 2001 From: Paul Mabileau Date: Fri, 5 Jul 2024 19:18:55 +0200 Subject: [PATCH 01/14] Feat+Refactor(block2/abi): Add new utils on `BlockFlags` Signed-off-by: Paul Mabileau --- crates/block2/src/abi.rs | 49 ++++++++++++++++++++++++++++++++----- crates/block2/src/debug.rs | 4 +-- crates/block2/src/global.rs | 3 +-- 3 files changed, 46 insertions(+), 10 deletions(-) diff --git a/crates/block2/src/abi.rs b/crates/block2/src/abi.rs index 919bf8515..888e4772c 100644 --- a/crates/block2/src/abi.rs +++ b/crates/block2/src/abi.rs @@ -8,6 +8,7 @@ use core::ffi::{c_char, c_int, c_ulong, c_void}; use core::fmt; use core::mem::MaybeUninit; +use core::ops::{BitAnd, BitOr}; use alloc::format; @@ -81,6 +82,45 @@ impl BlockFlags { /// Note: Not public ABI. const BLOCK_HAS_EXTENDED_LAYOUT: Self = Self(1 << 31); + + /// `const` version of [`PartialEq`]. + pub(crate) const fn equals(self, other: Self) -> bool { + self.0 == other.0 + } + + /// `const` version of [`BitOr`]: adds the flags together. + pub(crate) const fn union(self, other: Self) -> Self { + Self(self.0 | other.0) + } + + /// `const` version of [`BitAnd`]: only keeps the common flags. + pub(crate) const fn intersect(self, other: Self) -> Self { + Self(self.0 & other.0) + } + + /// Returns `true` if and only if all the flags from `other` are enabled, + /// i.e. `self & other == other`. + pub(crate) const fn has(self, other: Self) -> bool { + self.intersect(other).equals(other) + } +} + +/// See [`BlockFlags::union`]. +impl BitOr for BlockFlags { + type Output = Self; + + fn bitor(self, other: Self) -> Self { + self.union(other) + } +} + +/// See [`BlockFlags::intersect`]. +impl BitAnd for BlockFlags { + type Output = Self; + + fn bitand(self, other: Self) -> Self { + self.intersect(other) + } } impl fmt::Debug for BlockFlags { @@ -94,7 +134,7 @@ impl fmt::Debug for BlockFlags { $name:ident: $flag:ident );* $(;)?} => ($( $(#[$m])? - f.field(stringify!($name), &(self.0 & Self::$flag.0 != 0)); + f.field(stringify!($name), &self.has(Self::$flag)); )*) } test_flags! { @@ -119,13 +159,10 @@ impl fmt::Debug for BlockFlags { has_extended_layout: BLOCK_HAS_EXTENDED_LAYOUT; } - f.field( - "over_referenced", - &(self.0 & Self::BLOCK_REFCOUNT_MASK.0 == Self::BLOCK_REFCOUNT_MASK.0), - ); + f.field("over_referenced", &self.has(Self::BLOCK_REFCOUNT_MASK)); f.field( "reference_count", - &((self.0 & Self::BLOCK_REFCOUNT_MASK.0) >> 1), + &((*self & Self::BLOCK_REFCOUNT_MASK).0 >> 1), ); f.finish_non_exhaustive() diff --git a/crates/block2/src/debug.rs b/crates/block2/src/debug.rs index 3031ac78d..967359f60 100644 --- a/crates/block2/src/debug.rs +++ b/crates/block2/src/debug.rs @@ -43,8 +43,8 @@ pub(crate) fn debug_block_header(header: &BlockHeader, f: &mut DebugStruct<'_, ' f.field( "descriptor", &BlockDescriptorHelper { - has_copy_dispose: header.flags.0 & BlockFlags::BLOCK_HAS_COPY_DISPOSE.0 != 0, - has_signature: header.flags.0 & BlockFlags::BLOCK_HAS_SIGNATURE.0 != 0, + has_copy_dispose: header.flags.has(BlockFlags::BLOCK_HAS_COPY_DISPOSE), + has_signature: header.flags.has(BlockFlags::BLOCK_HAS_SIGNATURE), descriptor: header.descriptor, }, ); diff --git a/crates/block2/src/global.rs b/crates/block2/src/global.rs index 28aa5bb48..73b0b8c1c 100644 --- a/crates/block2/src/global.rs +++ b/crates/block2/src/global.rs @@ -44,8 +44,7 @@ unsafe impl Send for GlobalBlock {} // triggers an error. impl GlobalBlock { // TODO: Use new ABI with BLOCK_HAS_SIGNATURE - const FLAGS: BlockFlags = - BlockFlags(BlockFlags::BLOCK_IS_GLOBAL.0 | BlockFlags::BLOCK_USE_STRET.0); + const FLAGS: BlockFlags = BlockFlags::BLOCK_IS_GLOBAL.union(BlockFlags::BLOCK_USE_STRET); #[doc(hidden)] #[allow(clippy::declare_interior_mutable_const)] From 56fc5fcaf2be2ee9035a62802ab1bcda365f0be3 Mon Sep 17 00:00:00 2001 From: Paul Mabileau Date: Tue, 16 Jul 2024 20:29:38 +0200 Subject: [PATCH 02/14] Feat(block2): Add new `with_encoding` unsafe block constructors Signed-off-by: Paul Mabileau --- Cargo.lock | 1 + crates/block2/Cargo.toml | 5 + crates/block2/src/lib.rs | 2 +- crates/block2/src/rc_block.rs | 65 +++++++++- crates/block2/src/stack.rs | 223 +++++++++++++++++++++++++++++++--- crates/block2/src/traits.rs | 221 +++++++++++++++++++++++++++++++++ 6 files changed, 495 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4ecb6152b..1dbdd56a8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -75,6 +75,7 @@ name = "block2" version = "0.5.1" dependencies = [ "objc2", + "objc2-foundation", ] [[package]] diff --git a/crates/block2/Cargo.toml b/crates/block2/Cargo.toml index c142964f5..56651ee6e 100644 --- a/crates/block2/Cargo.toml +++ b/crates/block2/Cargo.toml @@ -47,6 +47,11 @@ unstable-private = [] [dependencies] objc2 = { path = "../objc2", version = "0.5.2", default-features = false } +[dev-dependencies.objc2-foundation] +path = "../../framework-crates/objc2-foundation" +default-features = false +features = ["NSError"] + [package.metadata.docs.rs] default-target = "aarch64-apple-darwin" features = ["unstable-private"] diff --git a/crates/block2/src/lib.rs b/crates/block2/src/lib.rs index a208c2617..4de5f021d 100644 --- a/crates/block2/src/lib.rs +++ b/crates/block2/src/lib.rs @@ -378,7 +378,7 @@ pub use self::block::Block; pub use self::global::GlobalBlock; pub use self::rc_block::RcBlock; pub use self::stack::StackBlock; -pub use self::traits::{BlockFn, IntoBlock}; +pub use self::traits::{BlockFn, IntoBlock, ManualBlockEncoding}; /// Deprecated alias for a `'static` `StackBlock`. #[deprecated = "renamed to `StackBlock`"] diff --git a/crates/block2/src/rc_block.rs b/crates/block2/src/rc_block.rs index b24323ad7..170049aa6 100644 --- a/crates/block2/src/rc_block.rs +++ b/crates/block2/src/rc_block.rs @@ -7,6 +7,7 @@ use objc2::encode::{EncodeArguments, EncodeReturn}; use crate::abi::BlockHeader; use crate::debug::debug_block_header; +use crate::traits::{ManualBlockEncoding, ManualBlockEncodingExt, NoBlockEncoding, UserSpecified}; use crate::{ffi, Block, IntoBlock, StackBlock}; /// A reference-counted Objective-C block that is stored on the heap. @@ -91,14 +92,70 @@ impl RcBlock { /// /// When the block is called, it will return the value that results from /// calling the closure. - // // Note: Unsure if this should be #[inline], but I think it may be able to - // benefit from not being so. + // benefit from not being completely so. + #[inline] pub fn new<'f, A, R, Closure>(closure: Closure) -> Self where A: EncodeArguments, R: EncodeReturn, Closure: IntoBlock<'f, A, R, Dyn = F>, + { + // SAFETY: no encoding is given. + unsafe { Self::maybe_encoded::<_, _, _, NoBlockEncoding>(closure) } + } + + /// Constructs a new [`RcBlock`] with the given function and encoding + /// information. + /// + /// 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 + /// + /// ``` + /// # use std::ffi::CStr; + /// # use block2::{Block, ManualBlockEncoding, RcBlock}; + /// # use objc2_foundation::NSError; + /// # + /// struct MyBlockEncoding; + /// unsafe impl ManualBlockEncoding for MyBlockEncoding { + /// type Arguments = (*mut NSError,); + /// type Return = i32; + /// const ENCODING_CSTR: &'static CStr = cr#"i16@?0@"NSError"8"#; + /// } + /// + /// let my_block = unsafe { + /// 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 + where + A: EncodeArguments, + R: EncodeReturn, + Closure: IntoBlock<'f, A, R, Dyn = F>, + E: ManualBlockEncoding, + { + // SAFETY: supposed to be upheld by the caller. + unsafe { Self::maybe_encoded::<_, _, _, UserSpecified>(closure) } + } + + unsafe fn maybe_encoded<'f, A, R, Closure, E>(closure: Closure) -> Self + where + A: EncodeArguments, + R: EncodeReturn, + Closure: IntoBlock<'f, A, R, Dyn = F>, + E: ManualBlockEncodingExt, { // SAFETY: The stack block is copied once below. // @@ -108,7 +165,9 @@ impl RcBlock { // // Clang doesn't do this optimization either. // - let block = unsafe { StackBlock::new_no_clone(closure) }; + // + // Encoding safety is supposed to be upheld by the caller. + let block = unsafe { StackBlock::new_no_clone::(closure) }; // Transfer ownership from the stack to the heap. let mut block = ManuallyDrop::new(block); diff --git a/crates/block2/src/stack.rs b/crates/block2/src/stack.rs index b7b92f66a..cc8a01a68 100644 --- a/crates/block2/src/stack.rs +++ b/crates/block2/src/stack.rs @@ -10,9 +10,11 @@ use core::ptr::{self, NonNull}; use objc2::encode::{EncodeArguments, EncodeReturn, Encoding, RefEncode}; use crate::abi::{ - BlockDescriptor, BlockDescriptorCopyDispose, BlockDescriptorPtr, BlockFlags, BlockHeader, + BlockDescriptor, BlockDescriptorCopyDispose, BlockDescriptorCopyDisposeSignature, + BlockDescriptorPtr, BlockDescriptorSignature, BlockFlags, BlockHeader, }; use crate::debug::debug_block_header; +use crate::traits::{ManualBlockEncoding, ManualBlockEncodingExt, NoBlockEncoding, UserSpecified}; use crate::{ffi, Block, IntoBlock}; /// An Objective-C block constructed on the stack. @@ -142,7 +144,12 @@ impl<'f, A, R, Closure: Clone> StackBlock<'f, A, R, Closure> { }; } -impl<'f, A, R, Closure> StackBlock<'f, A, R, Closure> { +impl<'f, A, R, Closure> StackBlock<'f, A, R, Closure> +where + A: EncodeArguments, + R: EncodeReturn, + Closure: IntoBlock<'f, A, R> + Clone, +{ /// Construct a `StackBlock` with the given closure. /// /// Note that this requires [`Clone`], as a C block is generally assumed @@ -154,22 +161,100 @@ impl<'f, A, R, Closure> StackBlock<'f, A, R, Closure> { /// /// [`RcBlock::new`]: crate::RcBlock::new #[inline] - pub fn new(closure: Closure) -> Self + pub fn new(closure: Closure) -> Self { + // SAFETY: no encoding is given. + unsafe { Self::maybe_encoded::>(closure) } + } + + /// Constructs a new [`StackBlock`] with the given function and encoding + /// information. + /// + /// Some particular newer-ish Apple Objective-C APIs expect the block they + /// are given to be created with encoding information set in the block + /// object itself and crash the calling process if they fail to find it, + /// which renders them pretty much unusable with only [`Self::new`] that + /// currently does not set such encoding information. This is for example + /// the case in [`FileProvider`] for [`NSFileProviderManager`]'s + /// [`reimportItemsBelowItemWithIdentifier:completionHandler:`] and + /// [`waitForStabilizationWithCompletionHandler:`], but also in + /// [`NetworkExtension`] for [`NEFilterDataProvider`]'s + /// [`applySettings:completionHandler`]. A complete list of such APIs may + /// not be easily obtained, though. + /// + /// This encoding information string could technically be generated at + /// compile time using the generic parameters already available to + /// [`Self::new`]. However, doing so would require some constant evaluation + /// features that are yet to be implemented and stabilized in the Rust + /// compiler. This function is therefore exposed in the meantime so users + /// may still be able to call the concerned APIs by providing the raw + /// encoding information string themselves, thus obtaining a block + /// containing it and working with these APIs. + /// + /// The same requirements as [`Self::new`] apply here as well. + /// + /// [`FileProvider`]: https://developer.apple.com/documentation/fileprovider?language=objc + /// [`NSFileProviderManager`]: https://developer.apple.com/documentation/fileprovider/nsfileprovidermanager?language=objc + /// [`reimportItemsBelowItemWithIdentifier:completionHandler:`]: https://developer.apple.com/documentation/fileprovider/nsfileprovidermanager/reimportitems(below:completionhandler:)?language=objc + /// [`waitForStabilizationWithCompletionHandler:`]: https://developer.apple.com/documentation/fileprovider/nsfileprovidermanager/waitforstabilization(completionhandler:)?language=objc + /// [`NetworkExtension`]: https://developer.apple.com/documentation/networkextension?language=objc + /// [`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 + /// + /// ``` + /// # use std::ffi::CStr; + /// # use block2::{Block, ManualBlockEncoding, StackBlock}; + /// # use objc2_foundation::NSError; + /// # + /// struct MyBlockEncoding; + /// unsafe impl ManualBlockEncoding for MyBlockEncoding { + /// type Arguments = (*mut NSError,); + /// type Return = i32; + /// const ENCODING_CSTR: &'static CStr = cr#"i16@?0@"NSError"8"#; + /// } + /// + /// let my_block = unsafe { + /// StackBlock::with_encoding::(|_err: *mut NSError| { + /// 42i32 + /// }) + /// }; + /// assert_eq!(my_block.call((std::ptr::null_mut(),)), 42); + /// ``` + #[inline] + pub unsafe fn with_encoding(closure: Closure) -> Self where - A: EncodeArguments, - R: EncodeReturn, - Closure: IntoBlock<'f, A, R> + Clone, + E: ManualBlockEncoding, + { + // SAFETY: supposed to be upheld by the caller. + unsafe { Self::maybe_encoded::>(closure) } + } + + #[inline] + unsafe fn maybe_encoded(closure: Closure) -> Self + where + E: ManualBlockEncodingExt, { let header = BlockHeader { #[allow(unused_unsafe)] isa: unsafe { ptr::addr_of!(ffi::_NSConcreteStackBlock) }, - // TODO: Add signature. - flags: BlockFlags::BLOCK_HAS_COPY_DISPOSE, + flags: BlockFlags::BLOCK_HAS_COPY_DISPOSE + | if !E::IS_NONE { + BlockFlags::BLOCK_HAS_SIGNATURE + } else { + BlockFlags::EMPTY + }, reserved: MaybeUninit::new(0), invoke: Some(Closure::__get_invoke_stack_block()), // TODO: Use `Self::DESCRIPTOR_BASIC` when `F: Copy` // (probably only possible with specialization). - descriptor: BlockDescriptorPtr { + descriptor: if E::IS_NONE { // SAFETY: The descriptor must (probably) point to `static` // memory, as Objective-C code may assume the block's // descriptor to be alive past the lifetime of the block @@ -185,7 +270,19 @@ impl<'f, A, R, Closure> StackBlock<'f, A, R, Closure> { // does not contain `UnsafeCell` (it does not). // // [promotion]: https://doc.rust-lang.org/reference/destructors.html#constant-promotion - with_copy_dispose: &Self::DESCRIPTOR_WITH_CLONE, + BlockDescriptorPtr { + with_copy_dispose: &Self::DESCRIPTOR_WITH_CLONE, + } + } 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_copy_dispose_signature: + &>::DESCRIPTOR_WITH_CLONE_AND_ENCODING, + } }, }; Self { @@ -196,11 +293,11 @@ impl<'f, A, R, Closure> StackBlock<'f, A, R, Closure> { } } -// `RcBlock::new` +// `RcBlock::with_encoding` impl<'f, A, R, Closure> StackBlock<'f, A, R, Closure> { unsafe extern "C-unwind" fn empty_clone_closure(_dst: *mut c_void, _src: *const c_void) { // We do nothing, the closure has been `memmove`'d already, and - // ownership will be passed in `RcBlock::new`. + // ownership will be passed in `RcBlock::with_encoding`. } const DESCRIPTOR_WITH_DROP: BlockDescriptorCopyDispose = BlockDescriptorCopyDispose { @@ -212,32 +309,60 @@ impl<'f, A, R, Closure> StackBlock<'f, A, R, Closure> { /// # Safety /// - /// `_Block_copy` must be called on the resulting stack block only once. + /// * `_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`]. #[inline] - pub(crate) unsafe fn new_no_clone(closure: Closure) -> Self + pub(crate) unsafe fn new_no_clone(closure: Closure) -> Self where A: EncodeArguments, R: EncodeReturn, Closure: IntoBlock<'f, A, R>, + E: ManualBlockEncodingExt, { // Don't need to emit copy and dispose helpers if the closure // doesn't need it. - // - // TODO: Add signature. let flags = if mem::needs_drop::() { BlockFlags::BLOCK_HAS_COPY_DISPOSE } else { BlockFlags::EMPTY + } | if !E::IS_NONE { + BlockFlags::BLOCK_HAS_SIGNATURE + } else { + BlockFlags::EMPTY }; // See discussion in `new` above with regards to the safety of the // pointer to the descriptor. let descriptor = if mem::needs_drop::() { + if E::IS_NONE { + // SAFETY: see above. + BlockDescriptorPtr { + with_copy_dispose: &Self::DESCRIPTOR_WITH_DROP, + } + } 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_copy_dispose_signature: + &>::DESCRIPTOR_WITH_DROP_AND_ENCODING, + } + } + } else if E::IS_NONE { + // SAFETY: see above. BlockDescriptorPtr { - with_copy_dispose: &Self::DESCRIPTOR_WITH_DROP, + basic: &Self::DESCRIPTOR_BASIC, } } else { + // SAFETY: see above; the value is already a similar constant, + // so promotion can be guaranteed as well here. BlockDescriptorPtr { - basic: &Self::DESCRIPTOR_BASIC, + // 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: &>::DESCRIPTOR_BASIC_WITH_ENCODING, } }; @@ -257,6 +382,68 @@ impl<'f, A, R, Closure> StackBlock<'f, A, R, Closure> { } } +/// Dummy trait used in order to link [`StackBlock`]'s descriptor constants and +/// a [`ManualBlockEncoding`] into new derived constants at compile time. +/// +/// This is definitely a hack that should be replaced with `const fn`s defined +/// next to [`StackBlock`]'s descriptor constants and used in the below +/// constants' stead with inline `const`s to guarantee proper promotion. +/// +/// See also the below [`EncodedCloneDescriptors`]. +trait EncodedDescriptors { + const DESCRIPTOR_BASIC_WITH_ENCODING: BlockDescriptorSignature; + const DESCRIPTOR_WITH_DROP_AND_ENCODING: BlockDescriptorCopyDisposeSignature; +} + +impl<'f, A, R, Closure, E> EncodedDescriptors for StackBlock<'f, A, R, Closure> +where + A: EncodeArguments, + R: EncodeReturn, + Closure: IntoBlock<'f, A, R>, + E: ManualBlockEncoding, +{ + /// [`Self::DESCRIPTOR_BASIC`] with the signature added from `E`. + const DESCRIPTOR_BASIC_WITH_ENCODING: BlockDescriptorSignature = BlockDescriptorSignature { + reserved: Self::DESCRIPTOR_BASIC.reserved, + size: Self::DESCRIPTOR_BASIC.size, + encoding: E::ENCODING_CSTR.as_ptr(), + }; + /// [`Self::DESCRIPTOR_WITH_DROP`] with the signature added from `E`. + const DESCRIPTOR_WITH_DROP_AND_ENCODING: BlockDescriptorCopyDisposeSignature = + BlockDescriptorCopyDisposeSignature { + reserved: Self::DESCRIPTOR_WITH_DROP.reserved, + size: Self::DESCRIPTOR_WITH_DROP.size, + copy: Self::DESCRIPTOR_WITH_DROP.copy, + dispose: Self::DESCRIPTOR_WITH_DROP.dispose, + encoding: E::ENCODING_CSTR.as_ptr(), + }; +} + +/// Identical role as [`EncodedDescriptors`], with the additional requirement +/// that the block closure be [`Clone`] when implemented on [`StackBlock`] +/// since [`StackBlock::DESCRIPTOR_WITH_CLONE`] is defined in such a context. +trait EncodedCloneDescriptors { + const DESCRIPTOR_WITH_CLONE_AND_ENCODING: BlockDescriptorCopyDisposeSignature; +} + +impl<'f, A, R, Closure, E> EncodedCloneDescriptors for StackBlock<'f, A, R, Closure> +where + A: EncodeArguments, + R: EncodeReturn, + Closure: IntoBlock<'f, A, R> + Clone, + E: ManualBlockEncoding, +{ + /// [`Self::DESCRIPTOR_WITH_CLONE`] with the signature added from `E`. + const DESCRIPTOR_WITH_CLONE_AND_ENCODING: BlockDescriptorCopyDisposeSignature = + BlockDescriptorCopyDisposeSignature { + reserved: Self::DESCRIPTOR_WITH_CLONE.reserved, + size: Self::DESCRIPTOR_WITH_CLONE.size, + copy: Self::DESCRIPTOR_WITH_CLONE.copy, + dispose: Self::DESCRIPTOR_WITH_CLONE.dispose, + encoding: E::ENCODING_CSTR.as_ptr(), + }; +} + impl<'f, A, R, Closure: Clone> Clone for StackBlock<'f, A, R, Closure> { #[inline] fn clone(&self) -> Self { diff --git a/crates/block2/src/traits.rs b/crates/block2/src/traits.rs index 13a3914b7..b3a8875da 100644 --- a/crates/block2/src/traits.rs +++ b/crates/block2/src/traits.rs @@ -1,5 +1,8 @@ +use core::marker::PhantomData; use core::mem; use core::ptr; +// TODO: use `core` when the MSRV is at least 1.64. +use std::ffi::CStr; use objc2::encode::EncodeArguments; use objc2::encode::{EncodeArgument, EncodeReturn}; @@ -135,3 +138,221 @@ impl_traits!(t0: T0, t1: T1, t2: T2, t3: T3, t4: T4, t5: T5, t6: T6, t7: T7, t8: impl_traits!(t0: T0, t1: T1, t2: T2, t3: T3, t4: T4, t5: T5, t6: T6, t7: T7, t8: T8, t9: T9); impl_traits!(t0: T0, t1: T1, t2: T2, t3: T3, t4: T4, t5: T5, t6: T6, t7: T7, t8: T8, t9: T9, t10: T10); impl_traits!(t0: T0, t1: T1, t2: T2, t3: T3, t4: T4, t5: T5, t6: T6, t7: T7, t8: T8, t9: T9, t10: T10, t11: T11); + +/// Interim abstraction to manually provide block encodings for use at compile +/// time with [`StackBlock::with_encoding`] and [`RcBlock::with_encoding`]. +/// +/// See these functions for examples of how to implement and use this trait, +/// since its sole purpose is passing values at compile time to them. +/// +/// As a side note, one might be surprised by the additional [`Self::Arguments`] +/// and [`Self::Return`] associated types requiring concrete implementations to +/// specify them while they are not actually used. This is intentional: +/// +/// * the types are checked at compile-time to be equal to the ones used with +/// [`RcBlock::with_encoding`] and [`StackBlock::with_encoding`], usually +/// inferred by the compiler when giving a closure: this should help avoid +/// some easy oversights; +/// * the user is forced to write both the standard Rust types and the +/// encoding string at the same time, so particular attention to the types +/// is put to the forefront for them; +/// * reading a block encoding string is tough when not initiated, so these +/// also serve as self-documentation; +/// +/// [`RcBlock::with_encoding`]: crate::RcBlock::with_encoding +/// +/// # Safety +/// +/// [`Self::ENCODING_CSTR`] must correspond to the actual signature string a +/// recent-enough Objective-C compiler would generate for a block taking in +/// [`Self::Arguments`] as input and returning [`Self::Return`] as output. +/// This information is actually used by the Objective-C runtime in order to +/// correctly invoke the block, so specifying a wrong encoding is definitely a +/// soundness issue: see [this issue comment][i442-sign-check] for more details +/// about what exactly goes on behind the scenes in order to justify all the +/// following precautions. +/// +/// The simplest way to achieve this is to somehow obtain a block with the +/// correct signature from the Objective-C runtime and [`Debug`]-display it to +/// copy its `encoding` C string. +/// +/// Another possibility is to help yourself with [`Encoding::to_string`][enc2s] +/// in order to get the various components of the signature string separately +/// and then concatenate them manually with the required numbers described +/// below inserted at their correct place. +/// +/// Yet another possibility is to compile a small Objective-C program, with GCC +/// and GNUstep for example, that simply displays the result of +/// `method_getTypeEncoding(class_getClassMethod([MyClass class] @selector(sel)))` +/// where `MyClass::sel` has a signature compatible with the block you want, +/// after which the string can be slightly modified to fit an actual block +/// instead of a method: see below. +/// +/// A more thorough but manual approach is to only follow the rules described +/// below. +/// +/// [enc2s]: objc2::encode::Encoding#impl-Display-for-Encoding +/// [i442-sign-check]: https://github.com/madsmtm/objc2/issues/442#issuecomment-2284932726 +/// +/// # Encoding string generation +/// +/// This is the result of the `@encode` Objective-C compiler directive. The +/// [Apple documentation] and [GCC documentation] explain how each base type is +/// encoded into a string representation. See there for a somewhat-formal +/// specification and a few basic examples. See also [`Encoding`]. +/// +/// See also the [GCC method signatures] section. It is mostly valid for blocks +/// as well, since they are basically functions with captured environment -- +/// i.e. closures, except that no selector is implicitely sent, only the block +/// object is. In short, the "signature" is a null-terminated string, composed +/// of the following, in order: +/// +/// * The return type, including type qualifiers. For example, a block +/// returning `int` ([`i32`]) would have `i` here. +/// * The total size (in bytes) required to pass all the parameters: the call +/// frame size. This includes the hidden block object parameter that is +/// passed as a pointer, so at least 4 bytes when under a 32-bit system or +/// most probably 8 bytes when under a 64-bit one. +/// * Each argument, with the type encoding, followed by the offset (in bytes) +/// of the argument in the list of parameters. The first is the always the +/// hidden block object pointer and is therefore `@?0`. +/// +/// Examples: +/// +/// | Objective-C signature | Runtime encoding | +/// | ------------------------ | ------------------------------------------ | +/// | `void (^)(void)` | `v8@?0` | +/// | `int (^)(void)` | `i8@?0` | +/// | `int (^)(float)` | `i12@?0f8` | +/// | `int (^)(float, _Bool)` | `i16@?0f8B12` | +/// | `void (^)(int*)` | `v16@?0^i8` | +/// | `void (^)(NSError*)` | `v16@?0@8` or `v16@?0@"NSError"8` | +/// | `NSError* (^)(NSError*)` | `@16@?0@8` or `@"NSError"16@?0@"NSError"8` | +/// +/// [Apple documentation]: https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/ObjCRuntimeGuide/Articles/ocrtTypeEncodings.html +/// [`Encoding`]: objc2::encode::Encoding +/// [GCC documentation]: https://gcc.gnu.org/onlinedocs/gcc/Type-encoding.html +/// [GCC method signatures]: https://gcc.gnu.org/onlinedocs/gcc/Method-signatures.html +pub unsafe trait ManualBlockEncoding { + /// The function's input argument types. + type Arguments: EncodeArguments; + /// The function's return type. + type Return: EncodeReturn; + /// The raw encoding information string. + const ENCODING_CSTR: &'static CStr; +} + +/// Particular [`ManualBlockEncoding`] that indicates no actual encoding should +/// be set in the block's descriptor. +/// +/// This is used in a bit of a hackish way in order to share more code between +/// the encoded and non-encoded paths. +pub(crate) struct NoBlockEncoding +where + A: EncodeArguments, + R: EncodeReturn, +{ + _a: PhantomData, + _r: PhantomData, +} + +unsafe impl ManualBlockEncoding for NoBlockEncoding +where + A: EncodeArguments, + R: EncodeReturn, +{ + type Arguments = A; + type Return = R; + // TODO: change this to `c""` when the MSRV is at least 1.77. + // SAFETY: the byte string is written here and contains exactly one nul byte. + const ENCODING_CSTR: &'static CStr = unsafe { CStr::from_bytes_with_nul_unchecked(b"\0") }; +} + +/// Crate-private extension to [`ManualBlockEncoding`]. +pub(crate) trait ManualBlockEncodingExt: ManualBlockEncoding { + /// Only `true` for [`NoBlockEncoding`]. + const IS_NONE: bool; +} + +impl ManualBlockEncodingExt for UserSpecified { + const IS_NONE: bool = false; +} + +impl ManualBlockEncodingExt for NoBlockEncoding +where + A: EncodeArguments, + R: EncodeReturn, +{ + const IS_NONE: bool = true; +} + +/// Dummy newtype used to conditionally implement [`ManualBlockEncodingExt`] +/// and therefore circumvent the need for specialization. +#[repr(transparent)] +pub(crate) struct UserSpecified(E); + +unsafe impl ManualBlockEncoding for UserSpecified { + type Arguments = E::Arguments; + type Return = E::Return; + const ENCODING_CSTR: &'static CStr = E::ENCODING_CSTR; +} + +#[cfg(test)] +mod tests { + use core::ffi::c_char; + + use super::*; + + #[test] + fn test_manual_block_encoding_is_none() { + // Normal case. + struct Enc1; + unsafe impl ManualBlockEncoding for Enc1 { + type Arguments = (i32, f32); + type Return = u8; + #[cfg(target_pointer_width = "64")] + const ENCODING_CSTR: &'static CStr = + // Somehow, using a C string literal seems to fail the MSRV + // check here, so use the old way instead here. + unsafe { CStr::from_bytes_with_nul_unchecked(b"C16@?0i8f12\0") }; + #[cfg(not(target_pointer_width = "64"))] + const ENCODING_CSTR: &'static CStr = + unsafe { CStr::from_bytes_with_nul_unchecked(b"C12@?0i4f8\0") }; + } + // HACK: use `identity` in order to circumvent a Clippy warning. + assert!(!std::convert::identity(UserSpecified::::IS_NONE)); + + // No input + no output case. + struct Enc2; + unsafe impl ManualBlockEncoding for Enc2 { + type Arguments = (); + type Return = (); + #[cfg(target_pointer_width = "64")] + const ENCODING_CSTR: &'static CStr = + unsafe { CStr::from_bytes_with_nul_unchecked(b"v8@?0\0") }; + #[cfg(not(target_pointer_width = "64"))] + const ENCODING_CSTR: &'static CStr = + unsafe { CStr::from_bytes_with_nul_unchecked(b"v4@?0\0") }; + } + assert!(!std::convert::identity(UserSpecified::::IS_NONE)); + + // Ensure we don't rely on the encoding string's emptiness. + struct Enc3; + unsafe impl ManualBlockEncoding for Enc3 { + type Arguments = (); + type Return = (); + const ENCODING_CSTR: &'static CStr = + unsafe { CStr::from_bytes_with_nul_unchecked(b"\0") }; + } + assert!(!std::convert::identity(UserSpecified::::IS_NONE)); + + // Only `NoBlockEncoding` should be `IS_NONE`. + assert!(std::convert::identity(NoBlockEncoding::<(), ()>::IS_NONE)); + assert!(std::convert::identity( + NoBlockEncoding::<(i32, f32), u8>::IS_NONE + )); + assert!(std::convert::identity( + NoBlockEncoding::<(*const u8,), *const c_char>::IS_NONE + )); + } +} From 7b2d824621e78e151e5ec8d088d653c67116a05a Mon Sep 17 00:00:00 2001 From: Paul Mabileau Date: Fri, 19 Jul 2024 03:11:42 +0200 Subject: [PATCH 03/14] Feat(encode): Add ability to compute block encoding string Signed-off-by: Paul Mabileau --- crates/block2/src/encoding.rs | 194 ++++++++++++++++++++++++++++ crates/block2/src/lib.rs | 1 + crates/block2/src/traits.rs | 3 +- crates/objc2-encode/src/encoding.rs | 8 ++ crates/objc2-encode/src/helper.rs | 94 ++++++++++++++ 5 files changed, 299 insertions(+), 1 deletion(-) create mode 100644 crates/block2/src/encoding.rs diff --git a/crates/block2/src/encoding.rs b/crates/block2/src/encoding.rs new file mode 100644 index 000000000..569411f58 --- /dev/null +++ b/crates/block2/src/encoding.rs @@ -0,0 +1,194 @@ +use alloc::ffi::CString; +use alloc::string::ToString; +use alloc::vec::Vec; +use core::mem; + +use objc2::encode::{EncodeArguments, EncodeReturn, Encoding}; + +/// Computes the raw signature string of the object corresponding to the block +/// taking `A` as inputs and returning `R`. +/// +/// Although this is currently implemented on a best-effort basis, this should +/// still serve as a good way to obtain what to fill in the encoding string +/// when implementing [`crate::ManualBlockEncoding`]. +/// +/// # Example +/// +/// ```ignore +/// assert_eq!(block_signature_string::<(i32, f32), u8>(), "C16@?0i8f12"); +/// ``` +#[allow(unused)] +pub fn block_signature_string() -> CString +where + A: EncodeArguments, + R: EncodeReturn, +{ + block_signature_string_inner(A::ENCODINGS, &R::ENCODING_RETURN) +} + +#[allow(unused)] +fn block_signature_string_inner(args: &[Encoding], ret: &Encoding) -> CString { + // TODO: alignment? + let arg_sizes = args + .iter() + .map(Encoding::size) + .map(Option::unwrap_or_default) + .collect::>(); + let args_size = arg_sizes.iter().sum::(); + + // Take the hidden block parameter into account. + let mut off = mem::size_of::<*const ()>(); + let mut res = ret.to_string(); + res.push_str(&(off + args_size).to_string()); + res.push_str("@?0"); + + for (arg_enc, arg_size) in args.iter().zip(arg_sizes) { + res.push_str(&arg_enc.to_string()); + res.push_str(&off.to_string()); + off += arg_size; + } + + // UNWRAP: The above construction only uses controlled `ToString` + // implementations that do not include nul bytes. + CString::new(res).unwrap() +} + +#[cfg(test)] +mod tests { + use super::*; + use alloc::borrow::ToOwned; + + #[test] + fn test_block_signature_string() { + for (args, ret, val) in [ + ( + &[][..], + &Encoding::Void, + #[cfg(target_pointer_width = "64")] + "v8@?0", + #[cfg(target_pointer_width = "32")] + "v4@?0", + ), + ( + &[], + &Encoding::Int, + #[cfg(target_pointer_width = "64")] + "i8@?0", + #[cfg(target_pointer_width = "32")] + "i4@?0", + ), + ( + &[], + &Encoding::Float, + #[cfg(target_pointer_width = "64")] + "f8@?0", + #[cfg(target_pointer_width = "32")] + "f4@?0", + ), + ( + &[], + &Encoding::Bool, + #[cfg(target_pointer_width = "64")] + "B8@?0", + #[cfg(target_pointer_width = "32")] + "B4@?0", + ), + ( + &[Encoding::Int], + &Encoding::Void, + #[cfg(target_pointer_width = "64")] + "v12@?0i8", + #[cfg(target_pointer_width = "32")] + "v8@?0i4", + ), + ( + &[Encoding::Int], + &Encoding::Int, + #[cfg(target_pointer_width = "64")] + "i12@?0i8", + #[cfg(target_pointer_width = "32")] + "i8@?0i4", + ), + ( + &[Encoding::Long, Encoding::Double, Encoding::FloatComplex], + &Encoding::Atomic(&Encoding::UChar), + #[cfg(target_pointer_width = "64")] + "AC32@?0l8d16jf24", + #[cfg(target_pointer_width = "32")] + "AC24@?0l4d8jf16", + ), + ( + &[ + Encoding::Union("ThisOrThat", &[Encoding::UShort, Encoding::Int]), + Encoding::Struct( + "ThisAndThat", + &[ + Encoding::ULongLong, + Encoding::LongDoubleComplex, + Encoding::Atomic(&Encoding::Bool), + ], + ), + ], + &Encoding::String, + // Probably unaligned. + #[cfg(any(target_arch = "x86_64", target_arch = "aarch64",))] + "*53@?0(ThisOrThat=Si)8{ThisAndThat=QjDAB}12", + #[cfg(all(target_arch = "x86", target_vendor = "apple"))] + "*45@?0(ThisOrThat=Si)4{ThisAndThat=QjDAB}8", + #[cfg(all(target_arch = "x86", not(target_vendor = "apple")))] + "*41@?0(ThisOrThat=Si)4{ThisAndThat=QjDAB}8", + #[cfg(target_arch = "arm")] + "*37@?0(ThisOrThat=Si)4{ThisAndThat=QjDAB}8", + ), + ( + &[ + Encoding::Block, + Encoding::Class, + Encoding::Object, + Encoding::Pointer(&Encoding::Char), + Encoding::Sel, + Encoding::String, + Encoding::Unknown, + Encoding::Unknown, + Encoding::Unknown, + ], + &Encoding::Pointer(&Encoding::Atomic(&Encoding::UChar)), + #[cfg(target_pointer_width = "64")] + "^AC56@?0@?8#16@24^c32:40*48?56?56?56", + #[cfg(target_pointer_width = "32")] + "^AC28@?0@?4#8@12^c16:20*24?28?28?28", + ), + ( + &[Encoding::Array(123, &Encoding::Object)], + &Encoding::Pointer(&Encoding::Class), + #[cfg(target_pointer_width = "64")] + "^#992@?0[123@]8", + #[cfg(target_pointer_width = "32")] + "^#496@?0[123@]4", + ), + // Bitfields can probably not be passed around through functions, + // so this may be a bit nonsensical, but let's test it anyway. + ( + &[ + Encoding::BitField(1, None), + Encoding::BitField(2, None), + Encoding::BitField(3, None), + Encoding::BitField(6, None), + Encoding::BitField(8, None), + Encoding::BitField(42, None), + Encoding::BitField(28, Some(&(2, Encoding::UInt))), + ], + &Encoding::Sel, + #[cfg(target_pointer_width = "64")] + ":25@?0b18b29b310b611b812b4213b2I2821", + #[cfg(target_pointer_width = "32")] + ":21@?0b14b25b36b67b88b429b2I2817", + ), + ] { + assert_eq!( + block_signature_string_inner(args, ret), + CString::new(val.to_owned().into_bytes()).unwrap() + ); + } + } +} diff --git a/crates/block2/src/lib.rs b/crates/block2/src/lib.rs index 4de5f021d..c3ad63cb8 100644 --- a/crates/block2/src/lib.rs +++ b/crates/block2/src/lib.rs @@ -368,6 +368,7 @@ extern crate objc2 as _; mod abi; mod block; mod debug; +mod encoding; pub mod ffi; mod global; mod rc_block; diff --git a/crates/block2/src/traits.rs b/crates/block2/src/traits.rs index b3a8875da..20dec5b0c 100644 --- a/crates/block2/src/traits.rs +++ b/crates/block2/src/traits.rs @@ -179,7 +179,8 @@ impl_traits!(t0: T0, t1: T1, t2: T2, t3: T3, t4: T4, t5: T5, t6: T6, t7: T7, t8: /// Another possibility is to help yourself with [`Encoding::to_string`][enc2s] /// in order to get the various components of the signature string separately /// and then concatenate them manually with the required numbers described -/// below inserted at their correct place. +/// below inserted at their correct place. `encoding::block_signature_string` +/// can help automate this particular process, still not at compile time though. /// /// Yet another possibility is to compile a small Objective-C program, with GCC /// and GNUstep for example, that simply displays the result of diff --git a/crates/objc2-encode/src/encoding.rs b/crates/objc2-encode/src/encoding.rs index 26a122e76..15486292d 100644 --- a/crates/objc2-encode/src/encoding.rs +++ b/crates/objc2-encode/src/encoding.rs @@ -248,6 +248,14 @@ impl Encoding { pub fn equivalent_to_box(&self, other: &EncodingBox) -> bool { compare_encodings(self, other, NestingLevel::new(), false) } + + /// Computes the theoretical size in bytes of the represented value type. + /// + /// Everything is considered packed, i.e. no alignment is computed. Also, + /// the result is only valid for the current build target. + pub fn size(&self) -> Option { + Helper::new(self).size(NestingLevel::new()) + } } /// Formats this [`Encoding`] in a similar way that the `@encode` directive diff --git a/crates/objc2-encode/src/helper.rs b/crates/objc2-encode/src/helper.rs index a6694954e..6423fbb12 100644 --- a/crates/objc2-encode/src/helper.rs +++ b/crates/objc2-encode/src/helper.rs @@ -1,4 +1,6 @@ +use core::ffi; use core::fmt; +use core::mem; use core::write; use crate::parse::verify_name; @@ -171,6 +173,61 @@ impl Primitive { Unknown => "?", } } + + pub(crate) const fn size(self) -> Option { + macro_rules! opt_double { + ($e:expr) => { + match $e { + Some(x) => Some(2 * x), + None => None, + } + }; + } + match self { + // Under all the considered targets, `_Bool` is sized and aligned + // to a single byte. See: + // https://github.com/search?q=repo%3Allvm%2Fllvm-project+path%3Aclang%2Flib%2FBasic%2FTargets+BoolWidth&type=code + // Obj-C's `BOOL` is `signed char`, i.e. `c`, so will fall in the + // below case. See: https://developer.apple.com/documentation/objectivec/bool?language=objc + Self::Bool => Some(1), + // Numbers. + Self::Char => Some(mem::size_of::()), + Self::UChar => Some(mem::size_of::()), + Self::Short => Some(mem::size_of::()), + Self::UShort => Some(mem::size_of::()), + Self::Int => Some(mem::size_of::()), + Self::UInt => Some(mem::size_of::()), + Self::Long => Some(mem::size_of::()), + Self::ULong => Some(mem::size_of::()), + Self::LongLong => Some(mem::size_of::()), + Self::ULongLong => Some(mem::size_of::()), + Self::Float => Some(mem::size_of::()), + Self::Double => Some(mem::size_of::()), + // https://github.com/search?q=repo%3Allvm%2Fllvm-project+path%3Aclang%2Flib%2FBasic%2FTargets+LongDoubleWidth&type=code + #[cfg(any( + target_arch = "x86_64", + all(target_arch = "x86", target_vendor = "apple"), + all(target_arch = "aarch64", not(target_vendor = "apple")), + ))] + Self::LongDouble => Some(16), + #[cfg(all(target_arch = "x86", not(target_vendor = "apple")))] + Self::LongDouble => Some(12), + #[cfg(any( + target_arch = "arm", + all(target_arch = "aarch64", target_vendor = "apple"), + ))] + Self::LongDouble => Some(8), + Self::FloatComplex => opt_double!(Self::Float.size()), + Self::DoubleComplex => opt_double!(Self::Double.size()), + Self::LongDoubleComplex => opt_double!(Self::LongDouble.size()), + // Pointers. + Self::String | Self::Object | Self::Block | Self::Class | Self::Sel => { + Some(mem::size_of::<*const ()>()) + } + // Nothing. + Self::Void | Self::Unknown => None, + } + } } #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] @@ -285,6 +342,43 @@ impl Helper<'_, E> { } Ok(()) } + + pub(crate) fn size(&self, level: NestingLevel) -> Option { + // TODO: alignment? + match self { + Self::NoneInvalid => None, + Self::Primitive(prim) => prim.size(), + Self::BitField(size_bits, off_typ) => Some( + (usize::from(*size_bits).next_power_of_two().max(8) / 8).max( + off_typ + .and_then(|(_, typ)| typ.helper().size(level.bitfield())) + .unwrap_or_default(), + ), + ), + Self::Indirection(kind, typ) => match kind { + IndirectionKind::Pointer => Some(mem::size_of::<*const ()>()), + IndirectionKind::Atomic => typ.helper().size(level.indirection(*kind)), + }, + Self::Array(len, typ) => typ + .helper() + .size(level.array()) + .map(|typ_size| *len as usize * typ_size), + Self::Container(kind, _, fields) => { + level + .container_include_fields() + .and_then(|level| match kind { + ContainerKind::Struct => { + fields.iter().map(|field| field.helper().size(level)).sum() + } + ContainerKind::Union => fields + .iter() + .map(|field| field.helper().size(level)) + .max() + .flatten(), + }) + } + } + } } impl Helper<'_> { From c6bb5e1c212b25a844a04e6adf716175fccbd331 Mon Sep 17 00:00:00 2001 From: Paul Mabileau Date: Fri, 19 Jul 2024 16:18:26 +0200 Subject: [PATCH 04/14] Feat(block2): Add placeholders for safety debug assertions to manually-encoded block constructors As per the review: until the block encoding generation is done better. Signed-off-by: Paul Mabileau --- crates/block2/src/stack.rs | 2 ++ crates/block2/src/traits.rs | 23 +++++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/crates/block2/src/stack.rs b/crates/block2/src/stack.rs index cc8a01a68..cd6220b62 100644 --- a/crates/block2/src/stack.rs +++ b/crates/block2/src/stack.rs @@ -241,6 +241,7 @@ where where E: ManualBlockEncodingExt, { + // TODO: Re-consider calling `crate::traits::debug_assert_block_encoding`. let header = BlockHeader { #[allow(unused_unsafe)] isa: unsafe { ptr::addr_of!(ffi::_NSConcreteStackBlock) }, @@ -320,6 +321,7 @@ impl<'f, A, R, Closure> StackBlock<'f, A, R, Closure> { Closure: IntoBlock<'f, A, R>, E: ManualBlockEncodingExt, { + // TODO: Re-consider calling `crate::traits::debug_assert_block_encoding`. // Don't need to emit copy and dispose helpers if the closure // doesn't need it. let flags = if mem::needs_drop::() { diff --git a/crates/block2/src/traits.rs b/crates/block2/src/traits.rs index 20dec5b0c..4902d1b9c 100644 --- a/crates/block2/src/traits.rs +++ b/crates/block2/src/traits.rs @@ -298,6 +298,29 @@ unsafe impl ManualBlockEncoding for UserSpecified { const ENCODING_CSTR: &'static CStr = E::ENCODING_CSTR; } +/// Checks for encoding compatibility between the given generic parameters, +/// panicking if it is not, but only on `cfg(debug_assertions)` and if `E` is +/// not none. +#[cfg_attr(not(debug_assertions), inline(always))] +#[allow(unused)] +pub(crate) fn debug_assert_block_encoding() +where + A: EncodeArguments, + R: EncodeReturn, + E: ManualBlockEncodingExt, +{ + #[cfg(debug_assertions)] + { + if !E::IS_NONE { + // TODO: relax to check for equivalence instead of strict equality. + assert_eq!( + E::ENCODING_CSTR, + &*crate::encoding::block_signature_string::() + ); + } + } +} + #[cfg(test)] mod tests { use core::ffi::c_char; From 8793842d292a5571de40a79651c3d767aec1fc84 Mon Sep 17 00:00:00 2001 From: Paul Mabileau Date: Wed, 17 Jul 2024 13:04:36 +0200 Subject: [PATCH 05/14] Chore: Update compiler UI test * `StackBlock`'s `Clone` constraint checks: order switch it seems. * `OptionEncode` compile-time checks: small line number shift. Signed-off-by: Paul Mabileau --- crates/test-ui/ui/stack_block_requires_clone.stderr | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/test-ui/ui/stack_block_requires_clone.stderr b/crates/test-ui/ui/stack_block_requires_clone.stderr index f385d73df..346f4c107 100644 --- a/crates/test-ui/ui/stack_block_requires_clone.stderr +++ b/crates/test-ui/ui/stack_block_requires_clone.stderr @@ -19,11 +19,11 @@ note: required because it's used within this closure note: required by a bound in `StackBlock::<'f, A, R, Closure>::new` --> $WORKSPACE/crates/block2/src/stack.rs | - | pub fn new(closure: Closure) -> Self - | --- required by a bound in this associated function + | Closure: IntoBlock<'f, A, R> + Clone, + | ^^^^^ required by this bound in `StackBlock::<'f, A, R, Closure>::new` ... - | Closure: IntoBlock<'f, A, R> + Clone, - | ^^^^^ required by this bound in `StackBlock::<'f, A, R, Closure>::new` + | pub fn new(closure: Closure) -> Self { + | --- required by a bound in this associated function help: consider annotating `Foo` with `#[derive(Clone)]` | 3 + #[derive(Clone)] From 69118012ad0dc36fe4596c783b74edf6301f71cd Mon Sep 17 00:00:00 2001 From: Paul Mabileau Date: Wed, 17 Jul 2024 20:01:30 +0200 Subject: [PATCH 06/14] Test(ui): Extend block tests for with_encoding Signed-off-by: Paul Mabileau --- .../test-ui/ui/block_lifetimes_independent.rs | 50 ++++++++- .../ui/block_lifetimes_independent.stderr | 104 ++++++++++++++++++ .../ui/lifetime_of_closure_tied_to_block.rs | 30 ++++- .../lifetime_of_closure_tied_to_block.stderr | 59 +++++++++- ...tack_block_with_encoding_requires_clone.rs | 20 ++++ ..._block_with_encoding_requires_clone.stderr | 31 ++++++ 6 files changed, 291 insertions(+), 3 deletions(-) create mode 100644 crates/test-ui/ui/stack_block_with_encoding_requires_clone.rs create mode 100644 crates/test-ui/ui/stack_block_with_encoding_requires_clone.stderr diff --git a/crates/test-ui/ui/block_lifetimes_independent.rs b/crates/test-ui/ui/block_lifetimes_independent.rs index 1f169b6ba..e35a5500d 100644 --- a/crates/test-ui/ui/block_lifetimes_independent.rs +++ b/crates/test-ui/ui/block_lifetimes_independent.rs @@ -1,7 +1,9 @@ //! Test that lifetimes in blocks are not bound to each other. //! //! These tests will succeed if there are `'a: 'b`-like bounds on the closure. -use block2::RcBlock; +use block2::{ManualBlockEncoding, RcBlock}; +use std::ffi::CStr; +use std::marker::PhantomData; fn args<'a, 'b>( f: impl Fn(&'a i32, &'b i32) + 'static, @@ -23,4 +25,50 @@ fn return_entire<'a, 'b>(f: impl Fn() -> &'a i32 + 'b) -> RcBlock &' RcBlock::new(f) } +fn args_with_encoding<'a, 'b>( + f: impl Fn(&'a i32, &'b i32) + 'static, +) -> RcBlock { + struct Enc<'a, 'b>(PhantomData<&'a i32>, PhantomData<&'b i32>); + unsafe impl<'a, 'b> ManualBlockEncoding for Enc<'a, 'b> { + type Arguments = (&'a i32, &'b i32); + type Return = (); + const ENCODING_CSTR: &'static CStr = c"v24@?0^i8^i16"; + } + unsafe { RcBlock::with_encoding::<_, _, _, Enc<'a, 'b>>(f) } +} + +fn args_return_with_encoding<'a, 'b>( + f: impl Fn(&'a i32) -> &'b i32 + 'static, +) -> RcBlock &'a i32 + 'static> { + struct Enc<'a, 'b>(PhantomData<&'a i32>, PhantomData<&'b i32>); + unsafe impl<'a, 'b> ManualBlockEncoding for Enc<'a, 'b> { + type Arguments = (&'a i32,); + type Return = &'b i32; + const ENCODING_CSTR: &'static CStr = c"^i816@?0^i8"; + } + unsafe { RcBlock::with_encoding::<_, _, _, Enc<'a, 'b>>(f) } +} + +fn args_entire_with_encoding<'a, 'b>(f: impl Fn(&'a i32) + 'b) -> RcBlock { + struct Enc<'a>(PhantomData<&'a i32>); + unsafe impl<'a> ManualBlockEncoding for Enc<'a> { + type Arguments = (&'a i32,); + type Return = (); + const ENCODING_CSTR: &'static CStr = c"v16@?0^i8"; + } + unsafe { RcBlock::with_encoding::<_, _, _, Enc<'a>>(f) } +} + +fn return_entire_with_encoding<'a, 'b>( + f: impl Fn() -> &'a i32 + 'b, +) -> RcBlock &'b i32 + 'a> { + struct Enc<'a>(PhantomData<&'a i32>); + unsafe impl<'a> ManualBlockEncoding for Enc<'a> { + type Arguments = (); + type Return = &'a i32; + const ENCODING_CSTR: &'static CStr = c"^i8@?0"; + } + unsafe { RcBlock::with_encoding::<_, _, _, Enc<'a>>(f) } +} + fn main() {} diff --git a/crates/test-ui/ui/block_lifetimes_independent.stderr b/crates/test-ui/ui/block_lifetimes_independent.stderr index 3ec490778..94aa8f600 100644 --- a/crates/test-ui/ui/block_lifetimes_independent.stderr +++ b/crates/test-ui/ui/block_lifetimes_independent.stderr @@ -99,3 +99,107 @@ error: lifetime may not live long enough | ^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'a` but it is returning data with lifetime `'b` | = help: consider adding the following bound: `'b: 'a` + +error: lifetime may not live long enough + --> ui/block_lifetimes_independent.rs + | + | fn args_with_encoding<'a, 'b>( + | -- -- lifetime `'b` defined here + | | + | lifetime `'a` defined here +... + | unsafe { RcBlock::with_encoding::<_, _, _, Enc<'a, 'b>>(f) } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'b` but it is returning data with lifetime `'a` + | + = help: consider adding the following bound: `'a: 'b` + +error: lifetime may not live long enough + --> ui/block_lifetimes_independent.rs + | + | fn args_with_encoding<'a, 'b>( + | -- -- lifetime `'b` defined here + | | + | lifetime `'a` defined here +... + | unsafe { RcBlock::with_encoding::<_, _, _, Enc<'a, 'b>>(f) } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'a` but it is returning data with lifetime `'b` + | + = help: consider adding the following bound: `'b: 'a` + +error: lifetime may not live long enough + --> ui/block_lifetimes_independent.rs + | + | fn args_return_with_encoding<'a, 'b>( + | -- -- lifetime `'b` defined here + | | + | lifetime `'a` defined here +... + | unsafe { RcBlock::with_encoding::<_, _, _, Enc<'a, 'b>>(f) } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'b` but it is returning data with lifetime `'a` + | + = help: consider adding the following bound: `'a: 'b` + +error: lifetime may not live long enough + --> ui/block_lifetimes_independent.rs + | + | fn args_return_with_encoding<'a, 'b>( + | -- -- lifetime `'b` defined here + | | + | lifetime `'a` defined here +... + | unsafe { RcBlock::with_encoding::<_, _, _, Enc<'a, 'b>>(f) } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'a` but it is returning data with lifetime `'b` + | + = help: consider adding the following bound: `'b: 'a` + +error: lifetime may not live long enough + --> ui/block_lifetimes_independent.rs + | + | fn args_entire_with_encoding<'a, 'b>(f: impl Fn(&'a i32) + 'b) -> RcBlock { + | -- -- lifetime `'b` defined here + | | + | lifetime `'a` defined here +... + | unsafe { RcBlock::with_encoding::<_, _, _, Enc<'a>>(f) } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'b` but it is returning data with lifetime `'a` + | + = help: consider adding the following bound: `'a: 'b` + +error: lifetime may not live long enough + --> ui/block_lifetimes_independent.rs + | + | fn args_entire_with_encoding<'a, 'b>(f: impl Fn(&'a i32) + 'b) -> RcBlock { + | -- -- lifetime `'b` defined here + | | + | lifetime `'a` defined here +... + | unsafe { RcBlock::with_encoding::<_, _, _, Enc<'a>>(f) } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'a` but it is returning data with lifetime `'b` + | + = help: consider adding the following bound: `'b: 'a` + +error: lifetime may not live long enough + --> ui/block_lifetimes_independent.rs + | + | fn return_entire_with_encoding<'a, 'b>( + | -- -- lifetime `'b` defined here + | | + | lifetime `'a` defined here +... + | unsafe { RcBlock::with_encoding::<_, _, _, Enc<'a>>(f) } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'b` but it is returning data with lifetime `'a` + | + = help: consider adding the following bound: `'a: 'b` + +error: lifetime may not live long enough + --> ui/block_lifetimes_independent.rs + | + | fn return_entire_with_encoding<'a, 'b>( + | -- -- lifetime `'b` defined here + | | + | lifetime `'a` defined here +... + | unsafe { RcBlock::with_encoding::<_, _, _, Enc<'a>>(f) } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'a` but it is returning data with lifetime `'b` + | + = help: consider adding the following bound: `'b: 'a` diff --git a/crates/test-ui/ui/lifetime_of_closure_tied_to_block.rs b/crates/test-ui/ui/lifetime_of_closure_tied_to_block.rs index b166d6e4a..6da1cfc69 100644 --- a/crates/test-ui/ui/lifetime_of_closure_tied_to_block.rs +++ b/crates/test-ui/ui/lifetime_of_closure_tied_to_block.rs @@ -1,4 +1,12 @@ -use block2::{RcBlock, StackBlock}; +use block2::{ManualBlockEncoding, RcBlock, StackBlock}; +use std::ffi::CStr; + +struct VoidToI32; +unsafe impl ManualBlockEncoding for VoidToI32 { + type Arguments = (); + type Return = i32; + const ENCODING_CSTR: &'static CStr = c"i8@?0"; +} fn main() { let _ = { @@ -11,6 +19,16 @@ fn main() { RcBlock::new(|| x + 2).clone() }; + let _ = { + let x = 2; + unsafe { RcBlock::with_encoding::<_, _, _, VoidToI32>(|| x + 2) } + }; + + let _ = { + let x = 2; + unsafe { RcBlock::with_encoding::<_, _, _, VoidToI32>(|| x + 2).clone() } + }; + let _ = { let x = 2; StackBlock::new(|| x + 2) @@ -20,4 +38,14 @@ fn main() { let x = 2; StackBlock::new(|| x + 2).copy() }; + + let _ = { + let x = 2; + unsafe { StackBlock::with_encoding::(|| x + 2) } + }; + + let _ = { + let x = 2; + unsafe { StackBlock::with_encoding::(|| x + 2).copy() } + }; } diff --git a/crates/test-ui/ui/lifetime_of_closure_tied_to_block.stderr b/crates/test-ui/ui/lifetime_of_closure_tied_to_block.stderr index b6b6c6ec4..02ea98624 100644 --- a/crates/test-ui/ui/lifetime_of_closure_tied_to_block.stderr +++ b/crates/test-ui/ui/lifetime_of_closure_tied_to_block.stderr @@ -28,9 +28,42 @@ error[E0597]: `x` does not live long enough | help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped | -11 | RcBlock::new(|| x + 2).clone(); +19 | RcBlock::new(|| x + 2).clone(); | + +error[E0597]: `x` does not live long enough + --> ui/lifetime_of_closure_tied_to_block.rs + | + | let x = 2; + | - binding `x` declared here + | unsafe { RcBlock::with_encoding::<_, _, _, VoidToI32>(|| x + 2) } + | -- ^ borrowed value does not live long enough + | | + | value captured here + | }; + | - `x` dropped here while still borrowed + +error[E0597]: `x` does not live long enough + --> ui/lifetime_of_closure_tied_to_block.rs + | + | let x = 2; + | - binding `x` declared here + | unsafe { RcBlock::with_encoding::<_, _, _, VoidToI32>(|| x + 2).clone() } + | ------------------------------------------------^----- + | | | | + | | | borrowed value does not live long enough + | | value captured here + | a temporary with access to the borrow is created here ... + | }; + | -- ... and the borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `RcBlock` + | | + | `x` dropped here while still borrowed + | +help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped + | +29 | unsafe { RcBlock::with_encoding::<_, _, _, VoidToI32>(|| x + 2).clone(); } + | + + error[E0597]: `x` does not live long enough --> ui/lifetime_of_closure_tied_to_block.rs | @@ -54,3 +87,27 @@ error[E0597]: `x` does not live long enough | value captured here | }; | - `x` dropped here while still borrowed + +error[E0597]: `x` does not live long enough + --> ui/lifetime_of_closure_tied_to_block.rs + | + | let x = 2; + | - binding `x` declared here + | unsafe { StackBlock::with_encoding::(|| x + 2) } + | -- ^ borrowed value does not live long enough + | | + | value captured here + | }; + | - `x` dropped here while still borrowed + +error[E0597]: `x` does not live long enough + --> ui/lifetime_of_closure_tied_to_block.rs + | + | let x = 2; + | - binding `x` declared here + | unsafe { StackBlock::with_encoding::(|| x + 2).copy() } + | -- ^ borrowed value does not live long enough + | | + | value captured here + | }; + | - `x` dropped here while still borrowed diff --git a/crates/test-ui/ui/stack_block_with_encoding_requires_clone.rs b/crates/test-ui/ui/stack_block_with_encoding_requires_clone.rs new file mode 100644 index 000000000..bd122a2eb --- /dev/null +++ b/crates/test-ui/ui/stack_block_with_encoding_requires_clone.rs @@ -0,0 +1,20 @@ +use block2::{ManualBlockEncoding, StackBlock}; +use std::ffi::CStr; + +struct Foo; + +fn main() { + struct FooBlockEncoding; + unsafe impl ManualBlockEncoding for FooBlockEncoding { + type Arguments = (); + type Return = (); + const ENCODING_CSTR: &'static CStr = c"v8@?0"; + } + + let foo = Foo; + let _ = unsafe { + StackBlock::with_encoding::(move || { + let _ = &foo; + }) + }; +} diff --git a/crates/test-ui/ui/stack_block_with_encoding_requires_clone.stderr b/crates/test-ui/ui/stack_block_with_encoding_requires_clone.stderr new file mode 100644 index 000000000..d615bf2cd --- /dev/null +++ b/crates/test-ui/ui/stack_block_with_encoding_requires_clone.stderr @@ -0,0 +1,31 @@ +error[E0277]: the trait bound `Foo: Clone` is not satisfied in `{closure@$DIR/ui/stack_block_with_encoding_requires_clone.rs:16:55: 16:62}` + --> ui/stack_block_with_encoding_requires_clone.rs + | + | StackBlock::with_encoding::(move || { + | --------------------------------------------- ^------ + | | | + | _________|_____________________________________________within this `{closure@$DIR/ui/stack_block_with_encoding_requires_clone.rs:16:55: 16:62}` + | | | + | | required by a bound introduced by this call + | | let _ = &foo; + | | }) + | |_________^ within `{closure@$DIR/ui/stack_block_with_encoding_requires_clone.rs:16:55: 16:62}`, the trait `Clone` is not implemented for `Foo`, which is required by `{closure@$DIR/ui/stack_block_with_encoding_requires_clone.rs:16:55: 16:62}: Clone` + | +note: required because it's used within this closure + --> ui/stack_block_with_encoding_requires_clone.rs + | + | StackBlock::with_encoding::(move || { + | ^^^^^^^ +note: required by a bound in `StackBlock::<'f, A, R, Closure>::with_encoding` + --> $WORKSPACE/crates/block2/src/stack.rs + | + | Closure: IntoBlock<'f, A, R> + Clone, + | ^^^^^ required by this bound in `StackBlock::<'f, A, R, Closure>::with_encoding` +... + | pub unsafe fn with_encoding(closure: Closure) -> Self + | ------------- required by a bound in this associated function +help: consider annotating `Foo` with `#[derive(Clone)]` + | +4 + #[derive(Clone)] +5 | struct Foo; + | From 2d960ac14bd985aa968df7389948d5b55a8787f4 Mon Sep 17 00:00:00 2001 From: Paul Mabileau Date: Thu, 18 Jul 2024 15:50:28 +0200 Subject: [PATCH 07/14] Test(block): Extend tests for with_encoding Signed-off-by: Paul Mabileau --- crates/tests/src/block.rs | 368 ++++++++++++++++++++++++++------------ 1 file changed, 256 insertions(+), 112 deletions(-) diff --git a/crates/tests/src/block.rs b/crates/tests/src/block.rs index 2607989f2..d73e00dab 100644 --- a/crates/tests/src/block.rs +++ b/crates/tests/src/block.rs @@ -1,8 +1,9 @@ +use alloc::string::ToString; use core::cell::RefCell; +use std::ffi::CStr; use std::thread_local; -use alloc::string::ToString; -use block2::{global_block, Block, RcBlock, StackBlock}; +use block2::{global_block, Block, ManualBlockEncoding, RcBlock, StackBlock}; use objc2::encode::{Encode, Encoding}; use objc2::rc::Retained; use objc2::runtime::{AnyObject, Bool, NSObject}; @@ -35,6 +36,36 @@ unsafe impl Encode for LargeStruct { type Add12 = Block i32>; +struct VoidToVoid; +unsafe impl ManualBlockEncoding for VoidToVoid { + type Arguments = (); + type Return = (); + #[cfg(target_pointer_width = "64")] + const ENCODING_CSTR: &'static CStr = c"v8@?0"; + #[cfg(target_pointer_width = "32")] + const ENCODING_CSTR: &'static CStr = c"v4@?0"; +} + +struct VoidToInt; +unsafe impl ManualBlockEncoding for VoidToInt { + type Arguments = (); + type Return = i32; + #[cfg(target_pointer_width = "64")] + const ENCODING_CSTR: &'static CStr = c"i8@?0"; + #[cfg(target_pointer_width = "32")] + const ENCODING_CSTR: &'static CStr = c"i4@?0"; +} + +struct IntToInt; +unsafe impl ManualBlockEncoding for IntToInt { + type Arguments = (i32,); + type Return = i32; + #[cfg(target_pointer_width = "64")] + const ENCODING_CSTR: &'static CStr = c"i12@?0i8"; + #[cfg(target_pointer_width = "32")] + const ENCODING_CSTR: &'static CStr = c"i8@?0i4"; +} + extern "C-unwind" { /// Returns a pointer to a global block that returns 7. fn get_int_block() -> *mut Block i32>; @@ -108,6 +139,14 @@ fn test_int_block() { ); invoke_assert(&StackBlock::new(|| 10), 10); invoke_assert(&RcBlock::new(|| 6), 6); + invoke_assert( + unsafe { &StackBlock::with_encoding::(|| 10) }, + 10, + ); + invoke_assert( + unsafe { &RcBlock::with_encoding::<_, _, _, VoidToInt>(|| 6) }, + 6, + ); invoke_assert(&GLOBAL_BLOCK, 42); } @@ -132,6 +171,14 @@ fn test_add_block() { ); invoke_assert(&StackBlock::new(|a: i32| a + 6), 11); invoke_assert(&RcBlock::new(|a: i32| a + 6), 11); + invoke_assert( + unsafe { &StackBlock::with_encoding::(|a: i32| a + 6) }, + 11, + ); + invoke_assert( + unsafe { &RcBlock::with_encoding::<_, _, _, IntToInt>(|a: i32| a + 6) }, + 11, + ); invoke_assert(&GLOBAL_BLOCK, 47); } @@ -149,6 +196,16 @@ fn test_add_12() { ); } + struct Enc; + unsafe impl ManualBlockEncoding for Enc { + type Arguments = (i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32); + type Return = i32; + #[cfg(target_pointer_width = "64")] + const ENCODING_CSTR: &'static CStr = c"i56@?0i8i12i16i20i24i28i32i36i40i44i48i52"; + #[cfg(target_pointer_width = "32")] + const ENCODING_CSTR: &'static CStr = c"i52@?0i4i8i12i16i20i24i28i32i36i40i44i48"; + } + global_block! { static GLOBAL_BLOCK = | a1: i32, a2: i32, a3: i32, a4: i32, @@ -166,6 +223,11 @@ fn test_add_12() { }; invoke_assert(&StackBlock::new(closure), 78); invoke_assert(&RcBlock::new(closure), 78); + invoke_assert(unsafe { &StackBlock::with_encoding::(closure) }, 78); + invoke_assert( + unsafe { &RcBlock::with_encoding::<_, _, _, Enc>(closure) }, + 78, + ); invoke_assert(&GLOBAL_BLOCK, 120); } @@ -192,6 +254,16 @@ fn test_large_struct_block() { }; } + struct Enc; + unsafe impl ManualBlockEncoding for Enc { + type Arguments = (LargeStruct,); + type Return = LargeStruct; + #[cfg(target_pointer_width = "64")] + const ENCODING_CSTR: &'static CStr = c"{LargeStruct=f[100C]}112@?0{LargeStruct=f[100C]}8"; + #[cfg(target_pointer_width = "32")] + const ENCODING_CSTR: &'static CStr = c"{LargeStruct=f[100C]}108@?0{LargeStruct=f[100C]}4"; + } + let data = LargeStruct::get(); let mut new_data = data; new_data.mutate(); @@ -206,17 +278,31 @@ fn test_large_struct_block() { assert_eq!(unsafe { invoke_large_struct_block(&block, data) }, new_data); let block = block.copy(); assert_eq!(unsafe { invoke_large_struct_block(&block, data) }, new_data); + + let block = unsafe { + StackBlock::with_encoding::(|mut x: LargeStruct| { + x.mutate(); + x + }) + }; + assert_eq!(unsafe { invoke_large_struct_block(&block, data) }, new_data); + let block = block.copy(); + assert_eq!(unsafe { invoke_large_struct_block(&block, data) }, new_data); } #[test] fn test_block_copy() { let s = "Hello!".to_string(); let expected_len = s.len() as i32; - let block = StackBlock::new(move || s.len() as i32); - assert_eq!(unsafe { invoke_int_block(&block) }, expected_len); - - let copied = block.copy(); - assert_eq!(unsafe { invoke_int_block(&copied) }, expected_len); + let closure = move || s.len() as i32; + + for block in [StackBlock::new(closure.clone()), unsafe { + StackBlock::with_encoding::(closure) + }] { + assert_eq!(unsafe { invoke_int_block(&block) }, expected_len); + let copied = block.copy(); + assert_eq!(unsafe { invoke_int_block(&copied) }, expected_len); + } } #[test] @@ -225,9 +311,17 @@ fn test_block_stack_move() { let x = 7; StackBlock::new(move || x) } + fn make_block_with_encoding() -> StackBlock<'static, (), i32, impl Fn() -> i32> { + let x = 7; + unsafe { StackBlock::with_encoding::(move || x) } + } - let block = make_block(); - assert_eq!(unsafe { invoke_int_block(&block) }, 7); + for block in [ + &make_block() as &Block i32>, + &make_block_with_encoding() as &Block i32>, + ] { + assert_eq!(unsafe { invoke_int_block(block) }, 7); + } } #[derive(Default, Debug, Clone, PartialEq, Eq, Hash)] @@ -289,117 +383,153 @@ impl Drop for CloneDropTracker { #[test] fn stack_new_clone_drop() { - let mut expected = Count::current(); - - let counter = CloneDropTracker::new(); - expected.new += 1; - expected.assert_current(); - - let block = StackBlock::new(move || { - let _ = &counter; - }); - expected.assert_current(); - - let clone = block.clone(); - expected.clone += 1; - expected.assert_current(); - - drop(clone); - expected.drop += 1; - expected.assert_current(); + macro_rules! test_with { + ($ctor:path) => {{ + let mut expected = Count::current(); + let counter = CloneDropTracker::new(); + expected.new += 1; + expected.assert_current(); + + #[allow(unused_unsafe)] + let block = unsafe { + $ctor(move || { + let _ = &counter; + }) + }; + expected.assert_current(); + + let clone = block.clone(); + expected.clone += 1; + expected.assert_current(); + + drop(clone); + expected.drop += 1; + expected.assert_current(); + + drop(block); + expected.drop += 1; + expected.assert_current(); + }}; + } - drop(block); - expected.drop += 1; - expected.assert_current(); + test_with!(StackBlock::new); + test_with!(StackBlock::with_encoding::); } #[test] fn rc_new_clone_drop() { - let mut expected = Count::current(); - - let counter = CloneDropTracker::new(); - expected.new += 1; - expected.assert_current(); - - let block = RcBlock::new(move || { - let _ = &counter; - }); - expected.assert_current(); - - let clone = block.clone(); - expected.assert_current(); - - drop(clone); - expected.assert_current(); + macro_rules! test_with { + ($ctor:path) => {{ + let mut expected = Count::current(); + let counter = CloneDropTracker::new(); + expected.new += 1; + expected.assert_current(); + + #[allow(unused_unsafe)] + let block = unsafe { + $ctor(move || { + let _ = &counter; + }) + }; + expected.assert_current(); + + let clone = block.clone(); + expected.assert_current(); + + drop(clone); + expected.assert_current(); + + drop(block); + expected.drop += 1; + expected.assert_current(); + }}; + } - drop(block); - expected.drop += 1; - expected.assert_current(); + test_with!(RcBlock::new); + test_with!(RcBlock::with_encoding::<_, _, _, VoidToVoid>); } #[test] fn stack_to_rc() { - let mut expected = Count::current(); - - let counter = CloneDropTracker::new(); - expected.new += 1; - expected.assert_current(); - - let stack = StackBlock::new(move || { - let _ = &counter; - }); - expected.assert_current(); - - let rc1 = stack.copy(); - expected.clone += 1; - expected.assert_current(); - - let rc2 = stack.copy(); - expected.clone += 1; - expected.assert_current(); - - let clone2 = rc2.clone(); - expected.assert_current(); - - drop(rc2); - expected.assert_current(); - - drop(stack); - expected.drop += 1; - expected.assert_current(); - - drop(rc1); - expected.drop += 1; - expected.assert_current(); + macro_rules! test_with { + ($ctor:path) => {{ + let mut expected = Count::current(); + let counter = CloneDropTracker::new(); + expected.new += 1; + expected.assert_current(); + + #[allow(unused_unsafe)] + let stack = unsafe { + $ctor(move || { + let _ = &counter; + }) + }; + expected.assert_current(); + + let rc1 = stack.copy(); + expected.clone += 1; + expected.assert_current(); + + let rc2 = stack.copy(); + expected.clone += 1; + expected.assert_current(); + + let clone2 = rc2.clone(); + expected.assert_current(); + + drop(rc2); + expected.assert_current(); + + drop(stack); + expected.drop += 1; + expected.assert_current(); + + drop(rc1); + expected.drop += 1; + expected.assert_current(); + + drop(clone2); + expected.drop += 1; + expected.assert_current(); + }}; + } - drop(clone2); - expected.drop += 1; - expected.assert_current(); + test_with!(StackBlock::new); + test_with!(StackBlock::with_encoding::); } #[test] fn retain_release_rc_block() { - let mut expected = Count::current(); - - let counter = CloneDropTracker::new(); - expected.new += 1; - expected.assert_current(); - - let block = RcBlock::new(move || { - let _ = &counter; - }); - expected.assert_current(); - - let ptr = &*block as *const Block<_> as *mut AnyObject; - let obj = unsafe { Retained::retain(ptr) }.unwrap(); - expected.assert_current(); - - drop(block); - expected.assert_current(); + macro_rules! test_with { + ($ctor:path) => {{ + let mut expected = Count::current(); + let counter = CloneDropTracker::new(); + expected.new += 1; + expected.assert_current(); + + #[allow(unused_unsafe)] + let block = unsafe { + $ctor(move || { + let _ = &counter; + }) + }; + expected.assert_current(); + + let ptr = &*block as *const Block<_> as *mut AnyObject; + let obj = unsafe { Retained::retain(ptr) }.unwrap(); + expected.assert_current(); + + drop(block); + expected.assert_current(); + + drop(obj); + expected.drop += 1; + expected.assert_current(); + }}; + } - drop(obj); - expected.drop += 1; - expected.assert_current(); + test_with!(RcBlock::new); + test_with!(RcBlock::with_encoding::<_, _, _, VoidToVoid>); } /// Retaining/releasing stack blocks is kinda weird and unsupported. @@ -447,13 +577,27 @@ fn retain_release_stack_block() { #[test] fn capture_id() { - let stack_block = { - let obj1 = NSObject::new(); - let obj2 = NSObject::new(); - StackBlock::new(move || Bool::new(obj1 == obj2)) - }; - - let rc_block = stack_block.copy(); + let obj1 = NSObject::new(); + let obj2 = NSObject::new(); + let closure = move || Bool::new(obj1 == obj2); + + struct Enc; + unsafe impl ManualBlockEncoding for Enc { + type Arguments = (); + type Return = Bool; + + #[cfg(all(target_os = "macos", target_arch = "x86_64"))] + const ENCODING_CSTR: &'static CStr = c"c8@?0"; + #[cfg(all(target_os = "linux", target_pointer_width = "64"))] + const ENCODING_CSTR: &'static CStr = c"C8@?0"; + #[cfg(all(target_os = "linux", target_pointer_width = "32"))] + const ENCODING_CSTR: &'static CStr = c"C4@?0"; + } - assert!(rc_block.call(()).is_false()); + for stack_block in [StackBlock::new(closure.clone()), unsafe { + StackBlock::with_encoding::(closure) + }] { + let rc_block = stack_block.copy(); + assert!(rc_block.call(()).is_false()); + } } From af13f708e22d21121d93e64e575455e819ad6705 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Tue, 17 Sep 2024 20:27:14 +0200 Subject: [PATCH 08/14] Make test more generic --- crates/tests/src/block.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/crates/tests/src/block.rs b/crates/tests/src/block.rs index d73e00dab..9e11a216a 100644 --- a/crates/tests/src/block.rs +++ b/crates/tests/src/block.rs @@ -586,12 +586,17 @@ fn capture_id() { type Arguments = (); type Return = Bool; - #[cfg(all(target_os = "macos", target_arch = "x86_64"))] - const ENCODING_CSTR: &'static CStr = c"c8@?0"; - #[cfg(all(target_os = "linux", target_pointer_width = "64"))] - const ENCODING_CSTR: &'static CStr = c"C8@?0"; - #[cfg(all(target_os = "linux", target_pointer_width = "32"))] - const ENCODING_CSTR: &'static CStr = c"C4@?0"; + const ENCODING_CSTR: &'static CStr = { + match (Bool::ENCODING, cfg!(target_pointer_width = "64")) { + (Encoding::Char, true) => c"c8@?0", + (Encoding::UChar, true) => c"C8@?0", + (Encoding::Bool, true) => c"b8@?0", + (Encoding::Char, false) => c"c4@?0", + (Encoding::UChar, false) => c"C4@?0", + (Encoding::Bool, false) => c"b4@?0", + _ => panic!("invalid Bool encoding"), + } + }; } for stack_block in [StackBlock::new(closure.clone()), unsafe { From 48d3aa1b3e8aad047a9cf81843cd9ba032931d5c Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Tue, 17 Sep 2024 20:28:56 +0200 Subject: [PATCH 09/14] Avoid opt_double! macro, since it's unnecessary --- crates/objc2-encode/src/helper.rs | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/crates/objc2-encode/src/helper.rs b/crates/objc2-encode/src/helper.rs index 6423fbb12..16cf4ffc4 100644 --- a/crates/objc2-encode/src/helper.rs +++ b/crates/objc2-encode/src/helper.rs @@ -175,14 +175,6 @@ impl Primitive { } pub(crate) const fn size(self) -> Option { - macro_rules! opt_double { - ($e:expr) => { - match $e { - Some(x) => Some(2 * x), - None => None, - } - }; - } match self { // Under all the considered targets, `_Bool` is sized and aligned // to a single byte. See: @@ -217,9 +209,12 @@ impl Primitive { all(target_arch = "aarch64", target_vendor = "apple"), ))] Self::LongDouble => Some(8), - Self::FloatComplex => opt_double!(Self::Float.size()), - Self::DoubleComplex => opt_double!(Self::Double.size()), - Self::LongDoubleComplex => opt_double!(Self::LongDouble.size()), + Self::FloatComplex => Some(mem::size_of::() * 2), + Self::DoubleComplex => Some(mem::size_of::() * 2), + Self::LongDoubleComplex => match Self::LongDouble.size() { + Some(size) => Some(size * 2), + None => None, + }, // Pointers. Self::String | Self::Object | Self::Block | Self::Class | Self::Sel => { Some(mem::size_of::<*const ()>()) From 2b47bf61661e162a5f3df4ffbff7638fa288239a Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Tue, 17 Sep 2024 20:29:20 +0200 Subject: [PATCH 10/14] Don't mention block_signature_string, it's not public --- crates/block2/src/traits.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/block2/src/traits.rs b/crates/block2/src/traits.rs index 4902d1b9c..5d6c1ab2a 100644 --- a/crates/block2/src/traits.rs +++ b/crates/block2/src/traits.rs @@ -179,8 +179,7 @@ impl_traits!(t0: T0, t1: T1, t2: T2, t3: T3, t4: T4, t5: T5, t6: T6, t7: T7, t8: /// Another possibility is to help yourself with [`Encoding::to_string`][enc2s] /// in order to get the various components of the signature string separately /// and then concatenate them manually with the required numbers described -/// below inserted at their correct place. `encoding::block_signature_string` -/// can help automate this particular process, still not at compile time though. +/// below inserted at their correct place. /// /// Yet another possibility is to compile a small Objective-C program, with GCC /// and GNUstep for example, that simply displays the result of From 23fceed1d3d09073f3842ca140310a2852d6d4f1 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Tue, 17 Sep 2024 20:32:06 +0200 Subject: [PATCH 11/14] All Encoding::size to use alignment in the future --- crates/objc2-encode/src/encoding.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/objc2-encode/src/encoding.rs b/crates/objc2-encode/src/encoding.rs index 15486292d..32823aff8 100644 --- a/crates/objc2-encode/src/encoding.rs +++ b/crates/objc2-encode/src/encoding.rs @@ -251,8 +251,10 @@ impl Encoding { /// Computes the theoretical size in bytes of the represented value type. /// - /// Everything is considered packed, i.e. no alignment is computed. Also, - /// the result is only valid for the current build target. + /// The size is only valid for the current target. + /// + /// This does not currently consider alignment, i.e. everything is + /// considered packed, but that may change in the future. pub fn size(&self) -> Option { Helper::new(self).size(NestingLevel::new()) } From 0acb681ad9158c673ff6a8700cc56750a4c67bfb Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Tue, 17 Sep 2024 20:54:14 +0200 Subject: [PATCH 12/14] Make `with_encoding` safe The `ManualBlockEncoding` is already `unsafe`, and contains the block's arguments and return type, so we can use that to mark the actual method that the user calls safe. --- crates/block2/src/rc_block.rs | 33 ++++----- crates/block2/src/stack.rs | 74 +++++++++---------- crates/block2/src/traits.rs | 4 + .../test-ui/ui/block_lifetimes_independent.rs | 8 +- .../ui/block_lifetimes_independent.stderr | 32 ++++---- .../ui/lifetime_of_closure_tied_to_block.rs | 8 +- .../lifetime_of_closure_tied_to_block.stderr | 40 +++++----- ...tack_block_with_encoding_requires_clone.rs | 8 +- ..._block_with_encoding_requires_clone.stderr | 28 +++---- crates/tests/src/block.rs | 50 +++++-------- 10 files changed, 134 insertions(+), 151 deletions(-) diff --git a/crates/block2/src/rc_block.rs b/crates/block2/src/rc_block.rs index 170049aa6..2b11c4db9 100644 --- a/crates/block2/src/rc_block.rs +++ b/crates/block2/src/rc_block.rs @@ -101,8 +101,7 @@ impl RcBlock { R: EncodeReturn, Closure: IntoBlock<'f, A, R, Dyn = F>, { - // SAFETY: no encoding is given. - unsafe { Self::maybe_encoded::<_, _, _, NoBlockEncoding>(closure) } + Self::maybe_encoded::<_, _, _, NoBlockEncoding>(closure) } /// Constructs a new [`RcBlock`] with the given function and encoding @@ -111,12 +110,6 @@ impl RcBlock { /// 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 RcBlock { /// # 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, { - // SAFETY: supposed to be upheld by the caller. - unsafe { Self::maybe_encoded::<_, _, _, UserSpecified>(closure) } + Self::maybe_encoded::<_, _, _, UserSpecified>(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 RcBlock { // // Clang doesn't do this optimization either. // - // - // Encoding safety is supposed to be upheld by the caller. let block = unsafe { StackBlock::new_no_clone::(closure) }; // Transfer ownership from the stack to the heap. diff --git a/crates/block2/src/stack.rs b/crates/block2/src/stack.rs index cd6220b62..42f88fec5 100644 --- a/crates/block2/src/stack.rs +++ b/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::>(closure) } + Self::maybe_encoded::>(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::(|_err: *mut NSError| { - /// 42i32 - /// }) - /// }; + /// let my_block = StackBlock::with_encoding::(|_err: *mut NSError| { + /// 42i32 + /// }); /// assert_eq!(my_block.call((std::ptr::null_mut(),)), 42); /// ``` #[inline] - pub unsafe fn with_encoding(closure: Closure) -> Self + pub fn with_encoding(closure: Closure) -> Self where E: ManualBlockEncoding, { - // SAFETY: supposed to be upheld by the caller. - unsafe { Self::maybe_encoded::>(closure) } + Self::maybe_encoded::>(closure) } #[inline] - unsafe fn maybe_encoded(closure: Closure) -> Self + fn maybe_encoded(closure: Closure) -> Self where E: ManualBlockEncodingExt, { @@ -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(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::() { - if E::IS_NONE { + let descriptor = match (mem::needs_drop::(), 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> { &>::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: &>::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: + &>::DESCRIPTOR_BASIC_WITH_ENCODING, + } } }; diff --git a/crates/block2/src/traits.rs b/crates/block2/src/traits.rs index 5d6c1ab2a..845d5df13 100644 --- a/crates/block2/src/traits.rs +++ b/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, } +// SAFETY: The encoding here is incorrect, but it will never be used because +// we specify `IS_NONE = true` in `ManualBlockEncodingExt`. unsafe impl ManualBlockEncoding for NoBlockEncoding where A: EncodeArguments, diff --git a/crates/test-ui/ui/block_lifetimes_independent.rs b/crates/test-ui/ui/block_lifetimes_independent.rs index e35a5500d..ed7f37ada 100644 --- a/crates/test-ui/ui/block_lifetimes_independent.rs +++ b/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 { @@ -56,7 +56,7 @@ fn args_entire_with_encoding<'a, 'b>(f: impl Fn(&'a i32) + 'b) -> RcBlock>(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() {} diff --git a/crates/test-ui/ui/block_lifetimes_independent.stderr b/crates/test-ui/ui/block_lifetimes_independent.stderr index 94aa8f600..db60f6242 100644 --- a/crates/test-ui/ui/block_lifetimes_independent.stderr +++ b/crates/test-ui/ui/block_lifetimes_independent.stderr @@ -108,8 +108,8 @@ error: lifetime may not live long enough | | | lifetime `'a` defined here ... - | unsafe { RcBlock::with_encoding::<_, _, _, Enc<'a, 'b>>(f) } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'b` but it is returning data with lifetime `'a` + | RcBlock::with_encoding::<_, _, _, Enc<'a, 'b>>(f) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'b` but it is returning data with lifetime `'a` | = help: consider adding the following bound: `'a: 'b` @@ -121,8 +121,8 @@ error: lifetime may not live long enough | | | lifetime `'a` defined here ... - | unsafe { RcBlock::with_encoding::<_, _, _, Enc<'a, 'b>>(f) } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'a` but it is returning data with lifetime `'b` + | RcBlock::with_encoding::<_, _, _, Enc<'a, 'b>>(f) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'a` but it is returning data with lifetime `'b` | = help: consider adding the following bound: `'b: 'a` @@ -134,8 +134,8 @@ error: lifetime may not live long enough | | | lifetime `'a` defined here ... - | unsafe { RcBlock::with_encoding::<_, _, _, Enc<'a, 'b>>(f) } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'b` but it is returning data with lifetime `'a` + | RcBlock::with_encoding::<_, _, _, Enc<'a, 'b>>(f) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'b` but it is returning data with lifetime `'a` | = help: consider adding the following bound: `'a: 'b` @@ -147,8 +147,8 @@ error: lifetime may not live long enough | | | lifetime `'a` defined here ... - | unsafe { RcBlock::with_encoding::<_, _, _, Enc<'a, 'b>>(f) } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'a` but it is returning data with lifetime `'b` + | RcBlock::with_encoding::<_, _, _, Enc<'a, 'b>>(f) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'a` but it is returning data with lifetime `'b` | = help: consider adding the following bound: `'b: 'a` @@ -160,8 +160,8 @@ error: lifetime may not live long enough | | | lifetime `'a` defined here ... - | unsafe { RcBlock::with_encoding::<_, _, _, Enc<'a>>(f) } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'b` but it is returning data with lifetime `'a` + | RcBlock::with_encoding::<_, _, _, Enc<'a>>(f) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'b` but it is returning data with lifetime `'a` | = help: consider adding the following bound: `'a: 'b` @@ -173,8 +173,8 @@ error: lifetime may not live long enough | | | lifetime `'a` defined here ... - | unsafe { RcBlock::with_encoding::<_, _, _, Enc<'a>>(f) } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'a` but it is returning data with lifetime `'b` + | RcBlock::with_encoding::<_, _, _, Enc<'a>>(f) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'a` but it is returning data with lifetime `'b` | = help: consider adding the following bound: `'b: 'a` @@ -186,8 +186,8 @@ error: lifetime may not live long enough | | | lifetime `'a` defined here ... - | unsafe { RcBlock::with_encoding::<_, _, _, Enc<'a>>(f) } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'b` but it is returning data with lifetime `'a` + | RcBlock::with_encoding::<_, _, _, Enc<'a>>(f) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'b` but it is returning data with lifetime `'a` | = help: consider adding the following bound: `'a: 'b` @@ -199,7 +199,7 @@ error: lifetime may not live long enough | | | lifetime `'a` defined here ... - | unsafe { RcBlock::with_encoding::<_, _, _, Enc<'a>>(f) } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'a` but it is returning data with lifetime `'b` + | RcBlock::with_encoding::<_, _, _, Enc<'a>>(f) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'a` but it is returning data with lifetime `'b` | = help: consider adding the following bound: `'b: 'a` diff --git a/crates/test-ui/ui/lifetime_of_closure_tied_to_block.rs b/crates/test-ui/ui/lifetime_of_closure_tied_to_block.rs index 6da1cfc69..daaa6d18f 100644 --- a/crates/test-ui/ui/lifetime_of_closure_tied_to_block.rs +++ b/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::(|| x + 2) } + StackBlock::with_encoding::(|| x + 2) }; let _ = { let x = 2; - unsafe { StackBlock::with_encoding::(|| x + 2).copy() } + StackBlock::with_encoding::(|| x + 2).copy() }; } diff --git a/crates/test-ui/ui/lifetime_of_closure_tied_to_block.stderr b/crates/test-ui/ui/lifetime_of_closure_tied_to_block.stderr index 02ea98624..7ea76b246 100644 --- a/crates/test-ui/ui/lifetime_of_closure_tied_to_block.stderr +++ b/crates/test-ui/ui/lifetime_of_closure_tied_to_block.stderr @@ -36,10 +36,10 @@ error[E0597]: `x` does not live long enough | | let x = 2; | - binding `x` declared here - | unsafe { RcBlock::with_encoding::<_, _, _, VoidToI32>(|| x + 2) } - | -- ^ borrowed value does not live long enough - | | - | value captured here + | RcBlock::with_encoding::<_, _, _, VoidToI32>(|| x + 2) + | -- ^ borrowed value does not live long enough + | | + | value captured here | }; | - `x` dropped here while still borrowed @@ -48,12 +48,12 @@ error[E0597]: `x` does not live long enough | | let x = 2; | - binding `x` declared here - | unsafe { RcBlock::with_encoding::<_, _, _, VoidToI32>(|| x + 2).clone() } - | ------------------------------------------------^----- - | | | | - | | | borrowed value does not live long enough - | | value captured here - | a temporary with access to the borrow is created here ... + | RcBlock::with_encoding::<_, _, _, VoidToI32>(|| x + 2).clone() + | ------------------------------------------------^----- + | | | | + | | | borrowed value does not live long enough + | | value captured here + | a temporary with access to the borrow is created here ... | }; | -- ... and the borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `RcBlock` | | @@ -61,8 +61,8 @@ error[E0597]: `x` does not live long enough | help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped | -29 | unsafe { RcBlock::with_encoding::<_, _, _, VoidToI32>(|| x + 2).clone(); } - | + +29 | RcBlock::with_encoding::<_, _, _, VoidToI32>(|| x + 2).clone(); + | + error[E0597]: `x` does not live long enough --> ui/lifetime_of_closure_tied_to_block.rs @@ -93,10 +93,10 @@ error[E0597]: `x` does not live long enough | | let x = 2; | - binding `x` declared here - | unsafe { StackBlock::with_encoding::(|| x + 2) } - | -- ^ borrowed value does not live long enough - | | - | value captured here + | StackBlock::with_encoding::(|| x + 2) + | -- ^ borrowed value does not live long enough + | | + | value captured here | }; | - `x` dropped here while still borrowed @@ -105,9 +105,9 @@ error[E0597]: `x` does not live long enough | | let x = 2; | - binding `x` declared here - | unsafe { StackBlock::with_encoding::(|| x + 2).copy() } - | -- ^ borrowed value does not live long enough - | | - | value captured here + | StackBlock::with_encoding::(|| x + 2).copy() + | -- ^ borrowed value does not live long enough + | | + | value captured here | }; | - `x` dropped here while still borrowed diff --git a/crates/test-ui/ui/stack_block_with_encoding_requires_clone.rs b/crates/test-ui/ui/stack_block_with_encoding_requires_clone.rs index bd122a2eb..82bbe2420 100644 --- a/crates/test-ui/ui/stack_block_with_encoding_requires_clone.rs +++ b/crates/test-ui/ui/stack_block_with_encoding_requires_clone.rs @@ -12,9 +12,7 @@ fn main() { } let foo = Foo; - let _ = unsafe { - StackBlock::with_encoding::(move || { - let _ = &foo; - }) - }; + let _ = StackBlock::with_encoding::(move || { + let _ = &foo; + }); } diff --git a/crates/test-ui/ui/stack_block_with_encoding_requires_clone.stderr b/crates/test-ui/ui/stack_block_with_encoding_requires_clone.stderr index d615bf2cd..e190cf94c 100644 --- a/crates/test-ui/ui/stack_block_with_encoding_requires_clone.stderr +++ b/crates/test-ui/ui/stack_block_with_encoding_requires_clone.stderr @@ -1,29 +1,29 @@ -error[E0277]: the trait bound `Foo: Clone` is not satisfied in `{closure@$DIR/ui/stack_block_with_encoding_requires_clone.rs:16:55: 16:62}` +error[E0277]: the trait bound `Foo: Clone` is not satisfied in `{closure@$DIR/ui/stack_block_with_encoding_requires_clone.rs:15:59: 15:66}` --> ui/stack_block_with_encoding_requires_clone.rs | - | StackBlock::with_encoding::(move || { - | --------------------------------------------- ^------ - | | | - | _________|_____________________________________________within this `{closure@$DIR/ui/stack_block_with_encoding_requires_clone.rs:16:55: 16:62}` - | | | - | | required by a bound introduced by this call - | | let _ = &foo; - | | }) - | |_________^ within `{closure@$DIR/ui/stack_block_with_encoding_requires_clone.rs:16:55: 16:62}`, the trait `Clone` is not implemented for `Foo`, which is required by `{closure@$DIR/ui/stack_block_with_encoding_requires_clone.rs:16:55: 16:62}: Clone` + | let _ = StackBlock::with_encoding::(move || { + | --------------------------------------------- ^------ + | | | + | _____________|_____________________________________________within this `{closure@$DIR/ui/stack_block_with_encoding_requires_clone.rs:15:59: 15:66}` + | | | + | | required by a bound introduced by this call + | | let _ = &foo; + | | }); + | |_____^ within `{closure@$DIR/ui/stack_block_with_encoding_requires_clone.rs:15:59: 15:66}`, the trait `Clone` is not implemented for `Foo`, which is required by `{closure@$DIR/ui/stack_block_with_encoding_requires_clone.rs:15:59: 15:66}: Clone` | note: required because it's used within this closure --> ui/stack_block_with_encoding_requires_clone.rs | - | StackBlock::with_encoding::(move || { - | ^^^^^^^ + | let _ = StackBlock::with_encoding::(move || { + | ^^^^^^^ note: required by a bound in `StackBlock::<'f, A, R, Closure>::with_encoding` --> $WORKSPACE/crates/block2/src/stack.rs | | Closure: IntoBlock<'f, A, R> + Clone, | ^^^^^ required by this bound in `StackBlock::<'f, A, R, Closure>::with_encoding` ... - | pub unsafe fn with_encoding(closure: Closure) -> Self - | ------------- required by a bound in this associated function + | pub fn with_encoding(closure: Closure) -> Self + | ------------- required by a bound in this associated function help: consider annotating `Foo` with `#[derive(Clone)]` | 4 + #[derive(Clone)] diff --git a/crates/tests/src/block.rs b/crates/tests/src/block.rs index 9e11a216a..e1fa31c72 100644 --- a/crates/tests/src/block.rs +++ b/crates/tests/src/block.rs @@ -139,14 +139,8 @@ fn test_int_block() { ); invoke_assert(&StackBlock::new(|| 10), 10); invoke_assert(&RcBlock::new(|| 6), 6); - invoke_assert( - unsafe { &StackBlock::with_encoding::(|| 10) }, - 10, - ); - invoke_assert( - unsafe { &RcBlock::with_encoding::<_, _, _, VoidToInt>(|| 6) }, - 6, - ); + invoke_assert(&StackBlock::with_encoding::(|| 10), 10); + invoke_assert(&RcBlock::with_encoding::<_, _, _, VoidToInt>(|| 6), 6); invoke_assert(&GLOBAL_BLOCK, 42); } @@ -171,12 +165,9 @@ fn test_add_block() { ); invoke_assert(&StackBlock::new(|a: i32| a + 6), 11); invoke_assert(&RcBlock::new(|a: i32| a + 6), 11); + invoke_assert(&StackBlock::with_encoding::(|a: i32| a + 6), 11); invoke_assert( - unsafe { &StackBlock::with_encoding::(|a: i32| a + 6) }, - 11, - ); - invoke_assert( - unsafe { &RcBlock::with_encoding::<_, _, _, IntToInt>(|a: i32| a + 6) }, + &RcBlock::with_encoding::<_, _, _, IntToInt>(|a: i32| a + 6), 11, ); invoke_assert(&GLOBAL_BLOCK, 47); @@ -223,11 +214,8 @@ fn test_add_12() { }; invoke_assert(&StackBlock::new(closure), 78); invoke_assert(&RcBlock::new(closure), 78); - invoke_assert(unsafe { &StackBlock::with_encoding::(closure) }, 78); - invoke_assert( - unsafe { &RcBlock::with_encoding::<_, _, _, Enc>(closure) }, - 78, - ); + invoke_assert(&StackBlock::with_encoding::(closure), 78); + invoke_assert(&RcBlock::with_encoding::<_, _, _, Enc>(closure), 78); invoke_assert(&GLOBAL_BLOCK, 120); } @@ -279,12 +267,10 @@ fn test_large_struct_block() { let block = block.copy(); assert_eq!(unsafe { invoke_large_struct_block(&block, data) }, new_data); - let block = unsafe { - StackBlock::with_encoding::(|mut x: LargeStruct| { - x.mutate(); - x - }) - }; + let block = StackBlock::with_encoding::(|mut x: LargeStruct| { + x.mutate(); + x + }); assert_eq!(unsafe { invoke_large_struct_block(&block, data) }, new_data); let block = block.copy(); assert_eq!(unsafe { invoke_large_struct_block(&block, data) }, new_data); @@ -296,9 +282,10 @@ fn test_block_copy() { let expected_len = s.len() as i32; let closure = move || s.len() as i32; - for block in [StackBlock::new(closure.clone()), unsafe { - StackBlock::with_encoding::(closure) - }] { + for block in [ + StackBlock::new(closure.clone()), + StackBlock::with_encoding::(closure), + ] { assert_eq!(unsafe { invoke_int_block(&block) }, expected_len); let copied = block.copy(); assert_eq!(unsafe { invoke_int_block(&copied) }, expected_len); @@ -313,7 +300,7 @@ fn test_block_stack_move() { } fn make_block_with_encoding() -> StackBlock<'static, (), i32, impl Fn() -> i32> { let x = 7; - unsafe { StackBlock::with_encoding::(move || x) } + StackBlock::with_encoding::(move || x) } for block in [ @@ -599,9 +586,10 @@ fn capture_id() { }; } - for stack_block in [StackBlock::new(closure.clone()), unsafe { - StackBlock::with_encoding::(closure) - }] { + for stack_block in [ + StackBlock::new(closure.clone()), + StackBlock::with_encoding::(closure), + ] { let rc_block = stack_block.copy(); assert!(rc_block.call(()).is_false()); } From 00173dde557162839e1f67f5d06fbb1c2e44ef6d Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Tue, 17 Sep 2024 20:58:16 +0200 Subject: [PATCH 13/14] Add changelog entries --- crates/block2/CHANGELOG.md | 4 ++++ crates/objc2-encode/CHANGELOG.md | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/crates/block2/CHANGELOG.md b/crates/block2/CHANGELOG.md index 7d996c2f8..224509a70 100644 --- a/crates/block2/CHANGELOG.md +++ b/crates/block2/CHANGELOG.md @@ -8,6 +8,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Added * Added `unstable-c-unwind` feature flag. +* Added `StackBlock::with_encoding` and `RcBlock::with_encoding` for creating + blocks with an encoding specified by `ManualBlockEncoding`. + + This is useful for certain APIs that require blocks to have an encoding. ## 0.5.1 - 2024-05-21 diff --git a/crates/objc2-encode/CHANGELOG.md b/crates/objc2-encode/CHANGELOG.md index d02e9b1c6..763cfe25f 100644 --- a/crates/objc2-encode/CHANGELOG.md +++ b/crates/objc2-encode/CHANGELOG.md @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## Unreleased - YYYY-MM-DD +### Added +* Added `Encoding::size`, which computes the size of the encoded type for the + current target. + ## 4.0.3 - 2024-05-21 From b132646328e0dfdbfb8edf7aac38b22c80e09ce0 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Tue, 17 Sep 2024 21:01:17 +0200 Subject: [PATCH 14/14] Fix test on Aarch64 --- crates/block2/src/encoding.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/block2/src/encoding.rs b/crates/block2/src/encoding.rs index 569411f58..cf25af2c0 100644 --- a/crates/block2/src/encoding.rs +++ b/crates/block2/src/encoding.rs @@ -131,8 +131,13 @@ mod tests { ], &Encoding::String, // Probably unaligned. - #[cfg(any(target_arch = "x86_64", target_arch = "aarch64",))] + #[cfg(any( + target_arch = "x86_64", + all(target_arch = "aarch64", not(target_vendor = "apple")) + ))] "*53@?0(ThisOrThat=Si)8{ThisAndThat=QjDAB}12", + #[cfg(all(target_arch = "aarch64", target_vendor = "apple"))] + "*37@?0(ThisOrThat=Si)8{ThisAndThat=QjDAB}12", #[cfg(all(target_arch = "x86", target_vendor = "apple"))] "*45@?0(ThisOrThat=Si)4{ThisAndThat=QjDAB}8", #[cfg(all(target_arch = "x86", not(target_vendor = "apple")))]