From e66dffaacce8049d1dc4634c26dfd1ed426a243d Mon Sep 17 00:00:00 2001 From: FrogTheFrog Date: Sat, 26 Oct 2024 00:11:06 +0300 Subject: [PATCH 1/4] feat: add const version of RetryScheduler::execute --- .../include/display_device/retry_scheduler.h | 34 +++++++++ tests/unit/general/test_retry_scheduler.cpp | 76 ++++++++++++++++++- 2 files changed, 109 insertions(+), 1 deletion(-) diff --git a/src/common/include/display_device/retry_scheduler.h b/src/common/include/display_device/retry_scheduler.h index 082837a..087d56c 100644 --- a/src/common/include/display_device/retry_scheduler.h +++ b/src/common/include/display_device/retry_scheduler.h @@ -72,6 +72,15 @@ namespace display_device { exec_fn(std::declval()); }; + /** + * @brief Check if the function signature matches the acceptable signature for RetryScheduler::execute (const) + * without a stop token. + */ + template + concept ExecuteWithoutStopTokenConst = requires(FunctionT exec_fn) { + exec_fn(std::declval()); + }; + /** * @brief Check if the function signature matches the acceptable signature for RetryScheduler::execute * with a stop token. @@ -81,11 +90,26 @@ namespace display_device { exec_fn(std::declval(), std::declval()); }; + /** + * @brief Check if the function signature matches the acceptable signature for RetryScheduler::execute (const) + * with a stop token. + */ + template + concept ExecuteWithStopTokenConst = requires(FunctionT exec_fn) { + exec_fn(std::declval(), std::declval()); + }; + /** * @brief Check if the function signature matches the acceptable signature for RetryScheduler::execute. */ template concept ExecuteCallbackLike = ExecuteWithoutStopToken || ExecuteWithStopToken; + + /** + * @brief Check if the function signature matches the acceptable signature for RetryScheduler::execute (const). + */ + template + concept ExecuteCallbackLikeConst = ExecuteWithoutStopTokenConst || ExecuteWithStopTokenConst; } // namespace detail /** @@ -280,6 +304,16 @@ namespace display_device { } } + /** + * @brief A const variant of the `execute` method. See non-const variant for details. + */ + template + requires detail::ExecuteCallbackLikeConst + auto + execute(FunctionT exec_fn) const { + return const_cast(this)->execute(std::forward(exec_fn)); + } + /** * @brief Check whether anything is scheduled for execution. * @return True if something is scheduled, false otherwise. diff --git a/tests/unit/general/test_retry_scheduler.cpp b/tests/unit/general/test_retry_scheduler.cpp index 2908011..26b3987 100644 --- a/tests/unit/general/test_retry_scheduler.cpp +++ b/tests/unit/general/test_retry_scheduler.cpp @@ -16,6 +16,26 @@ namespace { std::vector m_durations; }; + template + constexpr bool + isValidNonConstCallback(FunctionT) { + using namespace display_device::detail; + if constexpr (ExecuteCallbackLike) { + return true; + } + return false; + } + + template + constexpr bool + isValidConstCallback(FunctionT) { + using namespace display_device::detail; + if constexpr (ExecuteCallbackLikeConst) { + return true; + } + return false; + } + // Test fixture(s) for this file class RetrySchedulerTest: public BaseTest { public: @@ -315,7 +335,13 @@ TEST_F_S(Schedule, ExceptionThrown, DuringScheduledCall) { m_impl.stop(); } -TEST_F_S(Execute, NullptrCallbackProvided) { +TEST_F_S(Execute, NonConst, NullptrCallbackProvided) { + EXPECT_THAT([&]() { m_impl.execute(std::function {}); }, + ThrowsMessage(HasSubstr("Empty callback function provided in RetryScheduler::execute!"))); +} + +TEST_F_S(Execute, Const, NullptrCallbackProvided) { + // Note: this test verifies that non-const method is invoked from const method. EXPECT_THAT([&]() { m_impl.execute(std::function {}); }, ThrowsMessage(HasSubstr("Empty callback function provided in RetryScheduler::execute!"))); } @@ -435,6 +461,54 @@ TEST_F_S(Execute, ExceptionThrown, AfterStopToken) { EXPECT_FALSE(m_impl.isScheduled()); } +TEST_F_S(Execute, ConstVsNonConst, WithoutStopToken) { + const auto const_callback = [](const TestIface &) {}; + const auto non_const_callback = [](TestIface &) {}; + + // Verify that concepts are working as expected + static_assert(isValidNonConstCallback(const_callback), "Invalid non-const callback"); + static_assert(isValidNonConstCallback(non_const_callback), "Invalid non-const callback"); + static_assert(isValidConstCallback(const_callback), "Invalid const callback"); + static_assert(!isValidConstCallback(non_const_callback), "Invalid const callback"); + + // Verify it compiles with non-const + auto &non_const_impl { m_impl }; + non_const_impl.execute(const_callback); + non_const_impl.execute(non_const_callback); + + // Verify it compiles with const + const auto &const_impl { m_impl }; + const_impl.execute(const_callback); +} + +TEST_F_S(Execute, ConstVsNonConst, WithStopToken) { + const auto const_const_callback = [](const TestIface &, const display_device::SchedulerStopToken &) {}; + const auto const_non_const_callback = [](const TestIface &, display_device::SchedulerStopToken &) {}; + const auto non_const_const_callback = [](TestIface &, const display_device::SchedulerStopToken &) {}; + const auto non_const_non_const_callback = [](TestIface &, display_device::SchedulerStopToken &) {}; + + // Verify that concepts are working as expected + static_assert(isValidNonConstCallback(const_const_callback), "Invalid non-const callback"); + static_assert(isValidNonConstCallback(const_non_const_callback), "Invalid non-const callback"); + static_assert(isValidNonConstCallback(non_const_const_callback), "Invalid non-const callback"); + static_assert(isValidNonConstCallback(non_const_non_const_callback), "Invalid non-const callback"); + static_assert(isValidConstCallback(const_const_callback), "Invalid const callback"); + static_assert(!isValidConstCallback(const_non_const_callback), "Invalid const callback"); + static_assert(!isValidConstCallback(non_const_const_callback), "Invalid const callback"); + static_assert(!isValidConstCallback(non_const_non_const_callback), "Invalid const callback"); + + // Verify it compiles with non-const + auto &non_const_impl { m_impl }; + non_const_impl.execute(const_const_callback); + non_const_impl.execute(const_non_const_callback); + non_const_impl.execute(non_const_const_callback); + non_const_impl.execute(non_const_non_const_callback); + + // Verify it compiles with const + const auto &const_impl { m_impl }; + const_impl.execute(const_const_callback); +} + TEST_F_S(Stop) { EXPECT_FALSE(m_impl.isScheduled()); m_impl.stop(); From 8d90bc30fab25ea9a5c1ad04152841e799a6a830 Mon Sep 17 00:00:00 2001 From: FrogTheFrog Date: Sat, 26 Oct 2024 00:19:09 +0300 Subject: [PATCH 2/4] UGHHHHHH --- tests/unit/general/test_retry_scheduler.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/general/test_retry_scheduler.cpp b/tests/unit/general/test_retry_scheduler.cpp index 26b3987..77bf469 100644 --- a/tests/unit/general/test_retry_scheduler.cpp +++ b/tests/unit/general/test_retry_scheduler.cpp @@ -336,13 +336,13 @@ TEST_F_S(Schedule, ExceptionThrown, DuringScheduledCall) { } TEST_F_S(Execute, NonConst, NullptrCallbackProvided) { - EXPECT_THAT([&]() { m_impl.execute(std::function {}); }, + EXPECT_THAT([this]() { auto &non_const_impl { m_impl }; non_const_impl.execute(std::function {}); }, ThrowsMessage(HasSubstr("Empty callback function provided in RetryScheduler::execute!"))); } TEST_F_S(Execute, Const, NullptrCallbackProvided) { // Note: this test verifies that non-const method is invoked from const method. - EXPECT_THAT([&]() { m_impl.execute(std::function {}); }, + EXPECT_THAT([this]() { auto &const_impl { m_impl }; const_impl.execute(std::function {}); }, ThrowsMessage(HasSubstr("Empty callback function provided in RetryScheduler::execute!"))); } From c13630dbae533ffc86d4373a266966217564b459 Mon Sep 17 00:00:00 2001 From: FrogTheFrog Date: Sat, 26 Oct 2024 00:35:36 +0300 Subject: [PATCH 3/4] third time's the charm --- src/common/include/display_device/retry_scheduler.h | 2 +- tests/unit/general/test_retry_scheduler.cpp | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/common/include/display_device/retry_scheduler.h b/src/common/include/display_device/retry_scheduler.h index 087d56c..a8c1461 100644 --- a/src/common/include/display_device/retry_scheduler.h +++ b/src/common/include/display_device/retry_scheduler.h @@ -311,7 +311,7 @@ namespace display_device { requires detail::ExecuteCallbackLikeConst auto execute(FunctionT exec_fn) const { - return const_cast(this)->execute(std::forward(exec_fn)); + return const_cast(this)->execute(exec_fn); } /** diff --git a/tests/unit/general/test_retry_scheduler.cpp b/tests/unit/general/test_retry_scheduler.cpp index 77bf469..4840b2b 100644 --- a/tests/unit/general/test_retry_scheduler.cpp +++ b/tests/unit/general/test_retry_scheduler.cpp @@ -462,8 +462,8 @@ TEST_F_S(Execute, ExceptionThrown, AfterStopToken) { } TEST_F_S(Execute, ConstVsNonConst, WithoutStopToken) { - const auto const_callback = [](const TestIface &) {}; - const auto non_const_callback = [](TestIface &) {}; + const auto const_callback = [](const TestIface &) { /* noop */ }; + const auto non_const_callback = [](TestIface &) { /* noop */ }; // Verify that concepts are working as expected static_assert(isValidNonConstCallback(const_callback), "Invalid non-const callback"); @@ -482,10 +482,10 @@ TEST_F_S(Execute, ConstVsNonConst, WithoutStopToken) { } TEST_F_S(Execute, ConstVsNonConst, WithStopToken) { - const auto const_const_callback = [](const TestIface &, const display_device::SchedulerStopToken &) {}; - const auto const_non_const_callback = [](const TestIface &, display_device::SchedulerStopToken &) {}; - const auto non_const_const_callback = [](TestIface &, const display_device::SchedulerStopToken &) {}; - const auto non_const_non_const_callback = [](TestIface &, display_device::SchedulerStopToken &) {}; + const auto const_const_callback = [](const TestIface &, const display_device::SchedulerStopToken &) { /* noop */ }; + const auto const_non_const_callback = [](const TestIface &, display_device::SchedulerStopToken &) { /* noop */ }; + const auto non_const_const_callback = [](TestIface &, const display_device::SchedulerStopToken &) { /* noop */ }; + const auto non_const_non_const_callback = [](TestIface &, display_device::SchedulerStopToken &) { /* noop */ }; // Verify that concepts are working as expected static_assert(isValidNonConstCallback(const_const_callback), "Invalid non-const callback"); From 8cde9bd2b68723812ebd16b7b317d5c11be77787 Mon Sep 17 00:00:00 2001 From: FrogTheFrog Date: Sat, 26 Oct 2024 00:43:33 +0300 Subject: [PATCH 4/4] cmon --- src/common/include/display_device/retry_scheduler.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/include/display_device/retry_scheduler.h b/src/common/include/display_device/retry_scheduler.h index a8c1461..aecab81 100644 --- a/src/common/include/display_device/retry_scheduler.h +++ b/src/common/include/display_device/retry_scheduler.h @@ -311,7 +311,7 @@ namespace display_device { requires detail::ExecuteCallbackLikeConst auto execute(FunctionT exec_fn) const { - return const_cast(this)->execute(exec_fn); + return const_cast(this)->execute(std::move(exec_fn)); } /**