Allow slots with rvalue reference parameters #74
Conversation
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<Gtk::Sun> &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.
|
I expanded your test case to see what happens if you connect more than one signal This behavior is too risky. We would have to describe unnatural restrictions on Concerning signals in glibmm and gtkmm, I don't quite understand what temporary An aside: My comment in the gtkmm-documentation issue https://gitlab.gnome.org/GNOME/gtkmm-documentation/-/issues/10 |
|
Thank you for spending your time investigating the proposed change. You raised a great question. Let me address it. The situation you described is not particular to rvalue references. It's not even particular to C++. It can happen with anything that is being modified by a signal handler. Here is an example from the official GStreamer tutorial: If you have n handlers they will try to connect a poor pad n times. With the current sigc++ the correspondent C++ signal declaration will look like this:
At this point nothing prevents you to do whatever you want with There is more to it. The suggested change is intended for a case when you do modify the underlying object. If you do it more than once, you are doomed regardless of whether it's an rvalue reference or not, and nothing prevents you from doing this in sigc++. Giving that if you look at the declaration: you have no knowledge if you can modify It begs you with explicitely expressed semantic to pay attention to what you are doing. Isn't it more clear now that you probably shouldn't mess up with the guts of The bottom line is: it doesn't add more doom, on opposite it expresses the clear intent and begs the programmer to pay attention.
Rvalue reference is just a hint from the compiler that the object is temporary, "move" is an agreement that exists only for constructors and assignemnt operators. In contrast for regular methods rvalue reference doesn't mean you have to do a move or anything else. When a C object arrives to a C++ handler it must be converted to a C++ object. This C++ wrapper is temporary, just to pass the parameter. Creating a second wrapper (shared_ptr) around the first wrapper just to pass a parameter is absolutly unnecessary, because the lifetime of a callee is a strict subset of the caller. In addition it allocates memory on heap for strong and weak reference counters. Every time, just to pass a parameter, you allocate 8 bytes (4 for ref count and 4 for weak ref count) on heap, and then immediately delete it. Besides performace concerns it also creates holes in the heap, eventually it starts reminding Swiss cheese, the amount of consumed memory grows, though it's not allocated. |
|
Bjarne Stroustrup describes the different types of references like so
I don't understand what would be wrong with a non-const lvalue reference in the example
If the C object is a descendant of GObject, the C++ wrapper is not temporary. |
|
Why are you citing Stroustrup to me? I am reading his C++ books since 90s.
You can't pass Gst::Pad to it, if Gst::Pad is an rvalue. Only const lvalue references can be bound to rvalues.
You can pass only descendants of GObject as a parameter? Why are we discussing the qualities of the example not related to the qualities of the proposed change? You had a concern about multiple handlers modifying the same object. I showed you that this is not specific to rvalue references. Honestly, I think that somebody just forgot that univeral references must be forwarded in a couple of places when converted the code from .m4 macros. Let's make a fix that can benefit the users. |
|
Thank you for merging and for a good discussion! |
I tried to compile your test case in libsigc++2 (the libsigc++-2-10 branch), I think that slots with rvalue references have never worked. Have you seen it I'm slowly (too slowly?) beginning to understand why you want to use rvalue refs. to If the intent is to make it possible for the signal handler to call non-const methods The signal handler need not know that the C++ wrapper (but not the wrapped GstPad, I suppose) |
|
Why it would have worked? The code was introduced before C++11. Then it was refactored with occasional
Semantically it's temporary, because the signal is coming from the C code, thus the C++ wrapper is just to pass the parameter to the C++ code. That's why it is called a "wrapper". Caching it in the guts of GObject is an implementation detail. Therefore, in the proper design it should be the other way around: the handler need not know the C++ wrapper is NOT temporary.
No, I wouldn't. Everything works fine, exactly as I want without hand-written pieces. |
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:
Instead its more semantically correct to write:
And later somewhere in your code:
This commit makes a couple of simple modifications that allow declaring
signals and slots with rvalue reference parameter.