From b878ab7ae57e0147732667414327caf43e4d30f6 Mon Sep 17 00:00:00 2001 From: Daniel Boles Date: Sat, 22 Jul 2023 18:32:53 +0100 Subject: [PATCH] =?UTF-8?q?scoped=5Fconnection:=20new=20wrapper=20to=20aut?= =?UTF-8?q?o-disconnect=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …a contained sigc::connection, when the scoped_connection is destructed. https://github.com/libsigcplusplus/libsigcplusplus/issues/87 --- sigc++/connection.h | 9 ++ sigc++/filelist.am | 1 + sigc++/meson.build | 2 + sigc++/scoped_connection.cc | 117 +++++++++++++++++ sigc++/scoped_connection.h | 175 ++++++++++++++++++++++++++ sigc++/sigc++.h | 1 + tests/Makefile.am | 1 + tests/meson.build | 1 + tests/test_scoped_connection.cc | 214 ++++++++++++++++++++++++++++++++ 9 files changed, 521 insertions(+) create mode 100644 sigc++/scoped_connection.cc create mode 100644 sigc++/scoped_connection.h create mode 100644 tests/test_scoped_connection.cc diff --git a/sigc++/connection.h b/sigc++/connection.h index 0e733760..6f20a8bb 100644 --- a/sigc++/connection.h +++ b/sigc++/connection.h @@ -16,8 +16,10 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA * */ + #ifndef SIGC_CONNECTION_HPP #define SIGC_CONNECTION_HPP + #include #include #include @@ -30,13 +32,20 @@ namespace sigc * This may be used to disconnect the referred slot at any time (disconnect()). * @ref sigc::signal_with_accumulator::connect() "sigc::signal::connect()" * returns a %sigc::connection. + * * @code * sigc::connection conn = sig.connect(sigc::mem_fun(a, &A::foo)); * @endcode + * * If the slot has already been destroyed, disconnect() does nothing. empty() or * operator bool() can be used to test whether the connection is * still active. The connection can be blocked (block(), unblock()). * + * sigc::connection doesnʼt disconnect the slot automatically upon destruction. + * You do not need to keep the sigc::connection object to retain the connection + * of the slot to the signal. See also @ref sigc::scoped_connection, which does + * diconnect automatically when the connection object is destroyed or replaced. + * * @ingroup signal */ struct SIGC_API connection diff --git a/sigc++/filelist.am b/sigc++/filelist.am index 4092b241..af2afc6d 100644 --- a/sigc++/filelist.am +++ b/sigc++/filelist.am @@ -24,6 +24,7 @@ sigc_public_h = \ member_method_trait.h \ reference_wrapper.h \ retype_return.h \ + scoped_connection.h \ signal.h \ signal_base.h \ slot.h \ diff --git a/sigc++/meson.build b/sigc++/meson.build index d9c1231d..c645f074 100644 --- a/sigc++/meson.build +++ b/sigc++/meson.build @@ -8,6 +8,7 @@ source_cc_files = [ 'connection.cc', + 'scoped_connection.cc', 'signal_base.cc', 'trackable.cc', 'functors' / 'slot_base.cc', @@ -21,6 +22,7 @@ sigc_h_files = [ 'member_method_trait.h', 'reference_wrapper.h', 'retype_return.h', + 'scoped_connection.h', 'signal.h', 'signal_base.h', 'slot.h', diff --git a/sigc++/scoped_connection.cc b/sigc++/scoped_connection.cc new file mode 100644 index 00000000..24443bd7 --- /dev/null +++ b/sigc++/scoped_connection.cc @@ -0,0 +1,117 @@ +/* + * Copyright 2023, The libsigc++ Development Team + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + +#include +#include + +namespace sigc +{ + +// All we are doing is assigning weak_raw_ptr, which is noexcept, so declare it. +// connectionʼs copy operators can be noexcept for that reason, if breaking ABI. +scoped_connection::scoped_connection(connection c) noexcept +: conn_(std::move(c)) +{ +} + +scoped_connection& +scoped_connection::operator=(connection c) +{ + conn_.disconnect(); + conn_ = std::move(c); + return *this; +} + +// We do not implement move-ctor in terms of move-assign, so we can be noexcept, +// as we do not need to call the maybe-throwing disconnect() for obvious reason. +scoped_connection::scoped_connection(scoped_connection&& sc) noexcept +: conn_(std::exchange(sc.conn_, connection())) +{ +} + +scoped_connection& +scoped_connection::operator=(scoped_connection&& sc) +{ + conn_.disconnect(); + conn_ = std::exchange(sc.conn_, connection()); + return *this; +} + +scoped_connection::~scoped_connection() +{ + conn_.disconnect(); +} + +bool +scoped_connection::empty() const noexcept +{ + return conn_.empty(); +} + +bool +scoped_connection::connected() const noexcept +{ + return conn_.connected(); +} + +bool +scoped_connection::blocked() const noexcept +{ + return conn_.blocked(); +} + +bool +scoped_connection::block(bool should_block) noexcept +{ + return conn_.block(should_block); +} + +bool +scoped_connection::unblock() noexcept +{ + return conn_.unblock(); +} + +void +scoped_connection::disconnect() +{ + conn_.disconnect(); +} + +scoped_connection::operator bool() const noexcept +{ + return conn_.operator bool(); +} + +// Swapping can be noexcept, as it does not need to disconnect either connection +// because they will still stay alive, just in opposite instances post-swapping. +void +swap(scoped_connection &sca, scoped_connection &scb) noexcept +{ + using std::swap; + swap(sca.conn_, scb.conn_); +} + +connection +scoped_connection::release() noexcept +{ + return std::exchange(conn_, connection()); +} + +} /* namespace sigc */ diff --git a/sigc++/scoped_connection.h b/sigc++/scoped_connection.h new file mode 100644 index 00000000..8fcd779a --- /dev/null +++ b/sigc++/scoped_connection.h @@ -0,0 +1,175 @@ +/* + * Copyright 2023, The libsigc++ Development Team + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + +#ifndef SIGC_SCOPED_CONNECTION_HPP +#define SIGC_SCOPED_CONNECTION_HPP + +#include + +namespace sigc +{ + +/** Convenience class for safe disconnection, including automatic disconnection + * upon destruction. + * + * This is a variant of @ref sigc::connection which also disconnect()s the slot + * automatically when the scoped_connection is destructed or re-assigned. Refer + * to @ref sigc::connection for full information about the common functionality. + * + * You will use sigc::scoped_connection by constructing it from a ‘normal’, + * unscoped @ref sigc::connection, such as those returned by + * @ref sigc::signal_with_accumulator::connect() "sigc::signal::connect()", thus + * ‘wrapping’ the connection in a scoped_connection, adding auto-disconnection. + * It can also be assigned from an unscoped connection, in which case, if there + * was a previous slot referred to by the scoped connection, it is disconnected. + * + * Once a connection is scoped, it canʼt be copied as that would make it unclear + * which of the copies would hold responsibility to auto-disconnect the slot. It + * can, however, be moved, so itʼs usable in containers or so ‘ownership’ of the + * connection/auto-disconnect can be moved to another instance. Moving from the + * scoped_connection clears its reference to the slot so it wonʼt disconnect it. + * + * If you want a reference-counted scoped_connection, wrap in a std::shared_ptr. + * + * @code + * // Automatic disconnection: + * { + * sigc::scoped_connection sconn = sig.connect(&some_function); + * // Do stuff that requires the slot to be connected & called. + * } + * // The scoped_connection was destroyed, so the slot is no longer connected. + * + * // *** + * + * // Moving ownership: + * { + * sigc::scoped_connection sconn = sig.connect(&some_function); + * // Do stuff that requires the slot to be connected & called. + * take_ownership(std::move(sconn)); // Pass by rvalue. + * } + * // Now our `sconn` no longer referred to slot, so it did NOT auto-disconnect. + * + * // *** + * + * // Shared ownership: + * { + * auto shconn = std::make_shared(sig.connect(&some_function)); + * take_copy(shconn); // Pass by copy/value + * // Now we AND take_copy() must destroy our shared_ptr to auto-disconnect(). + * } + * // take_copy() may still hold a shared_ptr reference, keeping the slot alive. + * @endcode + * + * @ingroup signal + * @newin{3,6} + */ +struct SIGC_API scoped_connection final +{ + /** Constructs an empty scoped connection object. */ + [[nodiscard]] scoped_connection() noexcept = default; + + /** Constructs a scoped connection object from an unscoped connection object. + * The source connection still refers to the slot and can manually disconnect. + * @param c The connection object to make a copy from, whose slot weʼll + * automatically disconnect when the scoped_connection object is destroyed. + */ + [[nodiscard]] scoped_connection(connection c) noexcept; + + /** Overrides this scoped connection object copying an unscoped connection. + * The current slot, if any, will be disconnect()ed before being replaced. + * The source connection still refers to the slot and can manually disconnect. + * @param c The connection object to make a copy from, whose slot weʼll + * automatically disconnect when the scoped_connection object is destroyed. + */ + scoped_connection& operator=(connection c); + + /// scoped_connection canʼt be copied as it would confuse ownership—see intro. + scoped_connection& operator=(const scoped_connection&) = delete; + /// scoped_connection canʼt be copied as it would confuse ownership—see intro. + scoped_connection(const scoped_connection&) = delete; + + /** Constructs a scoped connection object moving an existing one. + * The source scoped connection will no longer refer to / disconnect the slot. + * @param sc The scoped connection object to move from. + */ + scoped_connection(scoped_connection&& sc) noexcept; + + /** Overrides this scoped connection object moving another one. + * The current slot, if any, will be disconnect()ed before being replaced. + * The source scoped connection will no longer refer to / disconnect the slot. + * @param sc The scoped connection object to move from. + */ + scoped_connection& operator=(scoped_connection&& sc); + + /// Swap two scoped connections. + friend void swap(scoped_connection &sca, scoped_connection &scb) noexcept; + + /// scoped_connection disconnects the referred slot, if any, upon destruction. + ~scoped_connection(); + + /** Returns whether the connection is still active. + * @return @p false if the connection is still active. + */ + [[nodiscard]] bool empty() const noexcept; + + /** Returns whether the connection is still active. + * @return @p true if the connection is still active. + */ + [[nodiscard]] bool connected() const noexcept; + + /** Returns whether the connection is blocked. + * @return @p true if the connection is blocked. + */ + [[nodiscard]] bool blocked() const noexcept; + + /** Sets or unsets the blocking state of this connection. + * See slot_base::block() for details. + * @param should_block Indicates whether the blocking state should be set or unset. + * @return @p true if the connection has been in blocking state before. + */ + bool block(bool should_block = true) noexcept; + + /** Unsets the blocking state of this connection. + * @return @p true if the connection has been in blocking state before. + */ + bool unblock() noexcept; + + /// Disconnects the referred slot. This will also happen upon destruction. + void disconnect(); + + /** Returns whether the connection is still active. + * @return @p true if the connection is still active. + */ + [[nodiscard]] explicit operator bool() const noexcept; + + /** Releases the connection from a scoped connection object. + * The scoped connection will no longer refer to / disconnect the slot. + * @return An unscoped connection object referring to the same slot. + */ + [[nodiscard]] connection release() noexcept; + +private: + sigc::connection conn_; +}; + +void swap(scoped_connection &sca, scoped_connection &scb) noexcept; + +} /* namespace sigc */ + +#endif /* SIGC_SCOPED_CONNECTION_HPP */ diff --git a/sigc++/sigc++.h b/sigc++/sigc++.h index b0b4ebbb..1387f314 100644 --- a/sigc++/sigc++.h +++ b/sigc++/sigc++.h @@ -117,6 +117,7 @@ #include #include +#include #include #include #include diff --git a/tests/Makefile.am b/tests/Makefile.am index b05f7b47..7ea2b98f 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -47,6 +47,7 @@ check_PROGRAMS = \ test_retype \ test_retype_return \ test_rvalue_ref \ + test_scoped_connection \ test_signal \ test_signal_move \ test_size \ diff --git a/tests/meson.build b/tests/meson.build index e0f57d72..a9c65fdf 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -29,6 +29,7 @@ test_programs = [ [[], 'test_retype', ['test_retype.cc', 'testutilities.cc']], [[], 'test_retype_return', ['test_retype_return.cc', 'testutilities.cc']], [[], 'test_rvalue_ref', ['test_rvalue_ref.cc', 'testutilities.cc']], + [[], 'test_scoped_connection', ['test_scoped_connection.cc', 'testutilities.cc']], [[], 'test_signal', ['test_signal.cc', 'testutilities.cc']], [[], 'test_signal_move', ['test_signal_move.cc', 'testutilities.cc']], [[], 'test_size', ['test_size.cc', 'testutilities.cc']], diff --git a/tests/test_scoped_connection.cc b/tests/test_scoped_connection.cc new file mode 100644 index 00000000..bc4849a4 --- /dev/null +++ b/tests/test_scoped_connection.cc @@ -0,0 +1,214 @@ +/* Copyright 2023, The libsigc++ Development Team + * Assigned to public domain. Use as you wish without restriction. + */ + +#include "testutilities.h" +#include +#include +#include +#include +#include + +// Test the expected special members and conversions, esp. NOT copyable BUT movable. +static_assert( std::is_nothrow_default_constructible_v); +static_assert(not std::is_copy_constructible_v ); +static_assert(not std::is_copy_assignable_v ); +static_assert( std::is_nothrow_move_constructible_v ); +static_assert( std::is_move_assignable_v ); +static_assert( std::is_nothrow_swappable_v ); +// TODO: C++20: Test the stronger std::is_nothrow_convertible_v; it should pass. +static_assert( std::is_convertible_v); +static_assert(not std::is_convertible_v); +static_assert( std::is_assignable_v ); +static_assert(not std::is_assignable_v ); + +namespace +{ +std::ostringstream result_stream; + +int +foo(int i) +{ + result_stream << "foo(" << i << ") "; + return 1; +} + +int +bar(double i) +{ + result_stream << "bar(" << i << ") "; + return 1; +} + +struct A : public sigc::trackable +{ + int foo(int i) + { + result_stream << "A::foo(" << i << ") "; + return 1; + } +}; + +} // end anonymous namespace + +int +main(int argc, char* argv[]) +{ + auto util = TestUtilities::get_instance(); + + if (!util->check_command_args(argc, argv)) + return util->get_result_and_delete_instance() ? EXIT_SUCCESS : EXIT_FAILURE; + + sigc::signal sig; + sigc::connection confoo; + sigc::connection conbar; + sigc::connection cona; + + { + A a; + sig.connect(sigc::mem_fun(a, &A::foo)); + confoo = sig.connect(&foo); + conbar = sig.connect(&bar); + result_stream << "sig is connected to (size=" << sig.size() << "): "; + sig(1); + util->check_result( + result_stream, "sig is connected to (size=3): A::foo(1) foo(1) bar(1) "); + } + // normal connections are still connected. mem_fun disconnected via trackable. + result_stream << "sig is connected to (size=" << sig.size() << "): "; + sig(2); + util->check_result(result_stream, "sig is connected to (size=2): foo(2) bar(2) "); + + { + A a; + sig.connect(sigc::mem_fun(a, &A::foo)); + sigc::scoped_connection sconfoo = sig.connect(&foo); + sigc::scoped_connection sconbar = sig.connect(&bar); + result_stream << "sig is connected to (size=" << sig.size() << "): "; + sig(3); + util->check_result( + result_stream, "sig is connected to (size=5): foo(3) bar(3) A::foo(3) foo(3) bar(3) "); + } + // scoped connections are now disconnected. + result_stream << "sig is connected to (size=" << sig.size() << "): "; + sig(4); + util->check_result(result_stream, "sig is connected to (size=2): foo(4) bar(4) "); + + // copying connection to a scoped connection disconnects when latter destroyed + // copy-constructor: + { + sigc::scoped_connection sconfoo = confoo; + result_stream << "sig is connected to (size=" << sig.size() << "): "; + sig(5); + util->check_result( + result_stream, "sig is connected to (size=2): foo(5) bar(5) "); + } + result_stream << "sig is connected to (size=" << sig.size() << "): "; + sig(6); + util->check_result( + result_stream, "sig is connected to (size=1): bar(6) "); + // copy-assignment: + confoo = sig.connect(&foo); + { + sigc::scoped_connection sconfoo = sig.connect(&bar); + result_stream << "sig is connected to (size=" << sig.size() << "): "; + sig(7); + util->check_result( + result_stream, "sig is connected to (size=3): bar(7) foo(7) bar(7) "); + // copy-assignment disconnects currently held connection & replaces with new + sconfoo = confoo; + result_stream << "sig is connected to (size=" << sig.size() << "): "; + sig(8); + util->check_result( + result_stream, "sig is connected to (size=2): bar(8) foo(8) "); + } + result_stream << "sig is connected to (size=" << sig.size() << "): "; + sig(9); + util->check_result( + result_stream, "sig is connected to (size=1): bar(9) "); + + // moving scoped_connection transfers ownership/disconnection to destination + // move-constructor: + { + auto src = std::make_unique(sig.connect(&foo)); + result_stream << "sig is connected to (size=" << sig.size() << "): "; + sig(10); + util->check_result( + result_stream, "sig is connected to (size=2): bar(10) foo(10) "); + + sigc::scoped_connection dst = std::move(*src); + src.reset(); // This will NOT disconnect from foo() + result_stream << "sig is connected to (size=" << sig.size() << "): "; + sig(11); + util->check_result( + result_stream, "sig is connected to (size=2): bar(11) foo(11) "); + } + + // move-assignment: + { + auto src = std::make_unique(sig.connect(&foo)); + result_stream << "sig is connected to (size=" << sig.size() << "): "; + sig(12); + util->check_result( + result_stream, "sig is connected to (size=2): bar(12) foo(12) "); + + sigc::scoped_connection dst; + dst = std::move(*src); + src.reset(); // This will NOT disconnect from foo() + result_stream << "sig is connected to (size=" << sig.size() << "): "; + sig(13); + util->check_result( + result_stream, "sig is connected to (size=2): bar(13) foo(13) "); + } + + // dst from above is now destroyed + result_stream << "sig is connected to (size=" << sig.size() << "): "; + sig(14); + util->check_result( + result_stream, "sig is connected to (size=1): bar(14) "); + + // swap + sigc::scoped_connection sconfoo = sig.connect(&foo); + result_stream << "sig is connected to (size=" << sig.size() << "): "; + sig(15); + util->check_result( + result_stream, "sig is connected to (size=2): bar(15) foo(15) "); + sigc::scoped_connection sconbar = sig.connect(&bar); + result_stream << "sig is connected to (size=" << sig.size() << "): "; + sig(16); + util->check_result( + result_stream, "sig is connected to (size=3): bar(16) foo(16) bar(16) "); + swap(sconbar, sconfoo); + // disconnect sconbar, which was swapped to refer to &foo + sconbar.disconnect(); + result_stream << "sig is connected to (size=" << sig.size() << "): "; + sig(17); + util->check_result( + result_stream, "sig is connected to (size=2): bar(17) bar(17) "); + + // manual disconnection + sconfoo.disconnect(); // was swapped to refer to 2nd &bar + result_stream << "sig is connected to (size=" << sig.size() << "): "; + sig(18); + util->check_result( + result_stream, "sig is connected to (size=1): bar(18) "); + + // release + sconfoo = sig.connect(&foo); + result_stream << "sig is connected to (size=" << sig.size() << "): "; + sig(19); + util->check_result( + result_stream, "sig is connected to (size=2): bar(19) foo(19) "); + sigc::connection rconfoo = sconfoo.release(); + result_stream << "sig is connected to (size=" << sig.size() << "): "; + sig(20); + util->check_result( + result_stream, "sig is connected to (size=2): bar(20) foo(20) "); + rconfoo.disconnect(); + result_stream << "sig is connected to (size=" << sig.size() << "): "; + sig(21); + util->check_result( + result_stream, "sig is connected to (size=1): bar(21) "); + + return util->get_result_and_delete_instance() ? EXIT_SUCCESS : EXIT_FAILURE; +}