From c5b44beb6d8a62775d11a4141e9fb77de0a3131b Mon Sep 17 00:00:00 2001 From: dayfine Date: Thu, 29 Oct 2026 21:56:49 -0700 Subject: [PATCH 1/3] Common: add MoveOnlyFunction, a move-only callable wrapper std::function is copyable: copying it forks a stateful lambda's state, and it cannot hold move-only captures. MoveOnlyFunction is an alias of std::move_only_function where the standard library has it, and a minimal R(Args...) fallback for Apple's libc++, which lacks it. Tests: move-only capture, moving keeps one state, empty and nullptr comparison, reassignment; copy is rejected at compile time. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AKYzhrMnPK61XHWEch4a6E --- Common/Cpp/MoveOnlyFunction.h | 118 +++++++++++ Common/Cpp/MoveOnlyFunction_Tests.cpp | 191 ++++++++++++++++++ Common/Cpp/MoveOnlyFunction_Tests.h | 21 ++ .../ComputerPrograms_UnitTestRunner.cpp | 2 + SerialPrograms/cmake/SourceFiles.cmake | 3 + 5 files changed, 335 insertions(+) create mode 100644 Common/Cpp/MoveOnlyFunction.h create mode 100644 Common/Cpp/MoveOnlyFunction_Tests.cpp create mode 100644 Common/Cpp/MoveOnlyFunction_Tests.h diff --git a/Common/Cpp/MoveOnlyFunction.h b/Common/Cpp/MoveOnlyFunction.h new file mode 100644 index 0000000000..9cc71f3422 --- /dev/null +++ b/Common/Cpp/MoveOnlyFunction.h @@ -0,0 +1,118 @@ +/* Move-Only Function + * + * From: https://github.com/PokemonAutomation/ + * + * A callable wrapper that can be moved but not copied. + * + * Prefer this over std::function for a callback that has one owner. + * Copying a std::function copies its callable, so a stateful lambda + * silently splits into two independent states. And std::function cannot + * hold a lambda that captures a move-only object (e.g. std::unique_ptr). + * + * This is std::move_only_function (C++23) where the standard library has + * it. Apple's libc++ (Xcode 26 / Apple Clang 21) does not, so this header + * has a minimal fallback for the plain "R(Args...)" signature. Delete the + * fallback once every supported compiler defines + * __cpp_lib_move_only_function. + * + */ + +#ifndef PokemonAutomation_MoveOnlyFunction_H +#define PokemonAutomation_MoveOnlyFunction_H + +#include +#include + +#ifndef __cpp_lib_move_only_function +#include +#include +#include +#include +#endif + +namespace PokemonAutomation{ + + + +#ifdef __cpp_lib_move_only_function + +template +using MoveOnlyFunction = std::move_only_function; + +#else + +template +class MoveOnlyFunction; + +template +class MoveOnlyFunction{ +public: + MoveOnlyFunction() noexcept = default; + MoveOnlyFunction(std::nullptr_t) noexcept {} + + template + requires ( + !std::is_same_v, MoveOnlyFunction> && + std::is_invocable_r_v&, Args...> + ) + MoveOnlyFunction(Function&& function){ + using Stored = std::decay_t; + // Like std::move_only_function, a null function pointer makes an + // empty wrapper. + if constexpr (std::is_pointer_v || std::is_member_pointer_v){ + if (function == nullptr){ + return; + } + } + m_callable = std::make_unique>(std::forward(function)); + } + + MoveOnlyFunction(MoveOnlyFunction&&) noexcept = default; + MoveOnlyFunction& operator=(MoveOnlyFunction&&) noexcept = default; + MoveOnlyFunction(const MoveOnlyFunction&) = delete; + MoveOnlyFunction& operator=(const MoveOnlyFunction&) = delete; + + explicit operator bool() const noexcept{ + return m_callable != nullptr; + } + friend bool operator==(const MoveOnlyFunction& function, std::nullptr_t) noexcept{ + return !function; + } + + // Calling an empty wrapper is undefined, as for std::move_only_function. + R operator()(Args... args){ + return m_callable->call(std::forward(args)...); + } + + +private: + class Callable{ + public: + virtual ~Callable() = default; + virtual R call(Args&&... args) = 0; + }; + + template + class Holder final : public Callable{ + public: + template + explicit Holder(Source&& source) + : m_stored(std::forward(source)) + {} + virtual R call(Args&&... args) override{ + return std::invoke_r(m_stored, std::forward(args)...); + } + + private: + Stored m_stored; + }; + + std::unique_ptr m_callable; +}; + +#endif + + + +} +#endif diff --git a/Common/Cpp/MoveOnlyFunction_Tests.cpp b/Common/Cpp/MoveOnlyFunction_Tests.cpp new file mode 100644 index 0000000000..c133e8b8ec --- /dev/null +++ b/Common/Cpp/MoveOnlyFunction_Tests.cpp @@ -0,0 +1,191 @@ +/* Move-Only Function Tests + * + * From: https://github.com/PokemonAutomation/ + * + */ + +#include +#include +#include +#include +#include "TestRunners/UnitTestDatabase.h" +#include "MoveOnlyFunction.h" +#include "MoveOnlyFunction_Tests.h" + +namespace PokemonAutomation{ +namespace{ + + + +// Copying would fork a stateful callable, so it must not compile. +static_assert(!std::is_copy_constructible_v>); +static_assert(!std::is_copy_assignable_v>); +static_assert(std::is_nothrow_move_constructible_v>); +static_assert(std::is_nothrow_move_assignable_v>); + + + +std::string expected_int(const char* what, int expected, int actual){ + return std::string(what) + ": expected " + std::to_string(expected) + + ", got " + std::to_string(actual); +} + + + +class Test_MoveOnlyFunction_MoveOnlyCapture : public UnitTest{ +public: + Test_MoveOnlyFunction_MoveOnlyCapture() + : UnitTest("Common::MoveOnlyFunction - Move-only capture") + {} + + virtual UnitTestResult run(Logger& logger, CancellableScope& scope) const override{ + auto value = std::make_unique(42); + MoveOnlyFunction function = [value = std::move(value)](int add){ + return *value + add; + }; + int actual = function(1); + if (actual != 43){ + return UnitTestResult(expected_int("call with a unique_ptr capture", 43, actual)); + } + + MoveOnlyFunction moved = std::move(function); + actual = moved(2); + if (actual != 44){ + return UnitTestResult(expected_int("call after a move", 44, actual)); + } + return true; + } +}; + + + +class Test_MoveOnlyFunction_StateNotForked : public UnitTest{ +public: + Test_MoveOnlyFunction_StateNotForked() + : UnitTest("Common::MoveOnlyFunction - Moving keeps one state") + {} + + virtual UnitTestResult run(Logger& logger, CancellableScope& scope) const override{ + MoveOnlyFunction counter = [count = 0]() mutable{ + return ++count; + }; + int actual = counter(); + if (actual != 1){ + return UnitTestResult(expected_int("first call", 1, actual)); + } + + // The moved-to wrapper continues the same count. + MoveOnlyFunction constructed = std::move(counter); + actual = constructed(); + if (actual != 2){ + return UnitTestResult(expected_int("call after a move construction", 2, actual)); + } + + MoveOnlyFunction assigned; + assigned = std::move(constructed); + actual = assigned(); + if (actual != 3){ + return UnitTestResult(expected_int("call after a move assignment", 3, actual)); + } + return true; + } +}; + + + +int return_seven(){ + return 7; +} + +class Test_MoveOnlyFunction_Empty : public UnitTest{ +public: + Test_MoveOnlyFunction_Empty() + : UnitTest("Common::MoveOnlyFunction - Empty is false") + {} + + virtual UnitTestResult run(Logger& logger, CancellableScope& scope) const override{ + MoveOnlyFunction defaulted; + if (defaulted){ + return UnitTestResult("default constructed: expected empty, got non-empty"); + } + MoveOnlyFunction from_nullptr = nullptr; + if (from_nullptr){ + return UnitTestResult("constructed from nullptr: expected empty, got non-empty"); + } + int (*null_pointer)() = nullptr; + MoveOnlyFunction from_null_pointer = null_pointer; + if (from_null_pointer){ + return UnitTestResult("constructed from a null function pointer: expected empty, got non-empty"); + } + + MoveOnlyFunction from_pointer = &return_seven; + if (!from_pointer){ + return UnitTestResult("constructed from a function pointer: expected non-empty, got empty"); + } + int actual = from_pointer(); + if (actual != 7){ + return UnitTestResult(expected_int("call through a function pointer", 7, actual)); + } + return true; + } +}; + + + +class Test_MoveOnlyFunction_Reassignment : public UnitTest{ +public: + Test_MoveOnlyFunction_Reassignment() + : UnitTest("Common::MoveOnlyFunction - Reassignment") + {} + + virtual UnitTestResult run(Logger& logger, CancellableScope& scope) const override{ + // The use count shows whether the wrapper still holds the first + // callable. + auto token = std::make_shared(1); + MoveOnlyFunction function = [token]{ + return *token; + }; + int actual = function(); + if (actual != 1){ + return UnitTestResult(expected_int("first callable", 1, actual)); + } + + function = []{ + return 2; + }; + actual = function(); + if (actual != 2){ + return UnitTestResult(expected_int("call after reassignment", 2, actual)); + } + actual = (int)token.use_count(); + if (actual != 1){ + return UnitTestResult(expected_int("token owners after reassignment", 1, actual)); + } + + function = nullptr; + if (function){ + return UnitTestResult("assigned nullptr: expected empty, got non-empty"); + } + if (function != nullptr){ + return UnitTestResult("assigned nullptr: expected == nullptr, got != nullptr"); + } + return true; + } +}; + + + +} // anonymous namespace + + + +void add_tests_MoveOnlyFunction(UnitTestDatabase& database){ + database.add(); + database.add(); + database.add(); + database.add(); +} + + + +} diff --git a/Common/Cpp/MoveOnlyFunction_Tests.h b/Common/Cpp/MoveOnlyFunction_Tests.h new file mode 100644 index 0000000000..c43f518bdc --- /dev/null +++ b/Common/Cpp/MoveOnlyFunction_Tests.h @@ -0,0 +1,21 @@ +/* Move-Only Function Tests + * + * From: https://github.com/PokemonAutomation/ + * + */ + +#ifndef PokemonAutomation_MoveOnlyFunction_Tests_H +#define PokemonAutomation_MoveOnlyFunction_Tests_H + +#include "Common/Cpp/TestRunners/UnitTestDatabase.h" + +namespace PokemonAutomation{ + + + +void add_tests_MoveOnlyFunction(UnitTestDatabase& database); + + + +} +#endif diff --git a/SerialPrograms/Source/ComputerPrograms/ComputerPrograms_UnitTestRunner.cpp b/SerialPrograms/Source/ComputerPrograms/ComputerPrograms_UnitTestRunner.cpp index 1c19573466..d434036d6e 100644 --- a/SerialPrograms/Source/ComputerPrograms/ComputerPrograms_UnitTestRunner.cpp +++ b/SerialPrograms/Source/ComputerPrograms/ComputerPrograms_UnitTestRunner.cpp @@ -5,6 +5,7 @@ */ #include "Common/Cpp/ListenerSet.h" +#include "Common/Cpp/MoveOnlyFunction_Tests.h" #include "Common/Cpp/ScopeExit.h" #include "Common/Cpp/PrettyPrint.h" #include "Common/Cpp/TestRunners/TestImageDistortionManager.h" @@ -51,6 +52,7 @@ UnitTestDatabase make_UNIT_TESTS_ALL(){ add_tests_QtEventThreadPool(ret, GlobalThreadPools::computation_normal()); add_tests_BlackBorderDetector(ret); add_tests_ListenerSet(ret); + add_tests_MoveOnlyFunction(ret); add_tests_AudioInfo(ret); add_tests_ChannelMixerMatrix(ret); add_tests_FFTStreamer(ret); diff --git a/SerialPrograms/cmake/SourceFiles.cmake b/SerialPrograms/cmake/SourceFiles.cmake index 9e4e0895bb..f640a174ce 100644 --- a/SerialPrograms/cmake/SourceFiles.cmake +++ b/SerialPrograms/cmake/SourceFiles.cmake @@ -118,6 +118,9 @@ file(GLOB CORE_LIBRARY_SOURCES ../Common/Cpp/Logging/MultiOutputLogger.h ../Common/Cpp/Logging/TaggedLogger.cpp ../Common/Cpp/Logging/TaggedLogger.h + ../Common/Cpp/MoveOnlyFunction.h + ../Common/Cpp/MoveOnlyFunction_Tests.cpp + ../Common/Cpp/MoveOnlyFunction_Tests.h ../Common/Cpp/Options/BatchOption.cpp ../Common/Cpp/Options/BatchOption.h ../Common/Cpp/Options/BooleanCheckBoxOption.cpp From 38bb6aeddb7c2511735de09caf893978529477d6 Mon Sep 17 00:00:00 2001 From: dayfine Date: Thu, 29 Oct 2026 22:07:21 -0700 Subject: [PATCH 2/3] Common: drop the noexcept move-assignment check from MoveOnlyFunction tests The standard does not make std::move_only_function's move assignment noexcept, and MSVC's STL does not. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AKYzhrMnPK61XHWEch4a6E --- Common/Cpp/MoveOnlyFunction_Tests.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/Common/Cpp/MoveOnlyFunction_Tests.cpp b/Common/Cpp/MoveOnlyFunction_Tests.cpp index c133e8b8ec..72ed5998ae 100644 --- a/Common/Cpp/MoveOnlyFunction_Tests.cpp +++ b/Common/Cpp/MoveOnlyFunction_Tests.cpp @@ -21,7 +21,6 @@ namespace{ static_assert(!std::is_copy_constructible_v>); static_assert(!std::is_copy_assignable_v>); static_assert(std::is_nothrow_move_constructible_v>); -static_assert(std::is_nothrow_move_assignable_v>); From 93487aaa4a54a5bc0eb325d16e27ff7224338796 Mon Sep 17 00:00:00 2001 From: dayfine Date: Sat, 31 Oct 2026 08:39:41 -0700 Subject: [PATCH 3/3] Common: hold thread pool tasks in MoveOnlyFunction ThreadPool::dispatch(), dispatch_now_blocking() and try_dispatch_now() take a task that the pool owns until it runs, so it never needs to be copied. Taking MoveOnlyFunction lets a task capture move-only state and rules out forking a stateful task by accident. Callers that pass a lambda are unchanged; the two try_dispatch_now() callers that build the task first now build a MoveOnlyFunction. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AKYzhrMnPK61XHWEch4a6E --- Common/Cpp/Concurrency/Backends/AsyncTask_Default.h | 6 +++--- Common/Cpp/Concurrency/Backends/ThreadPool_Default.cpp | 6 +++--- Common/Cpp/Concurrency/Backends/ThreadPool_Default.h | 6 +++--- Common/Cpp/Concurrency/ThreadPool.h | 7 ++++--- .../VideoPipeline/Backends/SnapshotManager.cpp | 3 ++- .../ShinyDetection/PokemonSwSh_ShinySparkleSet.cpp | 3 ++- 6 files changed, 17 insertions(+), 14 deletions(-) diff --git a/Common/Cpp/Concurrency/Backends/AsyncTask_Default.h b/Common/Cpp/Concurrency/Backends/AsyncTask_Default.h index 6e83ebde75..e797904803 100644 --- a/Common/Cpp/Concurrency/Backends/AsyncTask_Default.h +++ b/Common/Cpp/Concurrency/Backends/AsyncTask_Default.h @@ -7,9 +7,9 @@ #ifndef PokemonAutomation_AsyncTask_Default_H #define PokemonAutomation_AsyncTask_Default_H -#include #include #include +#include "Common/Cpp/MoveOnlyFunction.h" #include "Common/Cpp/Concurrency/SpinPause.h" #include "Common/Cpp/Concurrency/Mutex.h" #include "Common/Cpp/Concurrency/ConditionVariable.h" @@ -50,7 +50,7 @@ class AsyncTask_Cpp : public AsyncTaskCore{ pause(); } } - AsyncTask_Cpp(std::function task) + AsyncTask_Cpp(MoveOnlyFunction task) : m_task(std::move(task)) , m_state(State::NOT_STARTED) {} @@ -102,7 +102,7 @@ class AsyncTask_Cpp : public AsyncTaskCore{ private: - std::function m_task; + MoveOnlyFunction m_task; std::atomic m_state; std::exception_ptr m_exception; diff --git a/Common/Cpp/Concurrency/Backends/ThreadPool_Default.cpp b/Common/Cpp/Concurrency/Backends/ThreadPool_Default.cpp index 297ba275cb..bf5ed9ce7a 100644 --- a/Common/Cpp/Concurrency/Backends/ThreadPool_Default.cpp +++ b/Common/Cpp/Concurrency/Backends/ThreadPool_Default.cpp @@ -90,7 +90,7 @@ WallDuration ThreadPool_Default::cpu_time() const{ return ret; } -AsyncTask ThreadPool_Default::dispatch(std::function&& func){ +AsyncTask ThreadPool_Default::dispatch(MoveOnlyFunction&& func){ AsyncTask task; { std::unique_lock lg(m_lock); @@ -114,7 +114,7 @@ AsyncTask ThreadPool_Default::dispatch(std::function&& func){ m_thread_cv.notify_one(); return task; } -AsyncTask ThreadPool_Default::dispatch_now_blocking(std::function&& func){ +AsyncTask ThreadPool_Default::dispatch_now_blocking(MoveOnlyFunction&& func){ AsyncTask task; { std::unique_lock lg(m_lock); @@ -138,7 +138,7 @@ AsyncTask ThreadPool_Default::dispatch_now_blocking(std::function&& func m_thread_cv.notify_one(); return task; } -AsyncTask ThreadPool_Default::try_dispatch_now(std::function& func){ +AsyncTask ThreadPool_Default::try_dispatch_now(MoveOnlyFunction& func){ AsyncTask task; { std::lock_guard lg(m_lock); diff --git a/Common/Cpp/Concurrency/Backends/ThreadPool_Default.h b/Common/Cpp/Concurrency/Backends/ThreadPool_Default.h index b2944c630f..d60a7378bb 100644 --- a/Common/Cpp/Concurrency/Backends/ThreadPool_Default.h +++ b/Common/Cpp/Concurrency/Backends/ThreadPool_Default.h @@ -49,9 +49,9 @@ class ThreadPool_Default final : public ThreadPool{ public: - [[nodiscard]] virtual AsyncTask dispatch(std::function&& func) override; - [[nodiscard]] virtual AsyncTask dispatch_now_blocking(std::function&& func) override; - [[nodiscard]] virtual AsyncTask try_dispatch_now(std::function& func) override; + [[nodiscard]] virtual AsyncTask dispatch(MoveOnlyFunction&& func) override; + [[nodiscard]] virtual AsyncTask dispatch_now_blocking(MoveOnlyFunction&& func) override; + [[nodiscard]] virtual AsyncTask try_dispatch_now(MoveOnlyFunction& func) override; virtual void run_in_parallel( const std::function& func, diff --git a/Common/Cpp/Concurrency/ThreadPool.h b/Common/Cpp/Concurrency/ThreadPool.h index 29ea2cf01c..6a47ab3337 100644 --- a/Common/Cpp/Concurrency/ThreadPool.h +++ b/Common/Cpp/Concurrency/ThreadPool.h @@ -17,6 +17,7 @@ #define PokemonAutomation_ThreadPool_H #include +#include "Common/Cpp/MoveOnlyFunction.h" #include "Common/Cpp/Time.h" namespace PokemonAutomation{ @@ -40,17 +41,17 @@ class ThreadPool{ // Dispatch the function and return immediately. // The function is not guaranteed to begin running immediately. - [[nodiscard]] virtual AsyncTask dispatch(std::function&& func) = 0; + [[nodiscard]] virtual AsyncTask dispatch(MoveOnlyFunction&& func) = 0; // Dispatch the function and begin running now. // If there are no threads available, it waits until there are. // This function will not return until the function has begin running. - [[nodiscard]] virtual AsyncTask dispatch_now_blocking(std::function&& func) = 0; + [[nodiscard]] virtual AsyncTask dispatch_now_blocking(MoveOnlyFunction&& func) = 0; // Try to dispatch the function and begin running now. // Returns null if no threads are available. "func" will be moved-from only on success. // This function returns immediately. - [[nodiscard]] virtual AsyncTask try_dispatch_now(std::function& func) = 0; + [[nodiscard]] virtual AsyncTask try_dispatch_now(MoveOnlyFunction& func) = 0; // Run function for all the indices [start, end). // Lower indices are not allowed to block on higher indices. diff --git a/SerialPrograms/Source/CommonFramework/VideoPipeline/Backends/SnapshotManager.cpp b/SerialPrograms/Source/CommonFramework/VideoPipeline/Backends/SnapshotManager.cpp index 068d6ec418..1f5369a0ab 100644 --- a/SerialPrograms/Source/CommonFramework/VideoPipeline/Backends/SnapshotManager.cpp +++ b/SerialPrograms/Source/CommonFramework/VideoPipeline/Backends/SnapshotManager.cpp @@ -5,6 +5,7 @@ */ //#include "Common/Cpp/Concurrency/ReverseLockGuard.h" +#include "Common/Cpp/MoveOnlyFunction.h" #include "Common/Cpp/Concurrency/AsyncTask.h" #include "CommonFramework/ImageTypes/ImageRGB32_Qt.h" #include "CommonFramework/Tools/GlobalThreadPools.h" @@ -100,7 +101,7 @@ bool SnapshotManager::try_dispatch_conversion(uint64_t seqnum, QVideoFrame frame } try{ - std::function lambda = [=, this, frame = std::move(frame)](){ + MoveOnlyFunction lambda = [=, this, frame = std::move(frame)](){ convert(seqnum, std::move(frame), timestamp); }; diff --git a/SerialPrograms/Source/VideoGames/PokemonSwSh/Inference/ShinyDetection/PokemonSwSh_ShinySparkleSet.cpp b/SerialPrograms/Source/VideoGames/PokemonSwSh/Inference/ShinyDetection/PokemonSwSh_ShinySparkleSet.cpp index ed375c9948..c3b2ae25f9 100644 --- a/SerialPrograms/Source/VideoGames/PokemonSwSh/Inference/ShinyDetection/PokemonSwSh_ShinySparkleSet.cpp +++ b/SerialPrograms/Source/VideoGames/PokemonSwSh/Inference/ShinyDetection/PokemonSwSh_ShinySparkleSet.cpp @@ -6,6 +6,7 @@ #include #include +#include "Common/Cpp/MoveOnlyFunction.h" #include "Common/Cpp/Concurrency/SpinLock.h" #include "Common/Cpp/Concurrency/AsyncTask.h" #include "Kernels/Waterfill/Kernels_Waterfill_Session.h" @@ -134,7 +135,7 @@ ShinySparkleSetSwSh find_sparkles(size_t screen_area, WaterfillSession& session) auto finder = session.make_iterator(20); WaterfillObject object; while (finder->find_next(object, true)){ - std::function func = [&, screen_area, object = std::move(object)]{ + MoveOnlyFunction func = [&, screen_area, object = std::move(object)]{ find_sparkles( screen_area, lock,