From 01ac48b36f3fc67bbbc0296a901979eb0edee396 Mon Sep 17 00:00:00 2001 From: Andrew Kaster Date: Sun, 16 Mar 2025 17:35:38 -0600 Subject: [PATCH] AK: Support storing blocks in AK::Function This has two slightly different implementations for ARC and non-ARC compiler modes. The main idea is to store a block pointer as our closure and use either ARC magic or BlockRuntime methods to manage the memory for the block. Things are complicated by the fact that we don't yet force-enable swift, so we can't count on the swift.org llvm fork being our compiler toolchain. The patch adds some CMake checks and ifdefs to still support environments without support for blocks or ARC. --- AK/Function.h | 102 +++++++++++++++++++--- AK/StdLibExtraDetails.h | 22 +++++ Meta/CMake/common_options.cmake | 19 +++++ Tests/AK/CMakeLists.txt | 10 +++ Tests/AK/TestFunction.mm | 145 ++++++++++++++++++++++++++++++++ 5 files changed, 287 insertions(+), 11 deletions(-) create mode 100644 Tests/AK/TestFunction.mm diff --git a/AK/Function.h b/AK/Function.h index 3faa013ffab..182dc06e104 100644 --- a/AK/Function.h +++ b/AK/Function.h @@ -2,6 +2,7 @@ * Copyright (C) 2016 Apple Inc. All rights reserved. * Copyright (c) 2021, Gunnar Beutner * Copyright (c) 2018-2023, Andreas Kling + * Copyright (c) 2025, Andrew Kaster * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -34,8 +35,14 @@ #include #include #include +#include #include +// BlockRuntime methods for Objective-C block closure support. +extern "C" void* _Block_copy(void const*); +extern "C" void _Block_release(void const*); +extern "C" size_t Block_size(void const*); + namespace AK { // These annotations are used to avoid capturing a variable with local storage in a lambda that outlives it @@ -48,6 +55,17 @@ namespace AK { # define IGNORE_USE_IN_ESCAPING_LAMBDA #endif +namespace Detail { +#ifdef AK_HAS_OBJC_ARC +inline constexpr bool HaveObjcArc = true; +#else +inline constexpr bool HaveObjcArc = false; +#endif + +// validated in TestFunction.mm +inline constexpr size_t block_layout_size = 32; +} + template class Function; @@ -84,7 +102,7 @@ public: if (!m_size) return {}; if (auto* wrapper = callable_wrapper()) - return ReadonlyBytes { wrapper, m_size }; + return ReadonlyBytes { wrapper->raw_callable(), m_size }; return {}; } @@ -102,6 +120,13 @@ public: init_with_callable(move(f), CallableKind::FunctionPointer); } + template + Function(BlockType b) + requires((IsBlockClosure && IsCallableWithArguments)) + { + init_with_callable(move(b), CallableKind::Block); + } + Function(Function&& other) { move_from(move(other)); @@ -141,6 +166,15 @@ public: return *this; } + template + Function& operator=(BlockType&& block) + requires((IsBlockClosure && IsCallableWithArguments)) + { + clear(); + init_with_callable(static_cast>(block), CallableKind::Block); + return *this; + } + Function& operator=(nullptr_t) { clear(); @@ -160,6 +194,7 @@ private: enum class CallableKind { FunctionPointer, FunctionObject, + Block, }; class CallableWrapperBase { @@ -169,6 +204,7 @@ private: virtual Out call(In...) = 0; virtual void destroy() = 0; virtual void init_and_swap(u8*, size_t) = 0; + virtual void const* raw_callable() const = 0; }; template @@ -189,7 +225,15 @@ private: void destroy() final override { - delete this; + if constexpr (IsBlockClosure) { + if constexpr (Detail::HaveObjcArc) + m_callable = nullptr; + else + _Block_release(m_callable); + } else { + // This code is a bit too clever for gcc. Pinky promise we're only deleting heap objects. + AK_IGNORE_DIAGNOSTIC("-Wfree-nonheap-object", delete this); + } } // NOLINTNEXTLINE(readability-non-const-parameter) False positive; destination is used in a placement new expression @@ -199,6 +243,14 @@ private: new (destination) CallableWrapper { move(m_callable) }; } + void const* raw_callable() const final override + { + if constexpr (IsBlockClosure) + return static_cast(bridge_cast(m_callable)) + Detail::block_layout_size; + else + return &m_callable; + } + private: CallableType m_callable; }; @@ -207,6 +259,7 @@ private: NullPointer, Inline, Outline, + Block, }; CallableWrapperBase* callable_wrapper() const @@ -215,6 +268,7 @@ private: case FunctionKind::NullPointer: return nullptr; case FunctionKind::Inline: + case FunctionKind::Block: return bit_cast(&m_storage); case FunctionKind::Outline: return *bit_cast(&m_storage); @@ -234,12 +288,22 @@ private: } m_deferred_clear = false; auto* wrapper = callable_wrapper(); - if (m_kind == FunctionKind::Inline) { + switch (m_kind) { + case FunctionKind::Inline: VERIFY(wrapper); wrapper->~CallableWrapperBase(); - } else if (m_kind == FunctionKind::Outline) { + break; + case FunctionKind::Outline: VERIFY(wrapper); wrapper->destroy(); + break; + case FunctionKind::Block: + VERIFY(wrapper); + wrapper->destroy(); + wrapper->~CallableWrapperBase(); + break; + case FunctionKind::NullPointer: + break; } m_kind = FunctionKind::NullPointer; } @@ -256,18 +320,33 @@ private: } VERIFY(m_call_nesting_level == 0); using WrapperType = CallableWrapper; + if (callable_kind == CallableKind::FunctionObject) + m_size = sizeof(Callable); + else + m_size = 0; + if constexpr (IsBlockClosure) { + auto block_size = Block_size(bridge_cast(callable)); + VERIFY(block_size >= Detail::block_layout_size); + m_size = block_size - Detail::block_layout_size; + } + if constexpr (alignof(Callable) > inline_alignment || sizeof(WrapperType) > inline_capacity) { *bit_cast(&m_storage) = new WrapperType(forward(callable)); m_kind = FunctionKind::Outline; } else { static_assert(sizeof(WrapperType) <= inline_capacity); - new (m_storage) WrapperType(forward(callable)); - m_kind = FunctionKind::Inline; + if constexpr (IsBlockClosure) { + if constexpr (Detail::HaveObjcArc) { + new (m_storage) WrapperType(forward(callable)); + } else { + new (m_storage) WrapperType(reinterpret_cast(_Block_copy(callable))); + } + m_kind = FunctionKind::Block; + } else { + new (m_storage) WrapperType(forward(callable)); + m_kind = FunctionKind::Inline; + } } - if (callable_kind == CallableKind::FunctionObject) - m_size = sizeof(WrapperType); - else - m_size = 0; } void move_from(Function&& other) @@ -279,8 +358,9 @@ private: case FunctionKind::NullPointer: break; case FunctionKind::Inline: + case FunctionKind::Block: other_wrapper->init_and_swap(m_storage, inline_capacity); - m_kind = FunctionKind::Inline; + m_kind = other.m_kind; break; case FunctionKind::Outline: *bit_cast(&m_storage) = other_wrapper; diff --git a/AK/StdLibExtraDetails.h b/AK/StdLibExtraDetails.h index 15aded72993..d3b5be83bec 100644 --- a/AK/StdLibExtraDetails.h +++ b/AK/StdLibExtraDetails.h @@ -142,6 +142,27 @@ inline constexpr bool IsFunction = true; template inline constexpr bool IsFunction = true; +template +inline constexpr bool IsBlockClosure = false; +#ifdef AK_HAS_BLOCKS +template +inline constexpr bool IsBlockClosure = true; +template +inline constexpr bool IsBlockClosure = true; +template +inline constexpr bool IsBlockClosure = true; +template +inline constexpr bool IsBlockClosure = true; +template +inline constexpr bool IsBlockClosure = true; +template +inline constexpr bool IsBlockClosure = true; +template +inline constexpr bool IsBlockClosure = true; +template +inline constexpr bool IsBlockClosure = true; +#endif + template inline constexpr bool IsRvalueReference = false; template @@ -641,6 +662,7 @@ using AK::Detail::InvokeResult; using AK::Detail::IsArithmetic; using AK::Detail::IsAssignable; using AK::Detail::IsBaseOf; +using AK::Detail::IsBlockClosure; using AK::Detail::IsClass; using AK::Detail::IsConst; using AK::Detail::IsConstructible; diff --git a/Meta/CMake/common_options.cmake b/Meta/CMake/common_options.cmake index 23bd279c767..4cf6863b897 100644 --- a/Meta/CMake/common_options.cmake +++ b/Meta/CMake/common_options.cmake @@ -45,3 +45,22 @@ serenity_option(ENABLE_STD_STACKTRACE OFF CACHE BOOL "Force use of std::stacktra if (ENABLE_SWIFT) include(${CMAKE_CURRENT_LIST_DIR}/Swift/swift-settings.cmake) endif() + +include(CheckCXXSourceCompiles) +set(BLOCKS_REQUIRED_LIBRARIES "") +if (NOT APPLE) + find_package(BlocksRuntime) + if (BlocksRuntime_FOUND) + set(BLOCKS_REQUIRED_LIBRARIES BlocksRuntime::BlocksRuntime) + set(CMAKE_REQUIRED_LIBRARIES BlocksRuntime::BlocksRuntime) + endif() +endif() +check_cxx_source_compiles([=[ + int main() { __block int x = 0; auto b = ^{++x;}; b(); } +]=] CXX_COMPILER_SUPPORTS_BLOCKS) + +set(CMAKE_REQUIRED_FLAGS "-fobjc-arc") +check_cxx_source_compiles([=[ + int main() { auto b = ^{}; auto __weak w = b; w(); } +]=] CXX_COMPILER_SUPPORTS_OBJC_ARC) +unset(CMAKE_REQUIRED_FLAGS) diff --git a/Tests/AK/CMakeLists.txt b/Tests/AK/CMakeLists.txt index 10f4a245050..92923f9a67b 100644 --- a/Tests/AK/CMakeLists.txt +++ b/Tests/AK/CMakeLists.txt @@ -88,6 +88,16 @@ foreach(source IN LISTS AK_TEST_SOURCES) serenity_test("${source}" AK) endforeach() +if (CXX_COMPILER_SUPPORTS_BLOCKS) + serenity_test(TestFunction.mm AK NAME TestFunction) + target_link_libraries(TestFunction PRIVATE ${BLOCKS_REQUIRED_LIBRARIES}) +endif() +if (CXX_COMPILER_SUPPORTS_OBJC_ARC) + serenity_test(TestFunction.mm AK NAME TestFunctionArc) + target_compile_options(TestFunctionArc PRIVATE -fobjc-arc) + target_link_libraries(TestFunction PRIVATE ${BLOCKS_REQUIRED_LIBRARIES}) +endif() + target_link_libraries(TestString PRIVATE LibUnicode) if (ENABLE_SWIFT) diff --git a/Tests/AK/TestFunction.mm b/Tests/AK/TestFunction.mm new file mode 100644 index 00000000000..3d6f562c96c --- /dev/null +++ b/Tests/AK/TestFunction.mm @@ -0,0 +1,145 @@ +/* + * Copyright (c) 2025, Andrew Kaster + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include + +#include +#include +#include + +TEST_CASE(SimpleBlock) +{ + auto b = ^{ }; + + static_assert(IsBlockClosure); + + auto f = Function(b); + + f(); +} + +TEST_CASE(BlockCaptureInt) +{ + __block int x = 0; + auto b = ^{ + x = 2; + }; + auto f = Function(b); + + f(); + + EXPECT_EQ(x, 2); +} + +TEST_CASE(BlockCaptureString) +{ + __block String s = "hello"_string; + auto b = ^{ + s = "world"_string; + }; + auto f = Function(b); + + f(); + + EXPECT_EQ(s, "world"_string); +} + +TEST_CASE(BlockCaptureLongStringAndInt) +{ + __block String s = "hello, world, this is a long string to avoid small string optimization"_string; + __block int x = 0; + auto b = ^{ + s = "world, hello, this is a long string to avoid small string optimization"_string; + x = 2; + }; + auto f = Function(b); + + f(); + + EXPECT_EQ(s, "world, hello, this is a long string to avoid small string optimization"_string); + EXPECT_EQ(x, 2); +} + +// Struct definitions from llvm-project/compiler-rt/lib/BlocksRuntime/Block_private.h @ d0177670a0e59e9d9719386f85bb78de0929407c +struct Block_descriptor { + unsigned long int reserved; + unsigned long int size; + void (*copy)(void* dst, void* src); + void (*dispose)(void*); +}; + +struct Block_layout { + void* isa; + int flags; + int reserved; + void (*invoke)(void*, ...); + struct Block_descriptor* descriptor; + /* Imported variables. */ +}; +// This check is super important for proper tracking of block closure captures +static_assert(sizeof(Block_layout) == AK::Detail::block_layout_size); + +TEST_CASE(BlockPointerCaptures) +{ + int x = 0; + int* p = &x; + + auto b = ^{ + *p = 2; + }; + + auto f = Function(b); + auto span = f.raw_capture_range(); + + int* captured_p = ByteReader::load_pointer(span.data()); + EXPECT_EQ(captured_p, p); + + f(); + + EXPECT_EQ(x, 2); +} + +TEST_CASE(AssignBlock) +{ + auto b = ^{ }; + + auto f = Function(b); + + auto b2 = ^{ }; + + f = b2; + + f(); + + f = b; + + f(); +} + +#ifdef AK_HAS_OBJC_ARC +TEST_CASE(AssignWeakBlock) +{ + __block int count = 0; + Function f; + + { + auto b = ^{ ++count; }; + f = b; + } + f(); + EXPECT_EQ(count, 1); + + { + auto b = ^{ ++count; }; + auto const __weak weak_b = b; + f = weak_b; + f(); + EXPECT_EQ(count, 2); + } + f(); + EXPECT_EQ(count, 3); +} +#endif