From ab0b2a490eb98f3216403e87e4ea108a10fbeed2 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Thu, 21 Mar 2024 11:56:56 +0100 Subject: [PATCH] QSignalSpy: fix -Wweak-vtable by removing the QObject inheritance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Normally, we'd fix -Wweak-vtable by exporting the class and making at least one virtual method out-of-line (typically the dtor), thereby pinning the vtable to exactly one TU. We can't export QSignalSpy, though, because it also inherits QList, and we don't want to export QList subclasses to avoid QList API becoming part of the ABI. So remove the vtable, and therefore its being a weak symbol, by moving the qt_metacall implementation from QSignalSpy into its newly-added Private, at the cost of an additional memory allocation at construction (though there was already the wish to make this class pimpl'ed for extensibility, and this patch accomplishes exactly that). This class used to be one of few places left that prevents adding -Wweak-vtable to headersclean, so while this is a breaking change, QSignalSpy doesn't really model is-a QObject. It uses QObject to reuse, not to be reused. In fact, no external code should use the QObject-ness of QSignalSpy, so it should be an acceptable SC break to drop the inheritance. We don't need to care about BC here, as we don't promise BC for QtTest. This now also allows (and requires) to make the dtor and the private ctor out-of-line, avoiding the need for the init() hack. [ChangeLog][QtTest][Potentially Source-Incompatible Changes] QSignalSpy no longer inherits from QObject. If your code uses the fact that QSignalSpy is-a QObject, you need to redesign around this now. Task-number: QTBUG-45582 Fixes: QTBUG-123544 Change-Id: Id93ba0ee6bbb811455d3744a045e38e1b9f9c584 Reviewed-by: Tor Arne Vestbø Reviewed-by: Jason McDonald Reviewed-by: Ahmad Samir --- src/testlib/qsignalspy.cpp | 47 +++++++++++++++++++++++++++++++++----- src/testlib/qsignalspy.h | 33 +++++++------------------- 2 files changed, 49 insertions(+), 31 deletions(-) diff --git a/src/testlib/qsignalspy.cpp b/src/testlib/qsignalspy.cpp index 5fd61d42dc..a45ca59378 100644 --- a/src/testlib/qsignalspy.cpp +++ b/src/testlib/qsignalspy.cpp @@ -110,10 +110,6 @@ QT_BEGIN_NAMESPACE Returns the normalized signal the spy is currently listening to. */ -/*! \fn int QSignalSpy::qt_metacall(QMetaObject::Call call, int id, void **a) - \internal -*/ - /*! \fn bool QSignalSpy::wait(int timeout) \since 5.0 @@ -244,22 +240,42 @@ QList QSignalSpy::makeArgs(const QMetaMethod &member, const QObject *obj) return result; } -void QSignalSpy::init(ObjectSignal os) +class QSignalSpyPrivate : public QObject +{ + QSignalSpy * const q; +public: + explicit QSignalSpyPrivate(QSignalSpy *qq) : q(qq) {} + + int qt_metacall(QMetaObject::Call call, int methodId, void **a) override; +}; + +QSignalSpy::QSignalSpy(ObjectSignal os) + : args(os.obj ? makeArgs(os.sig, os.obj) : QList{}) { if (!os.obj) return; + auto i = std::make_unique(this); + const auto signalIndex = os.sig.methodIndex(); const auto slotIndex = QObject::staticMetaObject.methodCount(); if (!QMetaObject::connect(os.obj, signalIndex, - this, slotIndex, Qt::DirectConnection)) { + i.get(), slotIndex, Qt::DirectConnection)) { qWarning("QSignalSpy: QMetaObject::connect returned false. Unable to connect."); return; } + d_ptr = std::move(i); + sig = os.sig.methodSignature(); } +/*! + Destructor. +*/ +QSignalSpy::~QSignalSpy() + = default; + void QSignalSpy::appendArgs(void **a) { QList list; @@ -280,4 +296,23 @@ void QSignalSpy::appendArgs(void **a) } } +/*! + \reimp + \internal +*/ +int QSignalSpyPrivate::qt_metacall(QMetaObject::Call call, int methodId, void **a) +{ + methodId = QObject::qt_metacall(call, methodId, a); + if (methodId < 0) + return methodId; + + if (call == QMetaObject::InvokeMetaMethod) { + if (methodId == 0) { + q->appendArgs(a); + } + --methodId; + } + return methodId; +} + QT_END_NAMESPACE diff --git a/src/testlib/qsignalspy.h b/src/testlib/qsignalspy.h index f4c322ae71..591545b4d5 100644 --- a/src/testlib/qsignalspy.h +++ b/src/testlib/qsignalspy.h @@ -6,24 +6,26 @@ #include #include -#include #include #include #include #include +#include + QT_BEGIN_NAMESPACE class QVariant; - -class QSignalSpy: public QObject, public QList > +class QSignalSpyPrivate; +class QSignalSpy : public QList > { struct ObjectSignal { const QObject *obj; QMetaMethod sig; }; - + friend class QSignalSpyPrivate; + std::unique_ptr d_ptr; public: explicit QSignalSpy(const QObject *obj, const char *aSignal) : QSignalSpy(verify(obj, aSignal)) {} @@ -37,6 +39,7 @@ public: #endif // Q_QDOC QSignalSpy(const QObject *obj, QMetaMethod signal) : QSignalSpy(verify(obj, signal)) {} + Q_TESTLIB_EXPORT ~QSignalSpy(); inline bool isValid() const { return !sig.isEmpty(); } inline QByteArray signal() const { return sig; } @@ -46,28 +49,8 @@ public: Q_TESTLIB_EXPORT bool wait(std::chrono::milliseconds timeout = std::chrono::seconds{5}); - int qt_metacall(QMetaObject::Call call, int methodId, void **a) override - { - methodId = QObject::qt_metacall(call, methodId, a); - if (methodId < 0) - return methodId; - - if (call == QMetaObject::InvokeMetaMethod) { - if (methodId == 0) { - appendArgs(a); - } - --methodId; - } - return methodId; - } - private: - explicit QSignalSpy(ObjectSignal os) - : args(os.obj ? makeArgs(os.sig, os.obj) : QList{}) - { - init(os); - } - Q_TESTLIB_EXPORT void init(ObjectSignal os); + Q_TESTLIB_EXPORT explicit QSignalSpy(ObjectSignal os); Q_TESTLIB_EXPORT static ObjectSignal verify(const QObject *obj, QMetaMethod signal); Q_TESTLIB_EXPORT static ObjectSignal verify(const QObject *obj, const char *aSignal);