From d1904cf652522ba966355fdf592db0aa4973cc5b Mon Sep 17 00:00:00 2001 From: Leon Matthes Date: Wed, 10 Jan 2024 10:36:08 +0100 Subject: [PATCH] Fix CI staticcheck warnings - 2 instances of use-after-move - Missing #pragma once - Also move ScopedConnection into connection_handle.h --- src/kdbindings/connection_handle.h | 97 ++++++++++++++++++++++++++++++ src/kdbindings/signal.h | 95 ----------------------------- tests/signal/tst_signal.cpp | 16 ++++- 3 files changed, 111 insertions(+), 97 deletions(-) diff --git a/src/kdbindings/connection_handle.h b/src/kdbindings/connection_handle.h index 247c9d5..a0a1811 100644 --- a/src/kdbindings/connection_handle.h +++ b/src/kdbindings/connection_handle.h @@ -9,6 +9,8 @@ Contact KDAB at for commercial licensing options. */ +#pragma once + #include #include #include @@ -206,4 +208,99 @@ class ConnectionHandle } }; +/** + * @brief A ScopedConnection is a RAII-style way to make sure a Connection is disconnected. + * + * When the ScopedConnections scope ends, the connection this ScopedConnection guards will be disconnected. + * + * Example: + * - @ref 08-managing-connections/main.cpp + */ +class ScopedConnection +{ +public: + /** + * @brief A ScopedConnection can be default constructed + * + * A default constructed ScopedConnection has no connection to guard. + * Therefore it does nothing when it is destructed, unless a ConnectionHandle is assigned to it. + */ + ScopedConnection() = default; + + /** A ScopedConnection can be move constructed */ + ScopedConnection(ScopedConnection &&) = default; + + /** A ScopedConnection cannot be copied */ + ScopedConnection(const ScopedConnection &) = delete; + /** A ScopedConnection cannot be copied */ + ScopedConnection &operator=(const ScopedConnection &) = delete; + + /** A ScopedConnection can be move assigned */ + ScopedConnection &operator=(ScopedConnection &&other) + { + m_connection.disconnect(); + m_connection = std::move(other.m_connection); + return *this; + } + + /** + * A ScopedConnection can be constructed from a ConnectionHandle + */ + ScopedConnection(ConnectionHandle &&h) + : m_connection(std::move(h)) + { + } + + /** + * A ScopedConnection can be assigned from a ConnectionHandle + */ + ScopedConnection &operator=(ConnectionHandle &&h) + { + return *this = ScopedConnection(std::move(h)); + } + + /** + * @return the handle to the connection this instance is managing + */ + ConnectionHandle &handle() + { + return m_connection; + } + + /** + * @overload + */ + const ConnectionHandle &handle() const + { + return m_connection; + } + + /** + * Convenience access to the underlying ConnectionHandle using the `->` operator. + */ + ConnectionHandle *operator->() + { + return &m_connection; + } + + /** + * @overload + */ + const ConnectionHandle *operator->() const + { + return &m_connection; + } + + /** + * When a ConnectionHandle is destructed it disconnects the connection it guards. + */ + ~ScopedConnection() + { + m_connection.disconnect(); + } + +private: + ConnectionHandle m_connection; +}; + } // namespace KDBindings diff --git a/src/kdbindings/signal.h b/src/kdbindings/signal.h index 44e0121..2350c31 100644 --- a/src/kdbindings/signal.h +++ b/src/kdbindings/signal.h @@ -460,101 +460,6 @@ class Signal mutable std::shared_ptr m_impl; }; -/** - * @brief A ScopedConnection is a RAII-style way to make sure a Connection is disconnected. - * - * When the ScopedConnections scope ends, the connection this ScopedConnection guards will be disconnected. - * - * Example: - * - @ref 08-managing-connections/main.cpp - */ -class ScopedConnection -{ -public: - /** - * @brief A ScopedConnection can be default constructed - * - * A default constructed ScopedConnection has no connection to guard. - * Therefore it does nothing when it is destructed, unless a ConnectionHandle is assigned to it. - */ - ScopedConnection() = default; - - /** A ScopedConnection can be move constructed */ - ScopedConnection(ScopedConnection &&) = default; - - /** A ScopedConnection cannot be copied */ - ScopedConnection(const ScopedConnection &) = delete; - /** A ScopedConnection cannot be copied */ - ScopedConnection &operator=(const ScopedConnection &) = delete; - - /** A ScopedConnection can be move assigned */ - ScopedConnection &operator=(ScopedConnection &&other) - { - m_connection.disconnect(); - m_connection = std::move(other.m_connection); - return *this; - } - - /** - * A ScopedConnection can be constructed from a ConnectionHandle - */ - ScopedConnection(ConnectionHandle &&h) - : m_connection(std::move(h)) - { - } - - /** - * A ScopedConnection can be assigned from a ConnectionHandle - */ - ScopedConnection &operator=(ConnectionHandle &&h) - { - return *this = ScopedConnection(std::move(h)); - } - - /** - * @return the handle to the connection this instance is managing - */ - ConnectionHandle &handle() - { - return m_connection; - } - - /** - * @overload - */ - const ConnectionHandle &handle() const - { - return m_connection; - } - - /** - * Convenience access to the underlying ConnectionHandle using the `->` operator. - */ - ConnectionHandle *operator->() - { - return &m_connection; - } - - /** - * @overload - */ - const ConnectionHandle *operator->() const - { - return &m_connection; - } - - /** - * When a ConnectionHandle is destructed it disconnects the connection it guards. - */ - ~ScopedConnection() - { - m_connection.disconnect(); - } - -private: - ConnectionHandle m_connection; -}; - /** * @brief A ConnectionBlocker is a convenient RAII-style mechanism for temporarily blocking a connection. * diff --git a/tests/signal/tst_signal.cpp b/tests/signal/tst_signal.cpp index 67cf4e9..e05c6d6 100644 --- a/tests/signal/tst_signal.cpp +++ b/tests/signal/tst_signal.cpp @@ -707,7 +707,14 @@ TEST_CASE("ScopedConnection") // This should drop the old connection of guard1 guard1 = std::move(guard2); - CHECK(!guard2->isActive()); + // Ideally we'd like to assert here that: + // CHECK(!guard2->isActive()); + // However, this is not possible, as a moved-from ScopedConnection is + // undefined (as is any C++ object for that matter). + // But because we assert that `guard1` is still active after the scope + // ends and that numCalled is only 1 after the emit, we can be sure that + // `guard2` was moved from and didn't disconnect. + CHECK(guard1->isActive()); signal.emit(); @@ -728,7 +735,12 @@ TEST_CASE("ScopedConnection") ScopedConnection guard = signal.connect([&called]() { called = true; }); REQUIRE(guard->isActive()); into = std::move(guard); - REQUIRE_FALSE(guard->isActive()); + // Ideally we'd like to assert here that: + // REQUIRE_FALSE(guard->isActive()); + // However, this is not possible, as a moved-from ScopedConnection is + // undefined (as is any C++ object for that matter). + // But because we assert that `into` is still active after the scope + // ends, we can be sure that `guard` was moved from and didn't disconnect. } REQUIRE(into->isActive());