GNOME Bugzilla – Bug 70358
g_signal_emitv() and g_signal_emit_valist() have different semantics
Last modified: 2011-02-18 15:55:24 UTC
There is a difference between how g_signal_emitv() and g_signal_emit_valist() works (and g_signal_emit which is just a wrapper over valist). The valist version sets the return value even if there is no handler while the emitv does not. In old gtk 1.2 the gtk_signal_emit() did not set the return value if there was no handler. Now gtk_signal_emit() is a wrapper of g_signal_emit(), a lot of old code could break here. I found many, many places in gtk where people set the return value before calling g_signal_emit(), in fact all but 4 emits did set a default. When I say set the default I mean code like: gboolean ret = TRUE; g_signal_emit ( ..., &ret); As an example of something broken I can show gtktipsquery.c line 491 where the return value is a gboolean that is set to TRUE before emit(). If there is no handler, then g_signal_emit() will set the default to FALSE anyway. Whoever that set it to TRUE obviously wanted that as a default. I think it is very natural that you a) should be able to set the default and b) that if there are no handler then there should be no one that assigns to the variable that I send in a pointer to. In gtkwidget there are two places (MNEMONIC_ACTIVATE,FOCUS) where you don't set the default before emitting, but in this case there is a class method so there will never be no handler there (unless the user can block the class method?). I've also grep:ed and looked at all signals in gtk that have a non-void return and almost all do set the return value before emitting, I found just 4 places where it was not set. (I'll attach a patch for these). It is very easy to change g_signal_emit_valist (and thus g_signal_emit) to work like gtk_signal_emit and g_signal_emitv. I'll attach a patch for that. The function signal_emit_unlocked_R() that does all the work already returns a boolean saying if a handler have altered the return value. But this information is not used. I realize that this is a change that you might not want to do this late in the freeze. And that is up to you. A problem so far is that I have found nowhere in the documentation saying what the semantics should be. So I can't even tell if this is a bug or a by design. I do know that no matter what you choose there is broken code out there. Either old code that depends on setting the default value before emitting (even gtk+ itself do this) or new code that assumes that the return value will always be set to something even without handlers. Also, with the new way there is no way for the programmer to select a default value. I think that is a design misstake, which is why I think this is actually a bug. Especially when you could do it before and that it is very natural and easy to allow it.
Created attachment 6585 [details] [review] don't overwrite default value when there is no handler
Created attachment 6586 [details] [review] 4 places in gtk where default is not set before emitting
This was discussed in detail when we made the decision to do it this way, there were good reasons for doing it this way, and we aren't going to change it back now. (It would be a very hard to catch API change if nothing else...) All the setting the value before emitting is simply a historical artifact of how things worked in GTK+-1.2. Could you file a separate bug about the GtkTipsQuery return value? Unfortunately, the discussion was on IRC and I don't really remember it well know. I'll ask Tim Janik to add some explanation here.
Can I depend on that g_signal_emitv() does not assign to the return value even in future versions of gtk+? Then you can use that function to emit a signal when you need to use another default value (in fact this is what I do now).
assigning the default value in the emit_valist() variant was changed in gtk+2.0 as a design decision. the reason is mostly, that many people forgot to provide default values or expected the default to be always set. besides that, there were some language binding issues, where LBs always had to connect a handler, even if no user code was executed which always overwrote the default value. with the new code, if the default value was used to figure whether any handler was executed at all, that should now be done via signal accumulators. in case it's just set to provide a special default value (different from what most handlers would return), then that behaviour is still available via g_signal_emitv(), without impacting LBs, as those can leave the return_value untouched in their closure marshal implementations.
I disagree with this choice because it breaks existing code in non-obvious ways to help avoid problems with new code that does not do things correctly. However, it is obviously too late to change this. This behavior should be documented somewhere in large letters. Especially the asymmetry with g_signal_emitv.
I have added notes on the return value behaviour in absence of handlers to g_signal_emit, g_signal_emitv and g_signal_emit_valist now.