Skip to content

Commit

Permalink
Fix CI staticcheck warnings
Browse files Browse the repository at this point in the history
- 2 instances of use-after-move
- Missing #pragma once
- Also move ScopedConnection into connection_handle.h
  • Loading branch information
LeonMatthesKDAB committed Jan 12, 2024
1 parent 0d2e8f5 commit 62de012
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 97 deletions.
97 changes: 97 additions & 0 deletions src/kdbindings/connection_handle.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
Contact KDAB at <[email protected]> for commercial licensing options.
*/

#pragma once

#include <kdbindings/genindex_array.h>
#include <kdbindings/utils.h>
#include <memory>
Expand Down Expand Up @@ -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
95 changes: 0 additions & 95 deletions src/kdbindings/signal.h
Original file line number Diff line number Diff line change
Expand Up @@ -460,101 +460,6 @@ class Signal
mutable std::shared_ptr<Impl> 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.
*
Expand Down
16 changes: 14 additions & 2 deletions tests/signal/tst_signal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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());

Expand Down

0 comments on commit 62de012

Please sign in to comment.