From ec7c8f12f864fd9eee7ed3a9779c5f95f3969182 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Wed, 24 Jan 2024 22:08:50 +0100 Subject: [PATCH] Change syntax for blocks, and allow them to carry a lifetime --- crates/block2/CHANGELOG.md | 17 +- crates/block2/src/block.rs | 167 +++++++++++++++--- crates/block2/src/global.rs | 98 +++++----- crates/block2/src/lib.rs | 144 +++++++-------- crates/block2/src/rc_block.rs | 103 ++++++++--- crates/block2/src/stack.rs | 128 ++++++++------ crates/block2/src/traits.rs | 73 +++++--- crates/header-translator/src/rust_type.rs | 18 +- .../icrate/src/additions/Foundation/data.rs | 2 +- crates/icrate/src/generated | 2 +- crates/objc2/src/rc/test_object.rs | 2 + .../test_block/expected/apple-aarch64.s | 2 +- .../crates/test_block/expected/apple-x86_64.s | 2 +- crates/test-assembly/crates/test_block/lib.rs | 10 +- crates/test-ui/ui/block_carrying_lifetime.rs | 15 ++ .../test-ui/ui/block_carrying_lifetime.stderr | 20 +++ .../test-ui/ui/block_inner_wrong_variance.rs | 34 ++++ .../ui/block_inner_wrong_variance.stderr | 84 +++++++++ .../test-ui/ui/block_lifetimes_independent.rs | 26 +++ .../ui/block_lifetimes_independent.stderr | 101 +++++++++++ crates/test-ui/ui/block_parameter_lifetime.rs | 15 ++ .../ui/block_parameter_lifetime.stderr | 44 +++++ .../test-ui/ui/global_block_args_covariant.rs | 2 +- .../ui/global_block_args_covariant.stderr | 10 +- .../test-ui/ui/global_block_not_encode.stderr | 4 +- .../ui/lifetime_of_closure_tied_to_block.rs | 23 +++ .../lifetime_of_closure_tied_to_block.stderr | 48 +++++ crates/test-ui/ui/not_encode.rs | 4 +- crates/test-ui/ui/not_encode.stderr | 40 ++++- .../test-ui/ui/stack_block_requires_clone.rs | 10 ++ .../ui/stack_block_requires_clone.stderr | 31 ++++ crates/tests/src/block.rs | 24 ++- crates/tests/src/ffi.rs | 22 +-- crates/tests/src/lib.rs | 26 ++- 34 files changed, 1034 insertions(+), 317 deletions(-) create mode 100644 crates/test-ui/ui/block_carrying_lifetime.rs create mode 100644 crates/test-ui/ui/block_carrying_lifetime.stderr create mode 100644 crates/test-ui/ui/block_inner_wrong_variance.rs create mode 100644 crates/test-ui/ui/block_inner_wrong_variance.stderr create mode 100644 crates/test-ui/ui/block_lifetimes_independent.rs create mode 100644 crates/test-ui/ui/block_lifetimes_independent.stderr create mode 100644 crates/test-ui/ui/block_parameter_lifetime.rs create mode 100644 crates/test-ui/ui/block_parameter_lifetime.stderr create mode 100644 crates/test-ui/ui/lifetime_of_closure_tied_to_block.rs create mode 100644 crates/test-ui/ui/lifetime_of_closure_tied_to_block.stderr create mode 100644 crates/test-ui/ui/stack_block_requires_clone.rs create mode 100644 crates/test-ui/ui/stack_block_requires_clone.stderr diff --git a/crates/block2/CHANGELOG.md b/crates/block2/CHANGELOG.md index 6d441de05..c1f7a3da8 100644 --- a/crates/block2/CHANGELOG.md +++ b/crates/block2/CHANGELOG.md @@ -8,8 +8,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Added * Added `RcBlock::new(closure)` as a more efficient and flexible alternative - to `StackBlock::new(closure).to_rc()`. -* Added `StackBlock::to_rc` to convert stack blocks to `RcBlock`. + to `StackBlock::new(closure).copy()`. +* **BREAKING**: Added `Block::copy` to convert blocks to `RcBlock`. This + replaces `StackBlock::copy`, but since `StackBlock` implements `Deref`, this + will likely work as before. ### Changed * **BREAKING**: Renamed `RcBlock::new(ptr)` to `RcBlock::from_raw(ptr)`. @@ -19,15 +21,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). `_Block_release`, `_Block_object_assign`, `_Block_object_dispose`, `_NSConcreteGlobalBlock`, `_NSConcreteStackBlock` and `Class` in `ffi` module. -* **BREAKING**: Renamed `IntoConcreteBlock` to `IntoBlock`, and moved - associated type `Output` to be a generic parameter. +* **BREAKING**: Renamed `IntoConcreteBlock` to `IntoBlock`, moved + associated type `Output` to be a generic parameter, and added lifetime + parameter. * No longer use the `block-sys` crate for linking to the blocks runtime. -* Renamed `ConcreteBlock` to `StackBlock`. The old name is deprecated. +* Renamed `ConcreteBlock` to `StackBlock`, and added a lifetime parameter. The + old name is deprecated. * Added `Copy` implementation for `StackBlock`. -### Deprecated -* Deprecated `StackBlock::copy`, it is no longer necessary. - ### Fixed * **BREAKING**: `StackBlock::new` now requires the closure to be `Clone`. If this is not desired, use `RcBlock::new` instead. diff --git a/crates/block2/src/block.rs b/crates/block2/src/block.rs index cb118f48b..89c1cd8cc 100644 --- a/crates/block2/src/block.rs +++ b/crates/block2/src/block.rs @@ -1,11 +1,13 @@ use core::fmt; use core::marker::PhantomData; +use core::ptr::NonNull; -use objc2::encode::{EncodeReturn, Encoding, RefEncode}; +use objc2::encode::{Encoding, RefEncode}; use crate::abi::BlockHeader; use crate::debug::debug_block_header; -use crate::BlockArguments; +use crate::rc_block::block_copy_fail; +use crate::{BlockFn, RcBlock}; /// An Objective-C block that takes arguments of `A` when called and /// returns a value of `R`. @@ -16,48 +18,161 @@ use crate::BlockArguments; /// This is intented to be an `extern type`, and as such the memory layout of /// this type is _not_ guaranteed. That said, pointers to this type are always /// thin, and match that of Objective-C blocks. +/// An opaque type that holds an Objective-C block. +/// +/// This is the Objective-C equivalent of [`dyn Fn`][Fn], ... TODO +/// +/// Takes arguments `A` and returns `R`. +/// +/// Safety invariant: +/// +/// This invokes foreign code that the caller must verify doesn't violate +/// any of Rust's safety rules. +/// +/// For example, if this block is shared with multiple references, the +/// caller must ensure that calling it will not cause a data race. +// +// We don't technically need to restrict the type to `Closure: BlockFnOnce` +// here, it is restricted elsewhere, but it helps giving better error messages +// TODO: Verify and test this claim. #[repr(C)] -pub struct Block { +pub struct Block { _inner: [u8; 0], - // We store `BlockHeader` + the closure captures, but `Block` has to - // remain an empty type otherwise the compiler thinks we only have - // provenance over `BlockHeader`. + /// We store `BlockHeader` + the closure, but `Block` has to remain an + /// empty type because we don't know the size of the closure, and + /// otherwise the compiler would think we only have provenance over + /// `BlockHeader`. + /// + /// This is possible to improve once we have extern types. _header: PhantomData, - // To get correct variance on args and return types - _p: PhantomData R>, + _p: PhantomData, } // SAFETY: Pointers to `Block` is an Objective-C block. -unsafe impl RefEncode for Block { +unsafe impl RefEncode for Block { const ENCODING_REF: Encoding = Encoding::Block; } -impl Block { - /// Call self with the given arguments. - /// - /// # Safety - /// - /// This invokes foreign code that the caller must verify doesn't violate - /// any of Rust's safety rules. +impl Block { + fn header(&self) -> &BlockHeader { + let ptr: NonNull = NonNull::from(self); + let ptr: NonNull = ptr.cast(); + // SAFETY: `Block` is `BlockHeader` + closure + unsafe { ptr.as_ref() } + } + + /// Copy the block onto the heap as an `RcBlock`. /// - /// For example, if this block is shared with multiple references, the - /// caller must ensure that calling it will not cause a data race. - pub unsafe fn call(&self, args: A) -> R { + /// TODO: This bumps reference count, or creates new. + // + // TODO: Place this on `Block`, once that carries a lifetime. + #[inline] + pub fn copy(&self) -> RcBlock { let ptr: *const Self = self; - let header = unsafe { ptr.cast::().as_ref().unwrap_unchecked() }; + let ptr: *mut Block = ptr as *mut _; + // SAFETY: The block will be alive for the lifetime of the new + // `RcBlock`, and the lifetime is carried over. TODO + unsafe { RcBlock::copy(ptr) }.unwrap_or_else(|| block_copy_fail()) + } + + /// Call self with the given arguments. + pub fn call(&self, args: F::Args) -> F::Output + where + F: BlockFn, + { // TODO: Is `invoke` actually ever null? - let invoke = header.invoke.unwrap_or_else(|| unreachable!()); + let invoke = self.header().invoke.unwrap_or_else(|| unreachable!()); - unsafe { A::__call_block(invoke, ptr as *mut Self, args) } + let ptr: NonNull = NonNull::from(self); + let ptr: *mut Self = ptr.as_ptr(); + + // SAFETY: The closure is `Fn`, and as such is safe to call from an + // immutable reference. + unsafe { F::__call_block(invoke, ptr, args) } } } -impl fmt::Debug for Block { +impl fmt::Debug for Block { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let mut f = f.debug_struct("Block"); - let ptr: *const Self = self; - let header = unsafe { ptr.cast::().as_ref().unwrap() }; - debug_block_header(header, &mut f); + debug_block_header(self.header(), &mut f); f.finish_non_exhaustive() } } + +#[cfg(test)] +mod tests { + use core::cell::Cell; + use core::sync::atomic::{AtomicUsize, Ordering}; + + use crate::RcBlock; + + use super::*; + + /// + #[test] + fn test_rust_dyn_lifetime_semantics() { + fn takes_static(block: &Block) { + block.call(()); + } + + fn takes_elided(block: &Block) { + block.call(()); + } + + fn takes_unspecified(block: &Block) { + block.call(()); + } + + // Static lifetime + static MY_STATIC: AtomicUsize = AtomicUsize::new(0); + MY_STATIC.store(0, Ordering::Relaxed); + let static_lifetime: RcBlock = RcBlock::new(|| { + MY_STATIC.fetch_add(1, Ordering::Relaxed); + }); + takes_static(&static_lifetime); + takes_elided(&static_lifetime); + takes_unspecified(&static_lifetime); + assert_eq!(MY_STATIC.load(Ordering::Relaxed), 3); + + // Lifetime declared with `'_` + let captured = Cell::new(0); + let elided_lifetime: RcBlock = RcBlock::new(|| { + captured.set(captured.get() + 1); + }); + // takes_static(&elided_lifetime); // Compile error + takes_elided(&elided_lifetime); + // takes_unspecified(&elided_lifetime); // Compile error + assert_eq!(captured.get(), 1); + + // Lifetime kept unspecified + let captured = Cell::new(0); + let unspecified_lifetime: RcBlock = RcBlock::new(|| { + captured.set(captured.get() + 1); + }); + // takes_static(&unspecified_lifetime); // Compile error + takes_elided(&unspecified_lifetime); + // takes_unspecified(&unspecified_lifetime); // Compile error + assert_eq!(captured.get(), 1); + } + + #[allow(dead_code)] + fn unspecified_in_fn_is_static(block: &Block) -> &Block { + block + } + + #[allow(dead_code)] + fn lending_block<'b>(block: &Block &'b i32 + 'b>) { + let _ = *block.call(()); + } + + #[allow(dead_code)] + fn takes_lifetime(_: &Block &i32>) { + // Not actually callable yet + } + + #[allow(dead_code)] + fn covariant<'b, 'f>(b: &'b Block) -> &'b Block { + b + } +} diff --git a/crates/block2/src/global.rs b/crates/block2/src/global.rs index 74d0ef176..d92f58531 100644 --- a/crates/block2/src/global.rs +++ b/crates/block2/src/global.rs @@ -6,11 +6,9 @@ use core::ops::Deref; use core::ptr::{self, NonNull}; use std::os::raw::c_ulong; -use objc2::encode::EncodeReturn; - use crate::abi::{BlockDescriptor, BlockDescriptorPtr, BlockFlags, BlockHeader}; use crate::debug::debug_block_header; -use crate::{Block, BlockArguments}; +use crate::{Block, BlockFn}; // TODO: Should this be a static to help the compiler deduplicating them? const GLOBAL_DESCRIPTOR: BlockDescriptor = BlockDescriptor { @@ -29,33 +27,22 @@ const GLOBAL_DESCRIPTOR: BlockDescriptor = BlockDescriptor { /// [`RcBlock`]: crate::RcBlock /// [`global_block!`]: crate::global_block #[repr(C)] -pub struct GlobalBlock { - // TODO: Make this properly private once we have a higher MSRV - #[doc(hidden)] - pub header: BlockHeader, - #[doc(hidden)] - pub p: PhantomData R>, +pub struct GlobalBlock { + header: BlockHeader, + f: PhantomData, } -unsafe impl Sync for GlobalBlock -where - A: BlockArguments, - R: EncodeReturn, -{ -} -unsafe impl Send for GlobalBlock -where - A: BlockArguments, - R: EncodeReturn, -{ -} +// TODO: Add `Send + Sync` bounds once the block itself supports that. +unsafe impl Sync for GlobalBlock {} +unsafe impl Send for GlobalBlock {} -// Note: We can't put correct bounds on A and R because we have a const fn! +// Note: We can't put correct bounds on A and R because we have a const fn, +// and that's not allowed yet in our MSRV. // // Fortunately, we don't need them, since they're present on `Sync`, so // constructing the static in `global_block!` with an invalid `GlobalBlock` // triggers an error. -impl GlobalBlock { +impl GlobalBlock { // TODO: Use new ABI with BLOCK_HAS_SIGNATURE const FLAGS: BlockFlags = BlockFlags(BlockFlags::BLOCK_IS_GLOBAL.0 | BlockFlags::BLOCK_USE_STRET.0); @@ -72,25 +59,38 @@ impl GlobalBlock { basic: &GLOBAL_DESCRIPTOR, }, }; + + /// Use the [`global_block`] macro instead. + #[doc(hidden)] + #[inline] + pub const unsafe fn from_header(header: BlockHeader) -> Self { + Self { + header, + f: PhantomData, + } + } + + // TODO: Add some constructor for when `F: Copy`. } -impl Deref for GlobalBlock -where - A: BlockArguments, - R: EncodeReturn, -{ - type Target = Block; +impl Deref for GlobalBlock { + type Target = Block; #[inline] fn deref(&self) -> &Self::Target { let ptr: NonNull = NonNull::from(self); - let ptr: NonNull> = ptr.cast(); - // TODO: SAFETY + let ptr: NonNull> = ptr.cast(); + // SAFETY: This has the same layout as `Block` + // + // A global block does not hold any data, so it is safe to call + // immutably. + // + // TODO unsafe { ptr.as_ref() } } } -impl fmt::Debug for GlobalBlock { +impl fmt::Debug for GlobalBlock { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let mut f = f.debug_struct("GlobalBlock"); debug_block_header(&self.header, &mut f); @@ -104,6 +104,8 @@ impl fmt::Debug for GlobalBlock { /// be specified). Note that the block cannot capture its environment, and /// its argument types and return type must be [`EncodeReturn`]. /// +/// [`EncodeReturn`]: objc2::encode::EncodeReturn +/// /// # Examples /// /// ``` @@ -113,7 +115,7 @@ impl fmt::Debug for GlobalBlock { /// 42 /// }; /// } -/// assert_eq!(unsafe { MY_BLOCK.call(()) }, 42); +/// assert_eq!(MY_BLOCK.call(()), 42); /// ``` /// /// ``` @@ -123,7 +125,7 @@ impl fmt::Debug for GlobalBlock { /// x + y /// }; /// } -/// assert_eq!(unsafe { ADDER_BLOCK.call((5, 7)) }, 12); +/// assert_eq!(ADDER_BLOCK.call((5, 7)), 12); /// ``` /// /// The following does not compile because [`Box`] is not [`EncodeReturn`]: @@ -135,8 +137,8 @@ impl fmt::Debug for GlobalBlock { /// } /// ``` /// -/// This also doesn't work (yet), as global blocks are overly restrictive -/// about the lifetimes involved. +/// This also doesn't work (yet), as blocks are overly restrictive about the +/// lifetimes involved. /// /// ```compile_fail /// use block2::global_block; @@ -146,7 +148,7 @@ impl fmt::Debug for GlobalBlock { /// }; /// } /// let x = 5; -/// let res = unsafe { BLOCK_WITH_LIFETIME.call((&x,)) }; +/// let res = BLOCK_WITH_LIFETIME.call((&x,)); /// assert_eq!(res, 47); /// ``` /// @@ -180,24 +182,21 @@ macro_rules! global_block { ) => { $(#[$m])* #[allow(unused_unsafe)] - $vis static $name: $crate::GlobalBlock<($($t,)*) $(, $r)?> = unsafe { - let mut header = $crate::GlobalBlock::<($($t,)*) $(, $r)?>::__DEFAULT_HEADER; + $vis static $name: $crate::GlobalBlock $r)?> = unsafe { + let mut header = $crate::GlobalBlock:: $r)?>::__DEFAULT_HEADER; header.isa = ::core::ptr::addr_of!($crate::ffi::_NSConcreteGlobalBlock); header.invoke = ::core::option::Option::Some({ - unsafe extern "C" fn inner(_: *mut $crate::GlobalBlock<($($t,)*) $(, $r)?>, $($a: $t),*) $(-> $r)? { + unsafe extern "C" fn inner(_: *mut $crate::GlobalBlock $r)?>, $($a: $t),*) $(-> $r)? { $body } // TODO: SAFETY ::core::mem::transmute::< - unsafe extern "C" fn(*mut $crate::GlobalBlock<($($t,)*) $(, $r)?>, $($a: $t),*) $(-> $r)?, + unsafe extern "C" fn(*mut $crate::GlobalBlock $r)?>, $($a: $t),*) $(-> $r)?, unsafe extern "C" fn(), >(inner) }); - $crate::GlobalBlock { - header, - p: ::core::marker::PhantomData - } + $crate::GlobalBlock::from_header(header) }; }; } @@ -222,7 +221,7 @@ mod tests { #[test] fn test_noop_block() { - unsafe { NOOP_BLOCK.call(()) }; + NOOP_BLOCK.call(()); } #[test] @@ -230,7 +229,7 @@ mod tests { global_block!(static MY_BLOCK = || -> i32 { 42 }); - assert_eq!(unsafe { MY_BLOCK.call(()) }, 42); + assert_eq!(MY_BLOCK.call(()), 42); } #[cfg(feature = "apple")] @@ -287,4 +286,9 @@ mod tests { ); assert_eq!(format!("{NOOP_BLOCK:#?}"), expected); } + + #[allow(dead_code)] + fn covariant<'b, 'f>(b: GlobalBlock) -> GlobalBlock { + b + } } diff --git a/crates/block2/src/lib.rs b/crates/block2/src/lib.rs index 0f9724d7b..0e0a93306 100644 --- a/crates/block2/src/lib.rs +++ b/crates/block2/src/lib.rs @@ -16,52 +16,44 @@ //! [ABI]: http://clang.llvm.org/docs/Block-ABI-Apple.html //! //! -//! ## Invoking blocks +//! ## External functions using blocks +//! +//! To declare external functions or methods that takes blocks, use +//! `&Block R>` or `Option<&Block R>>`, where +//! `Args` is the argument types, and `R` is the return type. +//! +//! In the next few examples, we're going to work with a function +//! `check_addition`, that takes as parameter a block that adds two integers, +//! and checks that the addition is correct. //! -//! The [`Block`] struct is used for invoking blocks from Objective-C. For -//! example, consider this Objective-C function that takes a block as a -//! parameter, executes the block with some arguments, and returns the result: +//! Such a function could be written in C like in the following. //! //! ```objc +//! #include //! #include //! #include -//! int32_t run_block(int32_t (^block)(int32_t, int32_t)) { -//! return block(5, 8); +//! +//! void check_addition(int32_t (^block)(int32_t, int32_t)) { +//! assert(block(5, 8) == 13); //! } //! ``` //! -//! We could write the equivalent function in Rust like this: +//! An `extern "C" { ... }` declaration for that function would then be: //! //! ``` //! use block2::Block; -//! unsafe fn run_block(block: &Block<(i32, i32), i32>) -> i32 { -//! block.call((5, 8)) +//! +//! extern "C" { +//! fn check_addition(block: &Block i32>); //! } //! ``` //! -//! Note the extra parentheses in the `call` method, since the arguments must -//! be passed as a tuple. -//! -//! -//! ## Creating blocks -//! -//! Creating a block to pass to Objective-C can be done with [`RcBlock`] or -//! [`StackBlock`], depending on if you want to move the block to the heap, -//! or let the callee decide if it needs to do that. -//! -//! To declare external functions or methods that takes blocks, use -//! `&Block` or `Option<&Block>`, where `A` is a tuple with the -//! argument types, and `R` is the return type. -//! -//! As an example, we're going to work with a block that adds two integers. +//! This can similarly be done inside external methods declared with +//! [`objc2::extern_methods!`]. //! //! ``` //! use block2::Block; -//! -//! // External function that takes a block -//! extern "C" { -//! fn add_numbers_using_block(block: &Block<(i32, i32), i32>); -//! } +//! use objc2::extern_methods; //! # //! # use objc2::ClassType; //! # objc2::extern_class!( @@ -74,33 +66,52 @@ //! # } //! # ); //! -//! // External method that takes a block -//! objc2::extern_methods!( +//! extern_methods!( //! unsafe impl MyClass { -//! #[method(addNumbersUsingBlock:)] -//! pub fn addNumbersUsingBlock(&self, block: &Block<(i32, i32), i32>); +//! #[method(checkAddition:)] +//! pub fn checkAddition(&self, block: &Block i32>); //! } //! ); //! ``` //! +//! +//! ## Invoking blocks +//! +//! We can also define the external function in Rust, and expose it to +//! Objective-C. To do this, we can use [`Block::call`] to invoke the block +//! inside the function. +//! +//! ``` +//! use block2::Block; +//! +//! #[no_mangle] +//! extern "C" fn check_addition(block: &Block i32>) { +//! assert_eq!(block.call((5, 8)), 13); +//! } +//! ``` +//! +//! Note the extra parentheses in the `call` method, since the arguments must +//! be passed as a tuple. +//! +//! +//! ## Creating blocks +//! +//! Creating a block to pass to Objective-C can be done with [`RcBlock`] or +//! [`StackBlock`], depending on if you want to move the block to the heap, +//! or let the callee decide if it needs to do that. +//! //! To call such a function / method, we could create a new block from a //! closure using [`RcBlock::new`]. //! //! ``` //! use block2::RcBlock; //! # -//! # extern "C" { -//! # fn add_numbers_using_block(block: &block2::Block<(i32, i32), i32>); -//! # } -//! # mod imp { -//! # #[no_mangle] -//! # extern "C" fn add_numbers_using_block(block: &block2::Block<(i32, i32), i32>) { -//! # assert_eq!(unsafe { block.call((5, 8)) }, 13); -//! # } +//! # extern "C" fn check_addition(block: &block2::Block i32>) { +//! # assert_eq!(block.call((5, 8)), 13); //! # } //! -//! let block = RcBlock::new(|a: i32, b: i32| a + b); -//! unsafe { add_numbers_using_block(&block) }; +//! let block = RcBlock::new(|a, b| a + b); +//! check_addition(&block); //! ``` //! //! This creates the block on the heap. If the external function you're @@ -108,52 +119,47 @@ //! construct a [`StackBlock`] directly, using [`StackBlock::new`]. //! //! Note though that this requires that the closure is [`Clone`], as the -//! external code may want to copy the block to the heap in the future. +//! external code is allowed to copy the block to the heap in the future. //! //! ``` //! use block2::StackBlock; //! # -//! # extern "C" { -//! # fn add_numbers_using_block(block: &block2::Block<(i32, i32), i32>); -//! # } -//! # mod imp { -//! # #[no_mangle] -//! # extern "C" fn add_numbers_using_block(block: &block2::Block<(i32, i32), i32>) { -//! # assert_eq!(unsafe { block.call((5, 8)) }, 13); -//! # } +//! # extern "C" fn check_addition(block: &block2::Block i32>) { +//! # assert_eq!(block.call((5, 8)), 13); //! # } //! -//! let block = StackBlock::new(|a: i32, b: i32| a + b); -//! unsafe { add_numbers_using_block(&block) }; +//! let block = StackBlock::new(|a, b| a + b); +//! check_addition(&block); //! ``` //! -//! As an optimization if your block doesn't capture any variables (as in the -//! above examples), you can use the [`global_block!`] macro to create a +//! As an optimization, if your closure doesn't capture any variables (as in +//! the above examples), you can use the [`global_block!`] macro to create a //! static block. //! //! ``` //! use block2::global_block; //! # -//! # extern "C" { -//! # fn add_numbers_using_block(block: &block2::Block<(i32, i32), i32>); -//! # } -//! # mod imp { -//! # #[no_mangle] -//! # extern "C" fn add_numbers_using_block(block: &block2::Block<(i32, i32), i32>) { -//! # assert_eq!(unsafe { block.call((5, 8)) }, 13); -//! # } +//! # extern "C" fn check_addition(block: &block2::Block i32>) { +//! # assert_eq!(block.call((5, 8)), 13); //! # } //! //! global_block! { -//! static MY_BLOCK = |a: i32, b: i32| -> i32 { +//! static BLOCK = |a: i32, b: i32| -> i32 { //! a + b //! }; //! } //! -//! unsafe { add_numbers_using_block(&MY_BLOCK) }; +//! check_addition(&BLOCK); //! ``` //! //! +//! ## Lifetimes +//! +//! Specifying the lifetimes involved in blocks can be quite difficult. +//! +//! TODO. +//! +//! //! ## Specifying a runtime //! //! Different runtime implementations exist and act in slightly different ways @@ -170,7 +176,7 @@ //! //! - Feature flag: `apple`. //! -//! This is the most common an most sophisticated runtime, and it has quite a +//! This is the most common and most sophisticated runtime, and it has quite a //! lot more features than the specification mandates. //! //! The minimum required operating system versions are as follows (though in @@ -324,11 +330,11 @@ 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::{BlockArguments, IntoBlock}; +pub use self::traits::{BlockFn, IntoBlock}; /// Deprecated alias for `StackBlock`. #[deprecated = "renamed to `StackBlock`"] -pub type ConcreteBlock = self::stack::StackBlock; +pub type ConcreteBlock = StackBlock<'static, A, R, Closure>; // Note: We could use `_Block_object_assign` and `_Block_object_dispose` to // implement a `ByRef` wrapper, which would behave like `__block` marked diff --git a/crates/block2/src/rc_block.rs b/crates/block2/src/rc_block.rs index c8f8724d5..20086b9b5 100644 --- a/crates/block2/src/rc_block.rs +++ b/crates/block2/src/rc_block.rs @@ -3,11 +3,11 @@ use core::mem::ManuallyDrop; use core::ops::Deref; use core::ptr::NonNull; -use objc2::encode::EncodeReturn; +use objc2::encode::{EncodeArguments, EncodeReturn}; use crate::abi::BlockHeader; use crate::debug::debug_block_header; -use crate::{ffi, Block, BlockArguments, IntoBlock, StackBlock}; +use crate::{ffi, Block, IntoBlock, StackBlock}; /// A reference-counted Objective-C block that is stored on the heap. /// @@ -23,11 +23,12 @@ use crate::{ffi, Block, BlockArguments, IntoBlock, StackBlock}; /// `Option>` is guaranteed to have the same size as /// `RcBlock`. #[doc(alias = "MallocBlock")] -pub struct RcBlock { - ptr: NonNull>, +pub struct RcBlock { + // Covariant + ptr: NonNull>, } -impl RcBlock { +impl RcBlock { /// Construct an `RcBlock` from the given block pointer by taking /// ownership. /// @@ -39,7 +40,7 @@ impl RcBlock { /// return type must be correct, and the block must have a +1 reference / /// retain count from somewhere else. #[inline] - pub unsafe fn from_raw(ptr: *mut Block) -> Option { + pub unsafe fn from_raw(ptr: *mut Block) -> Option { NonNull::new(ptr).map(|ptr| Self { ptr }) } @@ -58,14 +59,15 @@ impl RcBlock { #[doc(alias = "Block_copy")] #[doc(alias = "_Block_copy")] #[inline] - pub unsafe fn copy(ptr: *mut Block) -> Option { - let ptr: *mut Block = unsafe { ffi::_Block_copy(ptr.cast()) }.cast(); + pub unsafe fn copy(ptr: *mut Block) -> Option { + let ptr: *mut Block = unsafe { ffi::_Block_copy(ptr.cast()) }.cast(); // SAFETY: We just copied the block, so the reference count is +1 unsafe { Self::from_raw(ptr) } } } -impl RcBlock { +// TODO: Move so this appears first in the docs. +impl RcBlock { /// Construct a `RcBlock` with the given closure. /// /// The closure will be coped to the heap on construction. @@ -75,11 +77,11 @@ impl RcBlock { // // Note: Unsure if this should be #[inline], but I think it may be able to // benefit from not being so. - pub fn new(closure: F) -> Self + pub fn new<'f, A, R, Closure>(closure: Closure) -> Self where - // The `F: 'static` bound is required because the `RcBlock` has no way - // of tracking a lifetime. - F: IntoBlock + 'static, + A: EncodeArguments, + R: EncodeReturn, + Closure: IntoBlock<'f, A, R, Dyn = F>, { // SAFETY: The stack block is copied once below. // @@ -93,8 +95,8 @@ impl RcBlock { // Transfer ownership from the stack to the heap. let mut block = ManuallyDrop::new(block); - let ptr: *mut StackBlock = &mut *block; - let ptr: *mut Block = ptr.cast(); + let ptr: *mut StackBlock<'f, A, R, Closure> = &mut *block; + let ptr: *mut Block = ptr.cast(); // SAFETY: The block will be moved to the heap, and we forget the // original block because the heap block will drop in our dispose // helper. @@ -102,7 +104,7 @@ impl RcBlock { } } -impl Clone for RcBlock { +impl Clone for RcBlock { #[inline] fn clone(&self) -> Self { // SAFETY: The pointer is valid, since the only way to get an RcBlock @@ -113,21 +115,27 @@ impl Clone for RcBlock { // Intentionally not `#[track_caller]`, to keep the code-size smaller (as this // error is very unlikely). -pub(crate) fn rc_new_fail() -> ! { +fn rc_new_fail() -> ! { // This likely means the system is out of memory. panic!("failed creating RcBlock") } +// Intentionally not `#[track_caller]`, see above. +pub(crate) fn block_copy_fail() -> ! { + // This likely means the system is out of memory. + panic!("failed copying Block") +} + // Intentionally not `#[track_caller]`, see above. fn rc_clone_fail() -> ! { unreachable!("cloning a RcBlock bumps the reference count, which should be infallible") } -impl Deref for RcBlock { - type Target = Block; +impl Deref for RcBlock { + type Target = Block; #[inline] - fn deref(&self) -> &Block { + fn deref(&self) -> &Block { // SAFETY: The pointer is valid, as ensured by creation methods, and // will be so for as long as the `RcBlock` is, since that holds +1 // reference count. @@ -135,7 +143,7 @@ impl Deref for RcBlock { } } -impl Drop for RcBlock { +impl Drop for RcBlock { /// Release the block, decreasing the reference-count by 1. /// /// The `Drop` method of the underlying closure will be called once the @@ -150,7 +158,7 @@ impl Drop for RcBlock { } } -impl fmt::Debug for RcBlock { +impl fmt::Debug for RcBlock { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let mut f = f.debug_struct("RcBlock"); let header = unsafe { self.ptr.cast::().as_ref() }; @@ -158,3 +166,54 @@ impl fmt::Debug for RcBlock { f.finish_non_exhaustive() } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn return_rc_block() { + fn get_adder(x: i32) -> RcBlock i32> { + RcBlock::new(move |y| y + x) + } + + let add2 = get_adder(2); + assert_eq!(add2.call((5,)), 7); + assert_eq!(add2.call((-1,)), 1); + } + + #[test] + fn rc_block_with_precisely_described_lifetimes() { + fn args<'a, 'b>( + f: impl Fn(&'a i32, &'b i32) + 'static, + ) -> RcBlock { + RcBlock::new(f) + } + + fn args_return<'a, 'b>( + f: impl Fn(&'a i32) -> &'b i32 + 'static, + ) -> RcBlock &'b i32 + 'static> { + RcBlock::new(f) + } + + fn args_entire<'a, 'b>(f: impl Fn(&'a i32) + 'b) -> RcBlock { + RcBlock::new(f) + } + + fn return_entire<'a, 'b>( + f: impl Fn() -> &'a i32 + 'b, + ) -> RcBlock &'a i32 + 'b> { + RcBlock::new(f) + } + + let _ = args(|_, _| {}); + let _ = args_return(|x| x); + let _ = args_entire(|_| {}); + let _ = return_entire(|| &5); + } + + #[allow(dead_code)] + fn covariant<'f>(b: RcBlock) -> RcBlock { + b + } +} diff --git a/crates/block2/src/stack.rs b/crates/block2/src/stack.rs index 4c138f6ee..f21ae12d6 100644 --- a/crates/block2/src/stack.rs +++ b/crates/block2/src/stack.rs @@ -3,23 +3,38 @@ use core::fmt; use core::marker::PhantomData; use core::mem::{self, MaybeUninit}; use core::ops::Deref; +use core::panic::{RefUnwindSafe, UnwindSafe}; use core::ptr::{self, NonNull}; use std::os::raw::c_ulong; -use objc2::encode::{EncodeReturn, Encoding, RefEncode}; +use objc2::encode::{EncodeArguments, EncodeReturn, Encoding, RefEncode}; use crate::abi::{ BlockDescriptor, BlockDescriptorCopyDispose, BlockDescriptorPtr, BlockFlags, BlockHeader, }; use crate::debug::debug_block_header; -use crate::rc_block::rc_new_fail; -use crate::{ffi, Block, BlockArguments, IntoBlock, RcBlock}; +use crate::{ffi, Block, IntoBlock}; /// An Objective-C block constructed on the stack. /// /// This is a smart pointer that [`Deref`]s to [`Block`]. /// /// +/// # Type parameters +/// +/// The type parameters for this is a bit complex due to trait system +/// limitations. Usually, you will not need to specify them, as the +/// compiler will be able to infer them. +/// +/// - The lifetime `'f`, which is the lifetime of the closure. +/// - The parameter `A`, which is a tuple representing the arguments to the +/// closure. +/// - The parameter `R`, which is the return type of the closure. +/// - The parameter `Closure`, which is the actual closure type. This is +/// usually unnameable, and you will have to use `_`, `impl Fn()` or +/// similar. +/// +/// /// # Memory layout /// /// The memory layout of this type is _not_ guaranteed. @@ -27,8 +42,7 @@ use crate::{ffi, Block, BlockArguments, IntoBlock, RcBlock}; /// That said, it will always be safe to reintepret pointers to this as a /// pointer to a `Block`. #[repr(C)] -pub struct StackBlock { - p: PhantomData>, +pub struct StackBlock<'f, A, R, Closure> { header: BlockHeader, /// The block's closure. /// @@ -36,17 +50,26 @@ pub struct StackBlock { /// /// Note that this is not wrapped in a `ManuallyDrop`; once the /// `StackBlock` is dropped, the closure will also be dropped. - pub(crate) closure: F, + pub(crate) closure: Closure, + /// For correct variance of the other type parameters. + /// + /// We add auto traits such that they depend on the closure instead. + p: PhantomData R + Send + Sync + RefUnwindSafe + UnwindSafe + Unpin + 'f>, } // SAFETY: Pointers to the stack block is always safe to reintepret as an // ordinary block pointer. -unsafe impl RefEncode for StackBlock { +unsafe impl<'f, A, R, Closure> RefEncode for StackBlock<'f, A, R, Closure> +where + A: EncodeArguments, + R: EncodeReturn, + Closure: 'f + IntoBlock<'f, A, R>, +{ const ENCODING_REF: Encoding = Encoding::Block; } // Basic constants and helpers. -impl StackBlock { +impl<'f, A, R, Closure> StackBlock<'f, A, R, Closure> { /// The size of the block header and the trailing closure. /// /// This ensures that the closure that the block contains is also moved to @@ -83,7 +106,7 @@ impl StackBlock { } // `StackBlock::new` -impl StackBlock { +impl<'f, A, R, Closure: Clone> StackBlock<'f, A, R, Closure> { /// Clone the closure from one block to another. unsafe extern "C" fn clone_closure(dst: *mut c_void, src: *const c_void) { let dst: *mut Self = dst.cast(); @@ -117,7 +140,9 @@ impl StackBlock { copy: Some(Self::clone_closure), dispose: Some(Self::drop_closure), }; +} +impl<'f, A, R, Closure> StackBlock<'f, A, R, Closure> { /// Construct a `StackBlock` with the given closure. /// /// Note that this requires [`Clone`], as a C block is generally assumed @@ -126,17 +151,21 @@ impl StackBlock { /// /// When the block is called, it will return the value that results from /// calling the closure. + /// + /// [`RcBlock::new`]: crate::RcBlock::new #[inline] - pub fn new(closure: F) -> Self + pub fn new(closure: Closure) -> Self where - F: IntoBlock, + A: EncodeArguments, + R: EncodeReturn, + Closure: IntoBlock<'f, A, R> + Clone, { let header = BlockHeader { isa: unsafe { ptr::addr_of!(ffi::_NSConcreteStackBlock) }, // TODO: Add signature. flags: BlockFlags::BLOCK_HAS_COPY_DISPOSE, reserved: MaybeUninit::new(0), - invoke: Some(F::__get_invoke_stack_block()), + invoke: Some(Closure::__get_invoke_stack_block()), // TODO: Use `Self::DESCRIPTOR_BASIC` when `F: Copy` // (probably only possible with specialization). descriptor: BlockDescriptorPtr { @@ -144,46 +173,15 @@ impl StackBlock { }, }; Self { - p: PhantomData, header, closure, + p: PhantomData, } } - - /// Copy the stack block onto the heap as an `RcBlock`. - /// - /// Most of the time you'll want to use [`RcBlock::new`] directly instead. - // - // TODO: Place this on `Block`, once that carries a lifetime. - #[inline] - pub fn to_rc(&self) -> RcBlock - where - // This bound is required because the `RcBlock` has no way of tracking - // a lifetime. - F: 'static, - { - let ptr: *const Self = self; - let ptr: *const Block = ptr.cast(); - let ptr: *mut Block = ptr as *mut _; - // SAFETY: A pointer to `StackBlock` is safe to convert to a pointer - // to `Block`, and because of the `F: 'static` lifetime bound, the - // block will be alive for the lifetime of the new `RcBlock`. - unsafe { RcBlock::copy(ptr) }.unwrap_or_else(|| rc_new_fail()) - } - - /// No longer necessary, the implementation does this for you when needed. - #[inline] - #[deprecated = "no longer necessary"] - pub fn copy(self) -> RcBlock - where - F: 'static, - { - self.to_rc() - } } // `RcBlock::new` -impl StackBlock { +impl<'f, A, R, Closure> StackBlock<'f, A, R, Closure> { unsafe extern "C" 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`. @@ -200,9 +198,11 @@ impl StackBlock { /// /// `_Block_copy` must be called on the resulting stack block only once. #[inline] - pub(crate) unsafe fn new_no_clone(closure: F) -> Self + pub(crate) unsafe fn new_no_clone(closure: Closure) -> Self where - F: IntoBlock, + A: EncodeArguments, + R: EncodeReturn, + Closure: IntoBlock<'f, A, R>, { // Don't need to emit copy and dispose helpers if the closure // doesn't need it. @@ -227,37 +227,42 @@ impl StackBlock { isa: unsafe { ptr::addr_of!(ffi::_NSConcreteStackBlock) }, flags, reserved: MaybeUninit::new(0), - invoke: Some(F::__get_invoke_stack_block()), + invoke: Some(Closure::__get_invoke_stack_block()), descriptor, }; Self { - p: PhantomData, header, closure, + p: PhantomData, } } } -impl Clone for StackBlock { +impl<'f, A, R, Closure: Clone> Clone for StackBlock<'f, A, R, Closure> { #[inline] fn clone(&self) -> Self { Self { - p: PhantomData, header: self.header, closure: self.closure.clone(), + p: PhantomData, } } } -impl Copy for StackBlock {} +impl<'f, A, R, Closure: Copy> Copy for StackBlock<'f, A, R, Closure> {} -impl Deref for StackBlock { - type Target = Block; +impl<'f, A, R, Closure> Deref for StackBlock<'f, A, R, Closure> +where + A: EncodeArguments, + R: EncodeReturn, + Closure: IntoBlock<'f, A, R>, +{ + type Target = Block; #[inline] fn deref(&self) -> &Self::Target { let ptr: NonNull = NonNull::from(self); - let ptr: NonNull> = ptr.cast(); + let ptr: NonNull> = ptr.cast(); // SAFETY: A pointer to `StackBlock` is always safe to convert to a // pointer to `Block`, and the reference will be valid as long the // stack block exists. @@ -268,7 +273,7 @@ impl Deref for StackBlock { } } -impl fmt::Debug for StackBlock { +impl<'f, A, R, Closure> fmt::Debug for StackBlock<'f, A, R, Closure> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let mut f = f.debug_struct("StackBlock"); debug_block_header(&self.header, &mut f); @@ -284,11 +289,18 @@ mod tests { fn test_size() { assert_eq!( mem::size_of::(), - >::SIZE as _, + >::SIZE as _, ); assert_eq!( mem::size_of::() + mem::size_of::(), - >::SIZE as _, + >::SIZE as _, ); } + + #[allow(dead_code)] + fn covariant<'b, 'f>( + b: StackBlock<'static, (), (), impl Fn() + 'static>, + ) -> StackBlock<'b, (), (), impl Fn() + 'f> { + b + } } diff --git a/crates/block2/src/traits.rs b/crates/block2/src/traits.rs index 519a18ce5..502c930ea 100644 --- a/crates/block2/src/traits.rs +++ b/crates/block2/src/traits.rs @@ -1,32 +1,40 @@ use core::mem; use core::ptr; +use objc2::encode::EncodeArguments; use objc2::encode::{EncodeArgument, EncodeReturn}; use crate::{Block, StackBlock}; mod private { - pub trait Sealed {} + pub trait Sealed {} } -/// Types that may be used as the arguments of an Objective-C block. +/// TODO. /// -/// This is implemented for tuples of up to 12 arguments, where each argument -/// implements [`EncodeArgument`]. +/// This is implemented for [`dyn Fn`][Fn] closures of up to 12 arguments, +/// where each argument implements [`EncodeArgument`] and the return type +/// implements [`EncodeReturn`]. /// /// /// # Safety /// /// This is a sealed trait, and should not need to be implemented. Open an /// issue if you know a use-case where this restrition should be lifted! -pub unsafe trait BlockArguments: Sized { - /// Calls the given method the block and arguments. +pub unsafe trait BlockFn: private::Sealed { + /// The arguments to the block. + type Args: EncodeArguments; + + /// The return type of the block. + type Output: EncodeReturn; + + /// Calls the given invoke function with the block and arguments. #[doc(hidden)] - unsafe fn __call_block( + unsafe fn __call_block( invoke: unsafe extern "C" fn(), - block: *mut Block, - args: Self, - ) -> R; + block: *mut Block, + args: Self::Args, + ) -> Self::Output; } /// Types that may be converted into a block. @@ -40,50 +48,61 @@ pub unsafe trait BlockArguments: Sized { /// /// This is a sealed trait, and should not need to be implemented. Open an /// issue if you know a use-case where this restrition should be lifted! -pub unsafe trait IntoBlock: private::Sealed + Sized +pub unsafe trait IntoBlock<'f, A, R>: private::Sealed where - A: BlockArguments, + A: EncodeArguments, R: EncodeReturn, { + /// The type-erased `dyn Fn(...Args) -> R + 'f`. + type Dyn: ?Sized + BlockFn; + #[doc(hidden)] fn __get_invoke_stack_block() -> unsafe extern "C" fn(); } macro_rules! impl_traits { - ($($a:ident : $t:ident),*) => ( - impl<$($t: EncodeArgument,)* R: EncodeReturn, Closure> private::Sealed<($($t,)*)> for Closure + ($($a:ident: $t:ident),*) => ( + impl<$($t: EncodeArgument,)* R: EncodeReturn, Closure> private::Sealed<($($t,)*), R> for Closure where - Closure: Fn($($t,)*) -> R, + Closure: ?Sized + Fn($($t,)*) -> R, {} - unsafe impl<$($t: EncodeArgument),*> BlockArguments for ($($t,)*) { + unsafe impl<$($t: EncodeArgument,)* R: EncodeReturn> BlockFn for dyn Fn($($t),*) -> R + '_ { + type Args = ($($t,)*); + type Output = R; + #[inline] - unsafe fn __call_block( + unsafe fn __call_block( invoke: unsafe extern "C" fn(), - block: *mut Block, - ($($a,)*): Self, - ) -> R { + block: *mut Block, + ($($a,)*): Self::Args, + ) -> Self::Output { // Very similar to `MessageArguments::__invoke` - let invoke: unsafe extern "C" fn(*mut Block $(, $t)*) -> R = unsafe { + let invoke: unsafe extern "C" fn(*mut Block $(, $t)*) -> R = unsafe { mem::transmute(invoke) }; unsafe { invoke(block $(, $a)*) } } } + // TODO: Add `+ Send`, `+ Sync` and `+ Send + Sync`. - unsafe impl<$($t: EncodeArgument,)* R: EncodeReturn, Closure> IntoBlock<($($t,)*), R> for Closure + unsafe impl<'f, $($t,)* R, Closure> IntoBlock<'f, ($($t,)*), R> for Closure where - Closure: Fn($($t,)*) -> R, + $($t: EncodeArgument,)* + R: EncodeReturn, + Closure: Fn($($t,)*) -> R + 'f, { + type Dyn = dyn Fn($($t),*) -> R + 'f; + #[inline] fn __get_invoke_stack_block() -> unsafe extern "C" fn() { - unsafe extern "C" fn invoke<$($t,)* R, Closure>( - block: *mut StackBlock<($($t,)*), R, Closure>, + unsafe extern "C" fn invoke<'f, $($t,)* R, Closure>( + block: *mut StackBlock<'f, ($($t,)*), R, Closure>, $($a: $t,)* ) -> R where - Closure: Fn($($t,)*) -> R, + Closure: Fn($($t),*) -> R + 'f { let closure = unsafe { &*ptr::addr_of!((*block).closure) }; (closure)($($a),*) @@ -91,7 +110,7 @@ macro_rules! impl_traits { unsafe { mem::transmute::< - unsafe extern "C" fn(*mut StackBlock<($($t,)*), R, Closure>, $($a: $t,)*) -> R, + unsafe extern "C" fn(*mut StackBlock<'f, ($($t,)*), R, Closure>, $($t,)*) -> R, unsafe extern "C" fn(), >(invoke) } diff --git a/crates/header-translator/src/rust_type.rs b/crates/header-translator/src/rust_type.rs index 28f61a93f..a5753c859 100644 --- a/crates/header-translator/src/rust_type.rs +++ b/crates/header-translator/src/rust_type.rs @@ -1232,18 +1232,26 @@ impl fmt::Display for Inner { Self::Fn { .. } => write!(f, "TodoFunction"), Self::Block { sendable: _, - no_escape: _, + no_escape, arguments, result_type, } => { - write!(f, "Block<(")?; + write!(f, "Block write!(f, "()")?, - ty => write!(f, "{ty}")?, + Self::Void => {} + ty => write!(f, " -> {ty}")?, + } + if *no_escape { + write!(f, " + '_")?; + } else { + // `dyn Fn()` in function parameters implies `+ 'static`, + // so no need to specify that. + // + // write!(f, " + 'static")?; } write!(f, ">") } diff --git a/crates/icrate/src/additions/Foundation/data.rs b/crates/icrate/src/additions/Foundation/data.rs index 5c2f6d51e..e4ac0858f 100644 --- a/crates/icrate/src/additions/Foundation/data.rs +++ b/crates/icrate/src/additions/Foundation/data.rs @@ -278,7 +278,7 @@ unsafe fn with_vec(obj: Allocated, bytes: Vec) -> Id { // Recreate the Vec and let it drop let _ = unsafe { >::from_raw_parts(bytes.cast(), len, capacity) }; }); - let dealloc: &Block<(*mut c_void, usize), ()> = &dealloc; + let dealloc: &Block = &dealloc; let mut bytes = ManuallyDrop::new(bytes); let bytes_ptr: *mut c_void = bytes.as_mut_ptr().cast(); diff --git a/crates/icrate/src/generated b/crates/icrate/src/generated index 3484eafc6..d23149c01 160000 --- a/crates/icrate/src/generated +++ b/crates/icrate/src/generated @@ -1 +1 @@ -Subproject commit 3484eafc6bdb0fbd32939e6243afec9af947e5ab +Subproject commit d23149c01228ba41a9864673299065a11dbda9c3 diff --git a/crates/objc2/src/rc/test_object.rs b/crates/objc2/src/rc/test_object.rs index e9c54da71..65e5fd886 100644 --- a/crates/objc2/src/rc/test_object.rs +++ b/crates/objc2/src/rc/test_object.rs @@ -21,6 +21,8 @@ pub struct __ThreadTestData { pub autorelease: usize, pub try_retain: usize, pub try_retain_fail: usize, + // TODO: Is there some way we can test weak pointers? Or is that implemented entirely in Foundation? + // Maybe `_setWeaklyReferenced` can be useful? } impl __ThreadTestData { diff --git a/crates/test-assembly/crates/test_block/expected/apple-aarch64.s b/crates/test-assembly/crates/test_block/expected/apple-aarch64.s index 3fcb5695f..973eeba5f 100644 --- a/crates/test-assembly/crates/test_block/expected/apple-aarch64.s +++ b/crates/test-assembly/crates/test_block/expected/apple-aarch64.s @@ -124,7 +124,7 @@ Lloh7: add sp, sp, #48 ret LBB14_2: - bl SYM(block2::rc_block::rc_new_fail::GENERATED_ID, 0) + bl SYM(block2::rc_block::block_copy_fail::GENERATED_ID, 0) .loh AdrpAdd Lloh6, Lloh7 .loh AdrpAdd Lloh4, Lloh5 .loh AdrpLdrGot Lloh2, Lloh3 diff --git a/crates/test-assembly/crates/test_block/expected/apple-x86_64.s b/crates/test-assembly/crates/test_block/expected/apple-x86_64.s index 6c4c06c8b..88fb8139a 100644 --- a/crates/test-assembly/crates/test_block/expected/apple-x86_64.s +++ b/crates/test-assembly/crates/test_block/expected/apple-x86_64.s @@ -156,7 +156,7 @@ _stack_block_to_rc: pop rbp ret LBB14_2: - call SYM(block2::rc_block::rc_new_fail::GENERATED_ID, 0) + call SYM(block2::rc_block::block_copy_fail::GENERATED_ID, 0) .globl _rc_block .p2align 4, 0x90 diff --git a/crates/test-assembly/crates/test_block/lib.rs b/crates/test-assembly/crates/test_block/lib.rs index 5469efa0e..257f3494c 100644 --- a/crates/test-assembly/crates/test_block/lib.rs +++ b/crates/test-assembly/crates/test_block/lib.rs @@ -5,22 +5,22 @@ use block2::{Block, RcBlock, StackBlock}; #[no_mangle] -fn stack_block_to_rc() -> RcBlock<(i32,), i32> { - StackBlock::new(|x| x + 2).to_rc() +fn stack_block_to_rc() -> RcBlock i32> { + StackBlock::new(|x| x + 2).copy() } #[no_mangle] -fn rc_block() -> RcBlock<(i32,), i32> { +fn rc_block() -> RcBlock i32> { RcBlock::new(|x| x + 2) } #[no_mangle] -fn rc_block_drop(b: Box) -> RcBlock<(i32,), i32> { +fn rc_block_drop(b: Box) -> RcBlock i32> { RcBlock::new(move |x| x + *b) } extern "C" { - fn needs_block(block: &Block<(i32,), i32>); + fn needs_block(block: &Block i32>); } #[no_mangle] diff --git a/crates/test-ui/ui/block_carrying_lifetime.rs b/crates/test-ui/ui/block_carrying_lifetime.rs new file mode 100644 index 000000000..aadf460a0 --- /dev/null +++ b/crates/test-ui/ui/block_carrying_lifetime.rs @@ -0,0 +1,15 @@ +use std::cell::OnceCell; +use std::thread_local; + +use block2::{Block, RcBlock}; + +fn tries_to_retain_past_given_lifetime(block: &Block) { + thread_local! { + pub static BLOCK: OnceCell> = const { OnceCell::new() }; + } + BLOCK.with(|thread_local| { + thread_local.set(block.copy()).unwrap(); + }); +} + +fn main() {} diff --git a/crates/test-ui/ui/block_carrying_lifetime.stderr b/crates/test-ui/ui/block_carrying_lifetime.stderr new file mode 100644 index 000000000..0abb96d73 --- /dev/null +++ b/crates/test-ui/ui/block_carrying_lifetime.stderr @@ -0,0 +1,20 @@ +error[E0521]: borrowed data escapes outside of function + --> ui/block_carrying_lifetime.rs + | + | fn tries_to_retain_past_given_lifetime(block: &Block) { + | ----- + | | + | `block` is a reference that is only valid in the function body + | has type `&Block<(dyn Fn() + '1)>` +... + | / BLOCK.with(|thread_local| { + | | thread_local.set(block.copy()).unwrap(); + | | }); + | | ^ + | | | + | |______`block` escapes the function body here + | argument requires that `'1` must outlive `'static` + | + = note: requirement occurs because of the type `OnceCell>`, which makes the generic argument `RcBlock` invariant + = note: the struct `OnceCell` is invariant over the parameter `T` + = help: see for more information about variance diff --git a/crates/test-ui/ui/block_inner_wrong_variance.rs b/crates/test-ui/ui/block_inner_wrong_variance.rs new file mode 100644 index 000000000..8bdb4262b --- /dev/null +++ b/crates/test-ui/ui/block_inner_wrong_variance.rs @@ -0,0 +1,34 @@ +//! Ideally, blocks should have the same variance as `fn(A) -> R`, namely with +//! `A` being contravariant, and `R` being invariant, see: +//! +//! +//! However, this is currently not the case for `dyn Fn(A) -> R`, and hence +//! not for blocks, see: +//! +use block2::{Block, GlobalBlock, RcBlock, StackBlock}; + +fn block<'a, 'r, 'f, 'b>( + b: &'b Block &'static i32 + 'static>, +) -> &'b Block &'r i32 + 'f> { + b +} + +fn global<'a, 'r, 'f>( + b: GlobalBlock &'static i32 + 'static>, +) -> GlobalBlock &'r i32 + 'f> { + b +} + +fn rc<'a, 'r, 'f>( + b: RcBlock &'static i32 + 'static>, +) -> RcBlock &'r i32 + 'f> { + b +} + +fn stack<'a, 'r, 'f, 'b, F>( + b: StackBlock<'static, (&'a i32,), &'static i32, F>, +) -> StackBlock<'f, (&'static i32,), &'r i32, F> { + b +} + +fn main() {} diff --git a/crates/test-ui/ui/block_inner_wrong_variance.stderr b/crates/test-ui/ui/block_inner_wrong_variance.stderr new file mode 100644 index 000000000..e73a66600 --- /dev/null +++ b/crates/test-ui/ui/block_inner_wrong_variance.stderr @@ -0,0 +1,84 @@ +error: lifetime may not live long enough + --> ui/block_inner_wrong_variance.rs + | + | fn block<'a, 'r, 'f, 'b>( + | -- lifetime `'r` defined here +... + | b + | ^ returning this value requires that `'r` must outlive `'static` + +error: lifetime may not live long enough + --> ui/block_inner_wrong_variance.rs + | + | fn block<'a, 'r, 'f, 'b>( + | -- lifetime `'a` defined here +... + | b + | ^ returning this value requires that `'a` must outlive `'static` + +help: the following changes may resolve your lifetime errors + | + = help: replace `'r` with `'static` + = help: replace `'a` with `'static` + +error: lifetime may not live long enough + --> ui/block_inner_wrong_variance.rs + | + | fn global<'a, 'r, 'f>( + | -- lifetime `'r` defined here +... + | b + | ^ returning this value requires that `'r` must outlive `'static` + +error: lifetime may not live long enough + --> ui/block_inner_wrong_variance.rs + | + | fn global<'a, 'r, 'f>( + | -- lifetime `'a` defined here +... + | b + | ^ returning this value requires that `'a` must outlive `'static` + +error: lifetime may not live long enough + --> ui/block_inner_wrong_variance.rs + | + | fn rc<'a, 'r, 'f>( + | -- lifetime `'r` defined here +... + | b + | ^ returning this value requires that `'r` must outlive `'static` + +error: lifetime may not live long enough + --> ui/block_inner_wrong_variance.rs + | + | fn rc<'a, 'r, 'f>( + | -- lifetime `'a` defined here +... + | b + | ^ returning this value requires that `'a` must outlive `'static` + +error: lifetime may not live long enough + --> ui/block_inner_wrong_variance.rs + | + | fn stack<'a, 'r, 'f, 'b, F>( + | -- lifetime `'r` defined here +... + | b + | ^ returning this value requires that `'r` must outlive `'static` + | + = note: requirement occurs because of the type `StackBlock<'_, (&i32,), &i32, F>`, which makes the generic argument `(&i32,)` invariant + = note: the struct `StackBlock<'f, A, R, Closure>` is invariant over the parameter `A` + = help: see for more information about variance + +error: lifetime may not live long enough + --> ui/block_inner_wrong_variance.rs + | + | fn stack<'a, 'r, 'f, 'b, F>( + | -- lifetime `'a` defined here +... + | b + | ^ returning this value requires that `'a` must outlive `'static` + | + = note: requirement occurs because of the type `StackBlock<'_, (&i32,), &i32, F>`, which makes the generic argument `(&i32,)` invariant + = note: the struct `StackBlock<'f, A, R, Closure>` is invariant over the parameter `A` + = help: see for more information about variance diff --git a/crates/test-ui/ui/block_lifetimes_independent.rs b/crates/test-ui/ui/block_lifetimes_independent.rs new file mode 100644 index 000000000..1f169b6ba --- /dev/null +++ b/crates/test-ui/ui/block_lifetimes_independent.rs @@ -0,0 +1,26 @@ +//! 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; + +fn args<'a, 'b>( + f: impl Fn(&'a i32, &'b i32) + 'static, +) -> RcBlock { + RcBlock::new(f) +} + +fn args_return<'a, 'b>( + f: impl Fn(&'a i32) -> &'b i32 + 'static, +) -> RcBlock &'a i32 + 'static> { + RcBlock::new(f) +} + +fn args_entire<'a, 'b>(f: impl Fn(&'a i32) + 'b) -> RcBlock { + RcBlock::new(f) +} + +fn return_entire<'a, 'b>(f: impl Fn() -> &'a i32 + 'b) -> RcBlock &'b i32 + 'a> { + RcBlock::new(f) +} + +fn main() {} diff --git a/crates/test-ui/ui/block_lifetimes_independent.stderr b/crates/test-ui/ui/block_lifetimes_independent.stderr new file mode 100644 index 000000000..3ec490778 --- /dev/null +++ b/crates/test-ui/ui/block_lifetimes_independent.stderr @@ -0,0 +1,101 @@ +error: lifetime may not live long enough + --> ui/block_lifetimes_independent.rs + | + | fn args<'a, 'b>( + | -- -- lifetime `'b` defined here + | | + | lifetime `'a` defined here +... + | RcBlock::new(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<'a, 'b>( + | -- -- lifetime `'b` defined here + | | + | lifetime `'a` defined here +... + | RcBlock::new(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` + +help: `'a` and `'b` must be the same: replace one with the other + +error: lifetime may not live long enough + --> ui/block_lifetimes_independent.rs + | + | fn args_return<'a, 'b>( + | -- -- lifetime `'b` defined here + | | + | lifetime `'a` defined here +... + | RcBlock::new(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<'a, 'b>( + | -- -- lifetime `'b` defined here + | | + | lifetime `'a` defined here +... + | RcBlock::new(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<'a, 'b>(f: impl Fn(&'a i32) + 'b) -> RcBlock { + | -- -- lifetime `'b` defined here + | | + | lifetime `'a` defined here + | RcBlock::new(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<'a, 'b>(f: impl Fn(&'a i32) + 'b) -> RcBlock { + | -- -- lifetime `'b` defined here + | | + | lifetime `'a` defined here + | RcBlock::new(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<'a, 'b>(f: impl Fn() -> &'a i32 + 'b) -> RcBlock &'b i32 + 'a> { + | -- -- lifetime `'b` defined here + | | + | lifetime `'a` defined here + | RcBlock::new(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<'a, 'b>(f: impl Fn() -> &'a i32 + 'b) -> RcBlock &'b i32 + 'a> { + | -- -- lifetime `'b` defined here + | | + | lifetime `'a` defined here + | RcBlock::new(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/block_parameter_lifetime.rs b/crates/test-ui/ui/block_parameter_lifetime.rs new file mode 100644 index 000000000..4cb576714 --- /dev/null +++ b/crates/test-ui/ui/block_parameter_lifetime.rs @@ -0,0 +1,15 @@ +//! Test the implementation of `BlockFn` not being generic enough. +//! +//! This is a bug, but it is difficult to fix, so let's at least track the +//! error message. +use block2::Block; + +use objc2::encode::Encode; + +fn is_encode() {} + +fn main() { + is_encode::<&Block>(); + is_encode::<&Block &i16>>(); + is_encode::<&Block Fn(&'a i32, &'a i32) -> &'a i32>>(); +} diff --git a/crates/test-ui/ui/block_parameter_lifetime.stderr b/crates/test-ui/ui/block_parameter_lifetime.stderr new file mode 100644 index 000000000..81fbc887a --- /dev/null +++ b/crates/test-ui/ui/block_parameter_lifetime.stderr @@ -0,0 +1,44 @@ +error: implementation of `BlockFn` is not general enough + --> ui/block_parameter_lifetime.rs + | + | is_encode::<&Block>(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `BlockFn` is not general enough + | + = note: `BlockFn` would have to be implemented for the type `dyn for<'a> Fn(&'a i8)` + = note: ...but `BlockFn` is actually implemented for the type `dyn Fn(&'0 i8)`, for some specific lifetime `'0` + +error: implementation of `BlockFn` is not general enough + --> ui/block_parameter_lifetime.rs + | + | is_encode::<&Block &i16>>(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `BlockFn` is not general enough + | + = note: `BlockFn` would have to be implemented for the type `dyn for<'a> Fn(&'a i16) -> &i16` + = note: ...but `BlockFn` is actually implemented for the type `dyn Fn(&'0 i16) -> &i16`, for some specific lifetime `'0` + +error: implementation of `BlockFn` is not general enough + --> ui/block_parameter_lifetime.rs + | + | is_encode::<&Block &i16>>(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `BlockFn` is not general enough + | + = note: `BlockFn` would have to be implemented for the type `dyn for<'a> Fn(&'a i16) -> &i16` + = note: ...but `BlockFn` is actually implemented for the type `dyn Fn(&i16) -> &'0 i16`, for some specific lifetime `'0` + +error: implementation of `BlockFn` is not general enough + --> ui/block_parameter_lifetime.rs + | + | is_encode::<&Block Fn(&'a i32, &'a i32) -> &'a i32>>(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `BlockFn` is not general enough + | + = note: `BlockFn` would have to be implemented for the type `dyn for<'a> Fn(&'a i32, &'a i32) -> &'a i32` + = note: ...but `BlockFn` is actually implemented for the type `dyn Fn(&'0 i32, &'0 i32) -> &i32`, for some specific lifetime `'0` + +error: implementation of `BlockFn` is not general enough + --> ui/block_parameter_lifetime.rs + | + | is_encode::<&Block Fn(&'a i32, &'a i32) -> &'a i32>>(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `BlockFn` is not general enough + | + = note: `BlockFn` would have to be implemented for the type `dyn for<'a> Fn(&'a i32, &'a i32) -> &'a i32` + = note: ...but `BlockFn` is actually implemented for the type `dyn Fn(&i32, &i32) -> &'0 i32`, for some specific lifetime `'0` diff --git a/crates/test-ui/ui/global_block_args_covariant.rs b/crates/test-ui/ui/global_block_args_covariant.rs index ee415637a..fbc77ceb8 100644 --- a/crates/test-ui/ui/global_block_args_covariant.rs +++ b/crates/test-ui/ui/global_block_args_covariant.rs @@ -16,7 +16,7 @@ fn main() { { let x = 5 + 2; - unsafe { PUT_STATIC_IN_THREAD_LOCAL.call((&x,)) }; + PUT_STATIC_IN_THREAD_LOCAL.call((&x,)); } // `CONTAINS_STATIC` now references `x`, which has gone out of scope. diff --git a/crates/test-ui/ui/global_block_args_covariant.stderr b/crates/test-ui/ui/global_block_args_covariant.stderr index 939c131a0..a0a502971 100644 --- a/crates/test-ui/ui/global_block_args_covariant.stderr +++ b/crates/test-ui/ui/global_block_args_covariant.stderr @@ -3,10 +3,10 @@ error[E0597]: `x` does not live long enough | | let x = 5 + 2; | - binding `x` declared here - | unsafe { PUT_STATIC_IN_THREAD_LOCAL.call((&x,)) }; - | ---------------------------------^^--- - | | | - | | borrowed value does not live long enough - | argument requires that `x` is borrowed for `'static` + | PUT_STATIC_IN_THREAD_LOCAL.call((&x,)); + | ---------------------------------^^--- + | | | + | | borrowed value does not live long enough + | argument requires that `x` is borrowed for `'static` | } | - `x` dropped here while still borrowed diff --git a/crates/test-ui/ui/global_block_not_encode.stderr b/crates/test-ui/ui/global_block_not_encode.stderr index 4e7cd95e1..ab0ad0db9 100644 --- a/crates/test-ui/ui/global_block_not_encode.stderr +++ b/crates/test-ui/ui/global_block_not_encode.stderr @@ -17,7 +17,7 @@ error[E0277]: the trait bound `Box: objc2::encode::Encode` is not satisfied u16 and $N others = note: required for `Box` to implement `objc2::encode::EncodeArgument` - = note: required for `(Box,)` to implement `BlockArguments` - = note: required for `GlobalBlock<(Box,)>` to implement `Sync` + = note: required for `(dyn Fn(Box) + 'static)` to implement `BlockFn` + = note: required for `GlobalBlock<(dyn Fn(Box) + 'static)>` to implement `Sync` = note: shared static variables must have a type that implements `Sync` = note: this error originates in the macro `global_block` (in Nightly builds, run with -Z macro-backtrace for more info) 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 new file mode 100644 index 000000000..b166d6e4a --- /dev/null +++ b/crates/test-ui/ui/lifetime_of_closure_tied_to_block.rs @@ -0,0 +1,23 @@ +use block2::{RcBlock, StackBlock}; + +fn main() { + let _ = { + let x = 2; + RcBlock::new(|| x + 2) + }; + + let _ = { + let x = 2; + RcBlock::new(|| x + 2).clone() + }; + + let _ = { + let x = 2; + StackBlock::new(|| x + 2) + }; + + let _ = { + let x = 2; + StackBlock::new(|| 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 new file mode 100644 index 000000000..977162150 --- /dev/null +++ b/crates/test-ui/ui/lifetime_of_closure_tied_to_block.stderr @@ -0,0 +1,48 @@ +error[E0597]: `x` does not live long enough + --> ui/lifetime_of_closure_tied_to_block.rs + | + | RcBlock::new(|| 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 + | + | RcBlock::new(|| 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 + | +11 | RcBlock::new(|| x + 2).clone(); + | + + +error[E0597]: `x` does not live long enough + --> ui/lifetime_of_closure_tied_to_block.rs + | + | StackBlock::new(|| 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 + | + | StackBlock::new(|| 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/not_encode.rs b/crates/test-ui/ui/not_encode.rs index 50d8493af..677f2df64 100644 --- a/crates/test-ui/ui/not_encode.rs +++ b/crates/test-ui/ui/not_encode.rs @@ -15,7 +15,9 @@ fn main() { is_encode::<&()>(); is_encode::<*const ()>(); is_encode::(); - is_encode::<&Block<((), i32), ()>>(); + + is_encode::<&Block>(); + is_encode::<&Block bool>>(); is_encode:: &'static ()>(); is_encode::(); diff --git a/crates/test-ui/ui/not_encode.stderr b/crates/test-ui/ui/not_encode.stderr index 890485b3c..bbd262fdd 100644 --- a/crates/test-ui/ui/not_encode.stderr +++ b/crates/test-ui/ui/not_encode.stderr @@ -113,8 +113,8 @@ note: required by a bound in `is_encode` error[E0277]: the trait bound `(): Encode` is not satisfied --> ui/not_encode.rs | - | is_encode::<&Block<((), i32), ()>>(); - | ^^^^^^^^^^^^^^^^^^^^^ the trait `Encode` is not implemented for `()` + | is_encode::<&Block>(); + | ^^^^^^^^^^^^^^^^^^^^^^^ the trait `Encode` is not implemented for `()` | = help: the following other types implement trait `Encode`: isize @@ -127,9 +127,35 @@ error[E0277]: the trait bound `(): Encode` is not satisfied u16 and $N others = note: required for `()` to implement `EncodeArgument` - = note: required for `((), i32)` to implement `BlockArguments` - = note: required for `block2::Block<((), i32), ()>` to implement `RefEncode` - = note: required for `&block2::Block<((), i32), ()>` to implement `Encode` + = note: required for `dyn Fn((), i32)` to implement `BlockFn` + = note: required for `block2::Block` to implement `RefEncode` + = note: required for `&block2::Block` to implement `Encode` +note: required by a bound in `is_encode` + --> ui/not_encode.rs + | + | fn is_encode() {} + | ^^^^^^ required by this bound in `is_encode` + +error[E0277]: the trait bound `bool: Encode` is not satisfied + --> ui/not_encode.rs + | + | is_encode::<&Block bool>>(); + | ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Encode` is not implemented for `bool` + | + = help: the following other types implement trait `Encode`: + isize + i8 + i16 + i32 + i64 + usize + u8 + u16 + and $N others + = note: required for `bool` to implement `EncodeReturn` + = note: required for `dyn Fn() -> bool` to implement `BlockFn` + = note: required for `block2::Block bool>` to implement `RefEncode` + = note: required for `&block2::Block bool>` to implement `Encode` note: required by a bound in `is_encode` --> ui/not_encode.rs | @@ -216,8 +242,8 @@ note: required by a bound in `is_encode` | ^^^^^^ required by this bound in `is_encode` help: consider removing the leading `&`-reference | -24 - is_encode::<&Sel>(); -24 + is_encode::(); +26 - is_encode::<&Sel>(); +26 + is_encode::(); | error[E0277]: the trait bound `UnsafeCell<&u8>: OptionEncode` is not satisfied diff --git a/crates/test-ui/ui/stack_block_requires_clone.rs b/crates/test-ui/ui/stack_block_requires_clone.rs new file mode 100644 index 000000000..bea3bc26a --- /dev/null +++ b/crates/test-ui/ui/stack_block_requires_clone.rs @@ -0,0 +1,10 @@ +use block2::StackBlock; + +struct Foo; + +fn main() { + let foo = Foo; + let _ = StackBlock::new(move || { + let _ = &foo; + }); +} diff --git a/crates/test-ui/ui/stack_block_requires_clone.stderr b/crates/test-ui/ui/stack_block_requires_clone.stderr new file mode 100644 index 000000000..d2331a08f --- /dev/null +++ b/crates/test-ui/ui/stack_block_requires_clone.stderr @@ -0,0 +1,31 @@ +error[E0277]: the trait bound `Foo: Clone` is not satisfied in `{closure@$DIR/ui/stack_block_requires_clone.rs:7:29: 7:36}` + --> ui/stack_block_requires_clone.rs + | + | let _ = StackBlock::new(move || { + | --------------- ^------ + | | | + | _____________|_______________within this `{closure@$DIR/ui/stack_block_requires_clone.rs:7:29: 7:36}` + | | | + | | required by a bound introduced by this call + | | let _ = &foo; + | | }); + | |_____^ within `{closure@$DIR/ui/stack_block_requires_clone.rs:7:29: 7:36}`, the trait `Clone` is not implemented for `Foo` + | +note: required because it's used within this closure + --> ui/stack_block_requires_clone.rs + | + | let _ = StackBlock::new(move || { + | ^^^^^^^ +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` +help: consider annotating `Foo` with `#[derive(Clone)]` + | +3 + #[derive(Clone)] +4 | struct Foo; + | diff --git a/crates/tests/src/block.rs b/crates/tests/src/block.rs index aa6a95f10..e90e3cb4c 100644 --- a/crates/tests/src/block.rs +++ b/crates/tests/src/block.rs @@ -2,7 +2,8 @@ use core::cell::RefCell; use std::thread_local; use block2::{Block, RcBlock, StackBlock}; -use objc2::{rc::Id, runtime::AnyObject}; +use objc2::rc::Id; +use objc2::runtime::{AnyObject, Bool, NSObject}; #[derive(Default, Debug, Clone, PartialEq, Eq, Hash)] struct Count { @@ -124,11 +125,11 @@ fn stack_to_rc() { }); expected.assert_current(); - let rc1 = stack.to_rc(); + let rc1 = stack.copy(); expected.clone += 1; expected.assert_current(); - let rc2 = stack.to_rc(); + let rc2 = stack.copy(); expected.clone += 1; expected.assert_current(); @@ -164,7 +165,7 @@ fn retain_release_rc_block() { }); expected.assert_current(); - let ptr = &*block as *const Block<_, _> as *mut AnyObject; + let ptr = &*block as *const Block<_> as *mut AnyObject; let obj = unsafe { Id::retain(ptr) }.unwrap(); expected.assert_current(); @@ -194,7 +195,7 @@ fn retain_release_stack_block() { }); expected.assert_current(); - let ptr = &*block as *const Block<_, _> as *mut AnyObject; + let ptr = &*block as *const Block<_> as *mut AnyObject; // Don't use `Id::retain`, as that has debug assertions against the kind // of things GNUStep is doing. let obj = if cfg!(feature = "gnustep-1-7") { @@ -218,3 +219,16 @@ fn retain_release_stack_block() { expected.drop += 1; expected.assert_current(); } + +#[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(); + + assert!(rc_block.call(()).is_false()); +} diff --git a/crates/tests/src/ffi.rs b/crates/tests/src/ffi.rs index 5c4325ce2..ae94dcaa7 100644 --- a/crates/tests/src/ffi.rs +++ b/crates/tests/src/ffi.rs @@ -31,22 +31,24 @@ unsafe impl Encode for LargeStruct { extern "C" { /// Returns a pointer to a global block that returns 7. - pub fn get_int_block() -> *mut Block<(), i32>; + pub fn get_int_block() -> *mut Block i32>; /// Returns a pointer to a copied block that returns `i`. - pub fn get_int_block_with(i: i32) -> *mut Block<(), i32>; + pub fn get_int_block_with(i: i32) -> *mut Block i32>; /// Returns a pointer to a global block that returns its argument + 7. - pub fn get_add_block() -> *mut Block<(i32,), i32>; + pub fn get_add_block() -> *mut Block i32>; /// Returns a pointer to a copied block that returns its argument + `i`. - pub fn get_add_block_with(i: i32) -> *mut Block<(i32,), i32>; + pub fn get_add_block_with(i: i32) -> *mut Block i32>; /// Invokes a block and returns its result. - pub fn invoke_int_block(block: &Block<(), i32>) -> i32; + pub fn invoke_int_block(block: &Block i32>) -> i32; /// Invokes a block with `a` and returns the result. - pub fn invoke_add_block(block: &Block<(i32,), i32>, a: i32) -> i32; + pub fn invoke_add_block(block: &Block i32>, a: i32) -> i32; - pub fn get_large_struct_block() -> *mut Block<(LargeStruct,), LargeStruct>; - pub fn get_large_struct_block_with(i: LargeStruct) -> *mut Block<(LargeStruct,), LargeStruct>; + pub fn get_large_struct_block() -> *mut Block LargeStruct>; + pub fn get_large_struct_block_with( + i: LargeStruct, + ) -> *mut Block LargeStruct>; pub fn invoke_large_struct_block( - block: &Block<(LargeStruct,), LargeStruct>, + block: &Block LargeStruct>, s: LargeStruct, ) -> LargeStruct; @@ -55,7 +57,7 @@ extern "C" { #[no_mangle] extern "C" fn debug_block(block: *mut c_void) { - let block: &Block<(), ()> = unsafe { &*(block as *const Block<(), ()>) }; + let block: &Block = unsafe { &*(block as *const Block) }; std::println!("{block:#?}"); } diff --git a/crates/tests/src/lib.rs b/crates/tests/src/lib.rs index ea1add68f..fd3690eaf 100644 --- a/crates/tests/src/lib.rs +++ b/crates/tests/src/lib.rs @@ -23,30 +23,30 @@ mod test_object; use crate::ffi::LargeStruct; -pub fn get_int_block_with(i: i32) -> RcBlock<(), i32> { +pub fn get_int_block_with(i: i32) -> RcBlock i32> { unsafe { let ptr = ffi::get_int_block_with(i); RcBlock::from_raw(ptr).unwrap() } } -pub fn get_add_block_with(i: i32) -> RcBlock<(i32,), i32> { +pub fn get_add_block_with(i: i32) -> RcBlock i32> { unsafe { let ptr = ffi::get_add_block_with(i); RcBlock::from_raw(ptr).unwrap() } } -pub fn invoke_int_block(block: &Block<(), i32>) -> i32 { +pub fn invoke_int_block(block: &Block i32>) -> i32 { unsafe { ffi::invoke_int_block(block) } } -pub fn invoke_add_block(block: &Block<(i32,), i32>, a: i32) -> i32 { +pub fn invoke_add_block(block: &Block i32>, a: i32) -> i32 { unsafe { ffi::invoke_add_block(block, a) } } pub fn invoke_large_struct_block( - block: &Block<(LargeStruct,), LargeStruct>, + block: &Block LargeStruct>, x: LargeStruct, ) -> LargeStruct { unsafe { ffi::invoke_large_struct_block(block, x) } @@ -73,17 +73,13 @@ mod tests { #[test] fn test_call_block() { let block = get_int_block_with(13); - unsafe { - assert_eq!(block.call(()), 13); - } + assert_eq!(block.call(()), 13); } #[test] fn test_call_block_args() { let block = get_add_block_with(13); - unsafe { - assert_eq!(block.call((2,)), 15); - } + assert_eq!(block.call((2,)), 15); } #[test] @@ -107,13 +103,13 @@ mod tests { let block = StackBlock::new(move || s.len() as i32); assert_eq!(invoke_int_block(&block), expected_len); - let copied = block.to_rc(); + let copied = block.copy(); assert_eq!(invoke_int_block(&copied), expected_len); } #[test] fn test_block_stack_move() { - fn make_block() -> StackBlock<(), i32, impl Fn() -> i32> { + fn make_block() -> StackBlock<'static, (), i32, impl Fn() -> i32> { let x = 7; StackBlock::new(move || x) } @@ -136,7 +132,7 @@ mod tests { let mut new_data = data; new_data.mutate(); - assert_eq!(unsafe { BLOCK.call((data,)) }, new_data); + assert_eq!(BLOCK.call((data,)), new_data); assert_eq!(invoke_large_struct_block(&BLOCK, data), new_data); let block = StackBlock::new(|mut x: LargeStruct| { @@ -144,7 +140,7 @@ mod tests { x }); assert_eq!(invoke_large_struct_block(&block, data), new_data); - let block = block.to_rc(); + let block = block.copy(); assert_eq!(invoke_large_struct_block(&block, data), new_data); } }