From 44529fdebbab37504d27b5cf91f52678ecdeb13e Mon Sep 17 00:00:00 2001 From: Slava Andrejev Date: Wed, 26 May 2021 04:29:09 -0700 Subject: [PATCH] Allow slots with rvalue reference parameters In code based on Glibmm it's quite common to convert a Glib C structure to a Glib::Object's descendant before passing it as a parameter to a slot. If the slot is not supposed to modify the object, then everything is fine, we can declare the parameter as a const reference and the temporary object will be accurately dealt with by the compiler. However, if the Glib based object is getting modified, the only way to pass it to the slot is to declare it a Glib::RefPtr shared pointer. This creates a lot of unnecessary churn of memory allocations for a simple deal of calling a signal. However, C++ has a specific mean for this particular semantic of passing a temporary object: rvalue reference. For example, somewhere inside Gtkmm might be a declaration: _WRAP_SIGNAL(void sun_rose(const Glib::RefPtr &new_sun), "sun-rose") Instead its more semantically correct to write: _WRAP_SIGNAL(void sun_rose(Gtk::Sun &&new_sun), "sun-rose") And later somewhere in your code: world->signal_sun_rose().connect([&](auto &&new_sun) { new_sun.shine(); }); This commit makes a couple of simple modifications that allow declaring signals and slots with rvalue reference parameter. --- sigc++/functors/slot.h | 5 ++-- sigc++/signal.h | 10 +++++--- sigc++/type_traits.h | 7 +++++ tests/memleakcheck.sh | 4 +-- tests/meson.build | 1 + tests/test_rvalue_ref.cc | 55 ++++++++++++++++++++++++++++++++++++++++ 6 files changed, 75 insertions(+), 7 deletions(-) create mode 100644 tests/test_rvalue_ref.cc diff --git a/sigc++/functors/slot.h b/sigc++/functors/slot.h index 971de652..8e9e01a7 100644 --- a/sigc++/functors/slot.h +++ b/sigc++/functors/slot.h @@ -151,7 +151,8 @@ struct slot_call static T_return call_it(slot_rep* rep, type_trait_take_t... a_) { auto typed_rep = static_cast*>(rep); - return (*typed_rep->functor_).template operator()...>(a_...); + return (*typed_rep->functor_).template operator()...>( + std::forward>(a_)...); } /** Forms a function pointer from call_it(). @@ -220,7 +221,7 @@ class slot : public slot_base { return std::invoke(sigc::internal::function_pointer_cast(slot_base::rep_->call_), slot_base::rep_, - a...); + std::forward>(a)...); } return T_return(); diff --git a/sigc++/signal.h b/sigc++/signal.h index 69ca4d08..336bd5fd 100644 --- a/sigc++/signal.h +++ b/sigc++/signal.h @@ -363,7 +363,9 @@ struct signal_emit if (slot.empty() || slot.blocked()) continue; - (sigc::internal::function_pointer_cast(slot.rep_->call_))(slot.rep_, a...); + (sigc::internal::function_pointer_cast(slot.rep_->call_))( + slot.rep_, + std::forward>(a)...); } } }; @@ -450,11 +452,13 @@ class signal_with_accumulator : public signal_base decltype(auto) emit(type_trait_take_t... a) const { using emitter_type = internal::signal_emit; - return emitter_type::emit(impl_, a...); + return emitter_type::emit(impl_, std::forward>(a)...); } /** Triggers the emission of the signal (see emit()). */ - decltype(auto) operator()(type_trait_take_t... a) const { return emit(a...); } + decltype(auto) operator()(type_trait_take_t... a) const { + return emit(std::forward>(a)...); + } /** Creates a functor that calls emit() on this signal. * @code diff --git a/sigc++/type_traits.h b/sigc++/type_traits.h index 1af84e14..82690eb6 100644 --- a/sigc++/type_traits.h +++ b/sigc++/type_traits.h @@ -52,6 +52,13 @@ struct type_trait using take = const T_type&; }; +template +struct type_trait +{ + using pass = T_type&&; + using take = T_type&&; +}; + template<> struct type_trait { diff --git a/tests/memleakcheck.sh b/tests/memleakcheck.sh index 4c8f4cd9..38fbd586 100755 --- a/tests/memleakcheck.sh +++ b/tests/memleakcheck.sh @@ -9,8 +9,8 @@ for testprog in test_accum_iter test_accumulated test_bind test_bind_as_slot \ test_copy_invalid_slot test_cpp11_lambda test_custom test_disconnect \ test_disconnect_during_emit test_exception_catch test_hide \ test_limit_reference test_member_method_trait test_mem_fun test_ptr_fun \ - test_retype test_retype_return test_signal test_signal_move test_size \ - test_slot test_slot_disconnect test_slot_move test_trackable \ + test_retype test_retype_return test_rvalue_ref test_signal test_signal_move \ + test_size test_slot test_slot_disconnect test_slot_move test_trackable \ test_trackable_move test_track_obj test_tuple_cdr test_tuple_end \ test_tuple_for_each test_tuple_start test_tuple_transform_each \ test_visit_each test_visit_each_trackable test_weak_raw_ptr diff --git a/tests/meson.build b/tests/meson.build index 27ecda00..5b6b0c71 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -28,6 +28,7 @@ test_programs = [ [[], 'test_ptr_fun', ['test_ptr_fun.cc', 'testutilities.cc']], [[], '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_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_rvalue_ref.cc b/tests/test_rvalue_ref.cc new file mode 100644 index 00000000..b35ace17 --- /dev/null +++ b/tests/test_rvalue_ref.cc @@ -0,0 +1,55 @@ +#include "testutilities.h" +#include +#include + +struct MoveableStruct {}; + +namespace +{ +TestUtilities* util = nullptr; +std::ostringstream result_stream; + +struct foo +{ + void operator()(MoveableStruct &&x) + { + result_stream << "foo(MoveableStruct&&)"; + } +}; + +} // end anonymous namespace + +void +test_signal() +{ + sigc::signal signal; + foo f; + signal.connect(f); + MoveableStruct x; + signal(std::move(x)); + util->check_result(result_stream, "foo(MoveableStruct&&)"); +} + +void +test_slot() +{ + sigc::slot slot; + foo f; + slot = f; + MoveableStruct x; + slot(std::move(x)); + util->check_result(result_stream, "foo(MoveableStruct&&)"); +} + +int +main(int argc, char* argv[]) +{ + util = TestUtilities::get_instance(); + if (!util->check_command_args(argc, argv)) + return util->get_result_and_delete_instance() ? EXIT_SUCCESS : EXIT_FAILURE; + + test_signal(); + test_slot(); + + return util->get_result_and_delete_instance() ? EXIT_SUCCESS : EXIT_FAILURE; +} // end main()