address another helping of clang-tidy reports

bemanproject · Dec 21, 2024 · 4c701ed · 4c701ed
1 parent 38e376c
commit 4c701ed
Show file tree

Hide file tree

Showing 16 changed files with 75 additions and 43 deletions.
diff --git a/Makefile b/Makefile
@@ -14,7 +14,7 @@ SANITIZERS := run
 #     SANITIZERS += asan # TODO: tsan msan
 # endif
 
-.PHONY: default release debug doc run update check ce todo distclean clean codespell clang-tidy build test install all format $(SANITIZERS)
+.PHONY: default release debug doc run update check ce todo distclean clean codespell clang-tidy build test install all format unstage $(SANITIZERS)
 
 SYSROOT   ?=
 TOOLCHAIN ?=
@@ -79,6 +79,7 @@ doc:
 
 build:
 	CC=$(CXX) cmake --fresh -G Ninja -S $(SOURCEDIR) -B  $(BUILD) $(TOOLCHAIN) $(SYSROOT) \
+	  -DCMAKE_EXPORT_COMPILE_COMMANDS=1 \
       -DCMAKE_CXX_COMPILER=$(CXX) # XXX -DCMAKE_CXX_FLAGS="$(CXX_FLAGS) $(SAN_FLAGS)"
 	cmake --build $(BUILD)
 
@@ -111,8 +112,8 @@ check:
 	done | tsort > /dev/null
 
 build/$(SANITIZER)/compile_commands.json: $(SANITIZER)
-clang-tidy: build/$(SANITIZER)/compile_commands.json
-	run-clang-tidy -p build/$(SANITIZER) tests examples
+clang-tidy: $(BUILD)/compile_commands.json
+	run-clang-tidy -p $(BUILD) tests examples
 
 codespell:
 	codespell -L statics,snd,copyable,cancelled
@@ -128,6 +129,9 @@ clang-format:
 todo:
 	bin/mk-todo.py
 
+unstage:
+	git restore --staged tests/beman/execution26/CMakeLists.txt
+
 .PHONY: clean-doc
 clean-doc:
 	$(RM) -r docs/html docs/latex

diff --git a/examples/doc-just_error.cpp b/examples/doc-just_error.cpp
@@ -13,5 +13,6 @@ int main() {
                                     assert(ec.value() == 17);
                                     had_error = true;
                                 }));
+    assert(result);
     assert(had_error);
 }
diff --git a/examples/doc-just_stopped.cpp b/examples/doc-just_stopped.cpp
@@ -11,5 +11,6 @@ int main() {
     bool stopped{false};
 
     auto result = ex::sync_wait(ex::just_stopped() | ex::upon_stopped([&] { stopped = true; }));
+    assert(result);
     assert(stopped);
 }
diff --git a/examples/sender-demo.cpp b/examples/sender-demo.cpp
@@ -46,18 +46,22 @@ static_assert(ex::sender<just_sender<std::pmr::string>>);
 static_assert(ex::sender_in<just_sender<std::pmr::string>>);
 
 int main() {
-    auto j = just_sender{std::pmr::string("value")};
-    auto t = std::move(j) | ex::then([](const std::pmr::string& v) { return v + " then"; });
-    auto w = ex::when_all(std::move(t));
-    auto e = ex::detail::write_env(std::move(w),
-                                   ex::detail::make_env(ex::get_allocator, std::pmr::polymorphic_allocator<>()));
-
-    std::cout << "before start\n";
-    auto r = ex::sync_wait(std::move(e));
-    if (r) {
-        auto [v] = *r;
-        std::cout << "produced='" << v << "'\n";
-    } else
-        std::cout << "operation was cancelled\n";
-    std::cout << "after start\n";
+    try {
+        auto j = just_sender{std::pmr::string("value")};
+        auto t = std::move(j) | ex::then([](const std::pmr::string& v) { return v + " then"; });
+        auto w = ex::when_all(std::move(t));
+        auto e = ex::detail::write_env(std::move(w),
+                                       ex::detail::make_env(ex::get_allocator, std::pmr::polymorphic_allocator<>()));
+
+        std::cout << "before start\n";
+        auto r = ex::sync_wait(std::move(e));
+        if (r) {
+            auto [v] = *r;
+            std::cout << "produced='" << v << "'\n";
+        } else
+            std::cout << "operation was cancelled\n";
+        std::cout << "after start\n";
+    } catch (const std::exception& ex) {
+        std::cout << "ERROR: " << ex.what() << "\n";
+    }
 }
diff --git a/examples/stop_token.cpp b/examples/stop_token.cpp
@@ -4,6 +4,7 @@
 #include <beman/execution26/stop_token.hpp>
 #include <condition_variable>
 #include <iostream>
+#include <exception>
 #include <latch>
 #include <mutex>
 #include <thread>
@@ -92,15 +93,20 @@ auto inactive(Token token) -> void {
 } // namespace
 
 auto main() -> int {
-    exec::stop_source source;
-    ::std::thread     act([token = source.get_token()] { active(token); });
-    ::std::thread     inact([token = source.get_token()] { inactive(token); });
+    try {
 
-    print("threads started\n");
-    source.request_stop();
-    print("threads cancelled\n");
+        exec::stop_source source;
+        ::std::thread     act([token = source.get_token()] { active(token); });
+        ::std::thread     inact([token = source.get_token()] { inactive(token); });
 
-    act.join();
-    inact.join();
-    print("done\n");
+        print("threads started\n");
+        source.request_stop();
+        print("threads cancelled\n");
+
+        act.join();
+        inact.join();
+        print("done\n");
+    } catch (const std::exception& ex) {
+        std::cout << "ERROR: " << ex.what() << "\n";
+    }
 }
diff --git a/examples/when_all-cancel.cpp b/examples/when_all-cancel.cpp
@@ -7,6 +7,7 @@
 #include <optional>
 #include <type_traits>
 #include <utility>
+#include <cassert>
 
 namespace ex = beman::execution26;
 
@@ -106,11 +107,15 @@ struct eager {
         std::optional<helper> inner_state;
 
         template <typename R, typename S>
-        state(R&& r, S&& s) : outer_receiver(std::forward<R>(r)), inner_state() {
-            inner_state.emplace(std::forward<S>(s), receiver{this});
+        state(R&& r, S&& s)
+            : outer_receiver(std::forward<R>(r)), inner_state(std::in_place, std::forward<S>(s), receiver{this}) {}
+        auto start() & noexcept -> void {
+            if (this->inner_state) {
+                ex::start((*this->inner_state).st);
+            } else {
+                assert(this->inner_state);
+            }
         }
-        // TODO on next line: bugprone-unchecked-optional-access
-        auto start() & noexcept -> void { ex::start((*this->inner_state).st); }
     };
     template <ex::receiver Receiver>
     auto connect(Receiver&& receiver) {

diff --git a/include/beman/execution26/detail/as_awaitable.hpp b/include/beman/execution26/detail/as_awaitable.hpp
@@ -4,7 +4,6 @@
 #ifndef INCLUDED_BEMAN_EXECUTION26_DETAIL_AS_AWAITABLE
 #define INCLUDED_BEMAN_EXECUTION26_DETAIL_AS_AWAITABLE
 
-#include <beman/execution26/detail/as_awaitable.hpp>
 #include <beman/execution26/detail/awaitable_sender.hpp>
 #include <beman/execution26/detail/is_awaitable.hpp>
 #include <beman/execution26/detail/sender_awaitable.hpp>
@@ -31,12 +30,11 @@ struct as_awaitable_t {
                 "as_awaitable must return an awaitable");
             return ::std::forward<Expr>(expr).as_awaitable(promise);
         } else if constexpr (::beman::execution26::detail::
-                                 is_awaitable<Expr, ::beman::execution26::detail::unspecified_promise>) {
+                                 is_awaitable<Expr, ::beman::execution26::detail::unspecified_promise> ||
+                             not::beman::execution26::detail::awaitable_sender<Expr, Promise>) {
             return ::std::forward<Expr>(expr);
-        } else if constexpr (::beman::execution26::detail::awaitable_sender<Expr, Promise>) {
-            return ::beman::execution26::detail::sender_awaitable<Expr, Promise>{::std::forward<Expr>(expr), promise};
         } else {
-            return ::std::forward<Expr>(expr);
+            return ::beman::execution26::detail::sender_awaitable<Expr, Promise>{::std::forward<Expr>(expr), promise};
         }
     }
 };

diff --git a/include/beman/execution26/detail/inplace_stop_source.hpp b/include/beman/execution26/detail/inplace_stop_source.hpp
@@ -86,7 +86,7 @@ class beman::execution26::inplace_stop_callback final
     inplace_stop_callback(::beman::execution26::inplace_stop_token, Init&&);
     inplace_stop_callback(const inplace_stop_callback&) = delete;
     inplace_stop_callback(inplace_stop_callback&&)      = delete;
-    ~inplace_stop_callback() {
+    ~inplace_stop_callback() override {
         if (this->source) {
             this->source->deregister(this);
         }

diff --git a/include/beman/execution26/detail/run_loop.hpp b/include/beman/execution26/detail/run_loop.hpp
@@ -39,7 +39,7 @@ class run_loop {
     };
 
     struct opstate_base : ::beman::execution26::detail::virtual_immovable {
-        virtual ~opstate_base() = default;
+        // virtual ~opstate_base() override = default;
         opstate_base* next{};
         virtual auto  execute() noexcept -> void = 0;
     };

diff --git a/include/beman/execution26/detail/sender_awaitable.hpp b/include/beman/execution26/detail/sender_awaitable.hpp
@@ -4,7 +4,6 @@
 #ifndef INCLUDED_BEMAN_EXECUTION26_DETAIL_SENDER_AWAITABLE
 #define INCLUDED_BEMAN_EXECUTION26_DETAIL_SENDER_AWAITABLE
 
-#include <beman/execution26/detail/as_awaitable.hpp>
 #include <beman/execution26/detail/as_except_ptr.hpp>
 #include <beman/execution26/detail/connect_result_t.hpp>
 #include <beman/execution26/detail/connect.hpp>

diff --git a/include/beman/execution26/detail/with_awaitable_senders.hpp b/include/beman/execution26/detail/with_awaitable_senders.hpp
@@ -37,6 +37,9 @@ struct with_awaitable_senders {
     }
 
   private:
+    friend Promise;
+    with_awaitable_senders() = default;
+
     [[noreturn]] static auto default_unhandled_stopped(void*) noexcept -> ::std::coroutine_handle<> {
         ::std::terminate();
     }

diff --git a/tests/beman/execution26/exec-general.test.cpp b/tests/beman/execution26/exec-general.test.cpp
@@ -10,8 +10,9 @@
 // ----------------------------------------------------------------------------
 
 namespace {
-struct error {
+struct error : std::exception {
     int value;
+    explicit error(int v) : value(v) {}
 };
 
 struct non_movable {

diff --git a/tests/beman/execution26/exec-split.test.cpp b/tests/beman/execution26/exec-split.test.cpp
@@ -11,6 +11,8 @@
 
 // ----------------------------------------------------------------------------
 
+namespace {
+
 struct timed_scheduler_t : beman::execution26::scheduler_t {};
 
 class some_thread_pool {
@@ -198,6 +200,7 @@ void test_multiple_completions_from_other_threads() {
         ASSERT(val2 == 42);
     }
 }
+} // namespace
 
 TEST(exec_split) {
     test_destroy_unused_split();

diff --git a/tests/beman/execution26/exec-sync-wait.test.cpp b/tests/beman/execution26/exec-sync-wait.test.cpp
@@ -29,8 +29,9 @@ struct arg {
     int  value{};
     auto operator==(const arg&) const -> bool = default;
 };
-struct error {
+struct error : std::exception {
     int value{};
+    explicit error(int v) : value(v) {}
 };
 struct sender {
     using sender_concept = test_std::sender_t;

diff --git a/tests/beman/execution26/exec-with-awaitable-senders.test.cpp b/tests/beman/execution26/exec-with-awaitable-senders.test.cpp
@@ -14,6 +14,7 @@
 
 namespace exec = beman::execution26;
 
+namespace {
 struct promise;
 
 struct awaitable {
@@ -50,18 +51,18 @@ coroutine test_await_void() { co_await exec::just(); }
 
 void test_sync_wait_awaitable() {
     try {
-        auto [v] = exec::sync_wait(awaitable{}).value();
+        auto [v] = exec::sync_wait(awaitable{}).value_or(std::tuple(0));
         ASSERT(v == 1);
     } catch (...) {
-        ASSERT(false);
+        ASSERT_UNREACHABLE();
     }
 }
 
 void test_sync_wait_void_awaitable() {
     try {
         ASSERT(exec::sync_wait(void_awaitable{}));
     } catch (...) {
-        ASSERT(false);
+        ASSERT_UNREACHABLE();
     }
 }
 
@@ -71,6 +72,8 @@ coroutine test_mix_awaitable_and_sender() {
     ASSERT(value == 1);
 }
 
+} // namespace
+
 TEST(exec_with_awaitable_senders) {
     test_await_tuple().resume();
     test_await_void().resume();

diff --git a/tests/beman/execution26/include/test/execution.hpp b/tests/beman/execution26/include/test/execution.hpp
@@ -12,6 +12,7 @@
 #include <cassert>
 
 #define ASSERT(condition) assert(condition)
+#define ASSERT_UNREACHABLE() assert(::test::unreachable_helper())
 #define TEST(name) auto main() -> int
 
 namespace beman::execution26 {}
@@ -20,6 +21,8 @@ namespace test_std    = ::beman::execution26;
 namespace test_detail = ::beman::execution26::detail;
 
 namespace test {
+inline bool unreachable_helper() { return false; }
+
 template <typename>
 auto type_exists() {}
 template <typename T0, typename T1>