Review Request 117478: Convert dolphin (frameworks) to Qt5 signal/slot syntax

[Olivier Goffart]

On Monday 21 April 2014 14:49:21 Olivier Goffart wrote:
> On Wednesday 16 April 2014 20:47:10 Alexander Richardson wrote:
> > 2014-04-16 20:17 GMT+02:00 Alexander Richardson
> > 
> > > Ah yes, I completely forgot about slots being pseudo-virtual functions.
> > > 
> > > Would comparing the value of metaObject() be a possible solution to
> > > calling slots from partially destructed objects?
> > > Not sure if my approach in this test here is too naive, but it seems
> > > to work. Maybe it breaks with other compilers than GCC or multiple or
> > > virtual inheritance, I don't know that. To me it seems that the slot
> > > is not invoked with the old syntax because the slot is not found
> > > inside metaObject(), but maybe I am wrong, I didn't dig too deeply
> > > into the metaobject source code.
> > > 
> > > In what I gathered from
> > > http://woboq.com/blog/how-qt-signals-slots-work-part2-qt5.html a
> > > QMetaObject* could be stored in QSlotObjectBase and compared to
> > > receiver->metaObject() in order to determine whether the object is
> > > being destroyed. Maybe there is a better place to store it as well
> > > since adding another pointer to each signal slot connection sounds
> > > expensive. Maybe inside QObjectPrivate, not sure if that is possible.
> > > 
> > > I think this should work since the destructor changes the vtable
> > > pointer from what I found in my testing and googling, but I can't
> > > quote the standard on it.
> > > 
> > > Assuming this fix for calling slots from the destructor after the
> > > object has been destroyed the automatic conversion should be safe,
> > > right?
> > > Of course there needs to be a static analysis tool that makes sure
> > > that no slots with the same name and parameter as a base class slot
> > > are declared if they are not virtual, but that shouldn't be too
> > > complicated using clang.
> > > 
> > > Regards,
> > > Alex
> > 
> > I updated the test case, this should also allow valid calls from the
> > destructor, but would definitly require storing the metaObject used at
> > the time of connection to be stored inside the connection metadata
> > (QSlotObjectBase).
> > This would mean storing
> > &QtPrivate::FunctionPointer<Func1>::Object::staticMetaObject; in
> > checking in a loop over the superclasses if this MetaObject is equal
> > to metaObject();
> > Still not sure if this is correct, but it seems to be better than the
> > previous solution
> > 
> > Regards,
> > Alex
> 
> Yes.
> As a matter of fact, we do something very similar in the classic syntax
> case: if (... && c->method_offset <=
> receiver->metaObject()->methodOffset())
> http://code.woboq.org/qt5/qtbase/src/corelib/kernel/qobject.cpp.html#3674
> 
> However we really want the best performance when emitting a signal, and i am
> afraid that one more virtual call (to get the meta is not neglectible). But
> we probably will need to do something similar to what you did.
> 
> My idea was to store the metaobject in the QObjectPrivate::Connection.  But
> storing it in QSlotObject is also a good idea.  The only problem is that
> adding more code in QSlotObject means that the code will be duplicated for
> every possible template instance. But the advantage is that it will not slow
> down at all the lambda case.
> 
> 
> About the lambda case, this is another problem i was discussing with Dario
> some times ago:
>  connect(sender, &Sender::sig, context, [=]{context->doSomething(foobar);});
> In that case, we do not know what MetaObject context really need to have. 
> So I guess for this case there is nothing that can be done.  But that
> should not keep us away from fixing the other cases.

Implemented here:

https://codereview.qt-project.org/83800

-- 
Olivier

Woboq - Qt services and support - http://woboq.com - http://code.woboq.org