Fix clang tidy warnings (#105)

- fixed various clang-tidy warnings - fixed various gcc warnings - implemented missing noexcept specifications in some places to fix clang-tidy warnings
bemanproject · Dec 22, 2024 · 5b44c98 · 5b44c98
1 parent 38e376c
commit 5b44c98
Show file tree

Hide file tree

Showing 39 changed files with 367 additions and 252 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/allocator.cpp b/examples/allocator.cpp
@@ -12,7 +12,7 @@ namespace {
 template <std::size_t Size>
 struct inline_resource : std::pmr::memory_resource {
     const char* name;
-    explicit inline_resource(const char* name) : name(name) {}
+    explicit inline_resource(const char* n) : name(n) {}
     std::byte  buffer[Size]{};      // NOLINT(hicpp-avoid-c-arrays)
     std::byte* next{+this->buffer}; // NOLINT(hicpp-no-array-decay)
 
@@ -39,13 +39,13 @@ struct allocator_aware_fun {
 
     template <typename F>
         requires std::same_as<std::remove_cvref_t<F>, std::remove_cvref_t<Fun>>
-    explicit allocator_aware_fun(F&& fun) : fun(std::forward<F>(fun)) {}
-    allocator_aware_fun(const allocator_aware_fun& other, allocator_type allocator = {})
-        : fun(other.fun), allocator(allocator) {}
+    explicit allocator_aware_fun(F&& f) : fun(std::forward<F>(f)) {}
+    allocator_aware_fun(const allocator_aware_fun& other, allocator_type alloc = {})
+        : fun(other.fun), allocator(alloc) {}
     allocator_aware_fun(allocator_aware_fun&& other) noexcept
         : fun(std::move(other.fun)), allocator(other.allocator) {}
-    allocator_aware_fun(allocator_aware_fun&& other, allocator_type allocator)
-        : fun(std::move(other.fun)), allocator(allocator) {}
+    allocator_aware_fun(allocator_aware_fun&& other, allocator_type alloc)
+        : fun(std::move(other.fun)), allocator(alloc) {}
 
     template <typename... Args>
     auto operator()(Args&&... args) noexcept {

diff --git a/examples/doc-just_error.cpp b/examples/doc-just_error.cpp
@@ -6,12 +6,19 @@
 #include <cassert>
 namespace ex = beman::execution26;
 
+namespace {
+void use(auto&&...) {}
+} // namespace
+
 int main() {
     bool had_error{false};
     auto result = ex::sync_wait(ex::just_error(std::error_code(17, std::system_category())) |
                                 ex::upon_error([&](std::error_code ec) {
+                                    use(ec);
                                     assert(ec.value() == 17);
                                     had_error = true;
                                 }));
+    use(result, had_error);
+    assert(result);
     assert(had_error);
 }
diff --git a/examples/doc-just_stopped.cpp b/examples/doc-just_stopped.cpp
@@ -7,9 +7,15 @@
 #include <iostream> //-dk:TODO remove
 namespace ex = beman::execution26;
 
+namespace {
+void use(auto&&...) {}
+} // namespace
+
 int main() {
     bool stopped{false};
 
     auto result = ex::sync_wait(ex::just_stopped() | ex::upon_stopped([&] { stopped = true; }));
+    use(result, stopped);
+    assert(result);
     assert(stopped);
 }
diff --git a/examples/sender-demo.cpp b/examples/sender-demo.cpp
@@ -14,8 +14,8 @@ struct just_op_state {
     std::pmr::string              value;
 
     template <typename R>
-    just_op_state(R&& r, std::pmr::string&& value)
-        : rec(std::forward<R>(r)), value(std::move(value), ex::get_allocator(ex::get_env(rec))) {}
+    just_op_state(R&& r, std::pmr::string&& val)
+        : rec(std::forward<R>(r)), value(std::move(val), ex::get_allocator(ex::get_env(rec))) {}
 
     void start() & noexcept { ex::set_value(std::move(rec), std::move(value)); }
 };
@@ -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/stopping.cpp b/examples/stopping.cpp
@@ -22,7 +22,7 @@ namespace {
 struct env {
     ex::inplace_stop_token token;
 
-    env(ex::inplace_stop_token token) : token(token) {} // NOLINT(hicpp-explicit-conversions)
+    explicit env(ex::inplace_stop_token t) : token(t) {} // NOLINT(hicpp-explicit-conversions)
 
     auto query(const ex::get_stop_token_t&) const noexcept { return this->token; }
 };
@@ -38,7 +38,7 @@ struct inject_cancel_sender {
         std::remove_cvref_t<Receiver> inner_receiver;
         ex::inplace_stop_token        token{};
 
-        auto get_env() const noexcept -> env { return {this->token}; }
+        auto get_env() const noexcept -> env { return env(this->token); }
 
         template <typename... T>
         auto set_value(T&&... t) noexcept -> void {

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/basic_operation.hpp b/include/beman/execution26/detail/basic_operation.hpp
@@ -35,7 +35,12 @@ template <typename Sender, typename Receiver>
     using inner_ops_t = ::beman::execution26::detail::connect_all_result<Sender, Receiver>;
     inner_ops_t inner_ops;
 
-    basic_operation(Sender&& sender, Receiver&& receiver) noexcept(true /*-dk:TODO*/)
+    basic_operation(Sender&& sender, Receiver&& receiver) noexcept(
+        noexcept(::beman::execution26::detail::basic_state<Sender, Receiver>(::std::forward<Sender>(sender),
+                                                                             ::std::move(receiver))) &&
+        noexcept(::beman::execution26::detail::connect_all(this,
+                                                           ::std::forward<Sender>(sender),
+                                                           ::beman::execution26::detail::indices_for<Sender>())))
         : ::beman::execution26::detail::basic_state<Sender, Receiver>(::std::forward<Sender>(sender),
                                                                       ::std::move(receiver)),
           // NOLINTBEGIN(bugprone-use-after-move,hicpp-invalid-access-moved)

diff --git a/include/beman/execution26/detail/basic_sender.hpp b/include/beman/execution26/detail/basic_sender.hpp
@@ -44,25 +44,32 @@ struct basic_sender : ::beman::execution26::detail::product_type<Tag, Data, Chil
     auto connect(Receiver receiver) = BEMAN_EXECUTION26_DELETE("the passed receiver doesn't model receiver");
 
   private:
-#if __cpp_explicit_this_parameter < 202110L
+#if __cpp_explicit_this_parameter < 302110L //-dk:TODO need to figure out how to use explicit this with forwarding
     template <::beman::execution26::receiver Receiver>
-    auto connect(Receiver receiver) & noexcept(true /*-dk:TODO*/)
+    auto connect(Receiver receiver) & noexcept(
+        noexcept(::beman::execution26::detail::basic_operation<basic_sender&, Receiver>{*this, ::std::move(receiver)}))
         -> ::beman::execution26::detail::basic_operation<basic_sender&, Receiver> {
         return {*this, ::std::move(receiver)};
     }
     template <::beman::execution26::receiver Receiver>
-    auto connect(Receiver receiver) const& noexcept(true /*-dk:TODO*/)
+    auto connect(Receiver receiver) const& noexcept(noexcept(
+        ::beman::execution26::detail::basic_operation<const basic_sender&, Receiver>{*this, ::std::move(receiver)}))
         -> ::beman::execution26::detail::basic_operation<const basic_sender&, Receiver> {
         return {*this, ::std::move(receiver)};
     }
     template <::beman::execution26::receiver Receiver>
-    auto connect(Receiver receiver) && noexcept(true /*-dk:TODO*/)
+    auto connect(Receiver receiver) && noexcept(
+        noexcept(::beman::execution26::detail::basic_operation<basic_sender, Receiver>{::std::move(*this),
+                                                                                       ::std::move(receiver)}))
         -> ::beman::execution26::detail::basic_operation<basic_sender, Receiver> {
         return {::std::move(*this), ::std::move(receiver)};
     }
 #else
     template <::beman::execution26::detail::decays_to<basic_sender> Self, ::beman::execution26::receiver Receiver>
-    auto connect(this Self&& self, Receiver receiver) noexcept(true /*-dk:TODO*/)
+    auto
+    connect(this Self&& self,
+            Receiver receiver) noexcept(noexcept(::beman::execution26::detail::basic_operation<basic_sender, Receiver>{
+        ::std::forward<Self>(self), ::std::move(receiver)}))
         -> ::beman::execution26::detail::basic_operation<Self, Receiver> {
         return {::std::forward<Self>(self), ::std::move(receiver)};
     }

diff --git a/include/beman/execution26/detail/basic_state.hpp b/include/beman/execution26/detail/basic_state.hpp
@@ -20,8 +20,8 @@ namespace beman::execution26::detail {
  */
 template <typename Sender, typename Receiver>
 struct basic_state {
-    basic_state(Sender&& sender, Receiver&& receiver) noexcept(true)
-        : receiver(::std::move(receiver)),
+    basic_state(Sender&& sender, Receiver&& rcvr) noexcept(true)
+        : receiver(::std::move(rcvr)),
           state(::beman::execution26::detail::impls_for< ::beman::execution26::tag_of_t<Sender> >::get_state(
               ::std::forward<Sender>(sender), this->receiver)) {}
 

diff --git a/include/beman/execution26/detail/connect.hpp b/include/beman/execution26/detail/connect.hpp
@@ -21,14 +21,39 @@ namespace beman::execution26::detail {
  * \internal
  */
 struct connect_t {
+  private:
     template <typename Sender, typename Receiver>
-    auto operator()(Sender&& sender, Receiver&& receiver) const noexcept(true /*-dk:TODO*/) {
+    static auto make_new_sender(Sender&& sender, Receiver&& receiver) noexcept(true) -> decltype(auto) {
+        return ::beman::execution26::transform_sender(
+            decltype(::beman::execution26::detail::get_domain_late(::std::forward<Sender>(sender),
+                                                                   ::beman::execution26::get_env(receiver))){},
+            ::std::forward<Sender>(sender),
+            ::beman::execution26::get_env(receiver));
+    }
+    template <typename Sender, typename Receiver>
+    static constexpr auto connect_noexcept() -> bool {
+        if constexpr (requires {
+                          make_new_sender(::std::declval<Sender>(), ::std::declval<Receiver>())
+                              .connect(::std::declval<Receiver>());
+                      }) {
+            return noexcept(make_new_sender(::std::declval<Sender>(), ::std::declval<Receiver>())
+                                .connect(::std::declval<Receiver>()));
+        } else if constexpr (requires {
+                                 ::beman::execution26::detail::connect_awaitable(
+                                     make_new_sender(::std::declval<Sender>(), ::std::declval<Receiver>()),
+                                     ::std::declval<Receiver>());
+                             }) {
+            return noexcept(::beman::execution26::detail::connect_awaitable(
+                make_new_sender(::std::declval<Sender>(), ::std::declval<Receiver>()), ::std::declval<Receiver>()));
+        }
+        return true;
+    }
+
+  public:
+    template <typename Sender, typename Receiver>
+    auto operator()(Sender&& sender, Receiver&& receiver) const noexcept(connect_noexcept<Sender, Receiver>()) {
         auto new_sender = [&sender, &receiver]() -> decltype(auto) {
-            return ::beman::execution26::transform_sender(
-                decltype(::beman::execution26::detail::get_domain_late(::std::forward<Sender>(sender),
-                                                                       ::beman::execution26::get_env(receiver))){},
-                ::std::forward<Sender>(sender),
-                ::beman::execution26::get_env(receiver));
+            return make_new_sender(::std::forward<Sender>(sender), ::std::forward<Receiver>(receiver));
         };
 
         if constexpr (requires { new_sender().connect(::std::forward<Receiver>(receiver)); }) {