From kfm-devel Mon Apr 21 21:03:13 2014 From: Olivier Goffart Date: Mon, 21 Apr 2014 21:03:13 +0000 To: kfm-devel Subject: Re: Review Request 117478: Convert dolphin (frameworks) to Qt5 signal/slot syntax Message-Id: <3023261.pZipfcCloe () finn> X-MARC-Message: https://marc.info/?l=kfm-devel&m=139811421432004 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::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