From 6f0f9f69288925ef423c542ef5eb7302a5431867 Mon Sep 17 00:00:00 2001 From: "Bradley T. Hughes" Date: Tue, 22 Nov 2011 14:45:35 +0100 Subject: [PATCH] Remove QMetaObject guards and deprecate QPointer. QWeakPointer is superior and preferred. Remove QMetaObject::addGuard(), QMetaObject::changeGuard(), QMetaObject::removeGuard(), and QObjectPrivate::clearGuards(). Implement QPointer using QWeakPointer instead. This changes the behavior of QPointer in 2 ways: - During destruction of a QWidget. Previously, the destructor of QWidget would reset all QPointers so that they would return zero when destroying children. Update tst_QPointer to account for this change. - When constructing a QSharedPointer to take ownership of an object after a QPointer is already tracking the object. Previously, the shared pointer construction would not be affected by the QPointer, but now that QPointer is implemented using QWeakPoiner, constructing the QSharedPointer will cause an abort(). Fix tst_QSharedPointer by removing the use of QPointer in the objectCast() test. These behavior changes are documented in the QPointer class documentation and in the changes file. Change-Id: I92d0276219c076ece7bcb60f6e1b9120ce4f5747 Reviewed-by: Lars Knoll Reviewed-by: Olivier Goffart --- dist/changes-5.0.0 | 18 +++ src/corelib/kernel/qobject.cpp | 114 ------------------ src/corelib/kernel/qobject.h | 3 +- src/corelib/kernel/qobject_p.h | 1 - src/corelib/kernel/qobjectdefs.h | 5 - src/corelib/kernel/qpointer.cpp | 20 ++- src/corelib/kernel/qpointer.h | 37 +++--- src/widgets/kernel/qaction.cpp | 9 +- src/widgets/kernel/qwidget.cpp | 4 - .../corelib/kernel/qpointer/tst_qpointer.cpp | 4 +- .../qsharedpointer/tst_qsharedpointer.cpp | 2 - 11 files changed, 63 insertions(+), 154 deletions(-) diff --git a/dist/changes-5.0.0 b/dist/changes-5.0.0 index cec264337f8..7d9153d46bd 100644 --- a/dist/changes-5.0.0 +++ b/dist/changes-5.0.0 @@ -267,3 +267,21 @@ Qt for Windows CE * Important Behavior Changes * **************************************************************************** +- QPointer + + * QPointer itself is now deprecated, and the implementation of QPointer + has been changed to use QWeakPointer. The old guard mechanism has been + removed. This causes two slight changes in behavior when using + QPointer: + + * When using QPointer on a QWidget (or a subclass of QWidget), previously + the QPointer would be cleared by the QWidget destructor. Now, the QPointer + is cleared by the QObject destructor (since this is when QWeakPointers are + cleared). Any QPointers tracking a widget will NOT be cleared before the + QWidget destructor destroys the children for the widget being tracked. + + * When constructing a QSharedPointer to take ownership of an object after a + QPointer is already tracking the object. Previously, the shared pointer + construction would not be affected by the QPointer, but now that QPointer + is implemented using QWeakPoiner, constructing the QSharedPointer will + cause an abort(). diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp index f6e72e3e940..da06fc64339 100644 --- a/src/corelib/kernel/qobject.cpp +++ b/src/corelib/kernel/qobject.cpp @@ -177,7 +177,6 @@ QObjectPrivate::QObjectPrivate(int version) deleteWatch = 0; #endif metaObject = 0; - hasGuards = false; isWindow = false; } @@ -409,113 +408,6 @@ void QObjectPrivate::cleanConnectionLists() } } -typedef QMultiHash GuardHash; -Q_GLOBAL_STATIC(GuardHash, guardHash) -Q_GLOBAL_STATIC(QMutex, guardHashLock) - -/*!\internal - */ -void QMetaObject::addGuard(QObject **ptr) -{ - if (!*ptr) - return; - GuardHash *hash = guardHash(); - if (!hash) { - *ptr = 0; - return; - } - QMutexLocker locker(guardHashLock()); - QObjectPrivate::get(*ptr)->hasGuards = true; - hash->insert(*ptr, ptr); -} - -/*!\internal - */ -void QMetaObject::removeGuard(QObject **ptr) -{ - if (!*ptr) - return; - GuardHash *hash = guardHash(); - /* check that the hash is empty - otherwise we might detach - the shared_null hash, which will alloc, which is not nice */ - if (!hash || hash->isEmpty()) - return; - QMutexLocker locker(guardHashLock()); - if (!*ptr) //check again, under the lock - return; - GuardHash::iterator it = hash->find(*ptr); - const GuardHash::iterator end = hash->end(); - bool more = false; //if the QObject has more pointer attached to it. - for (; it.key() == *ptr && it != end; ++it) { - if (it.value() == ptr) { - it = hash->erase(it); - if (!more) more = (it != end && it.key() == *ptr); - break; - } - more = true; - } - if (!more) - QObjectPrivate::get(*ptr)->hasGuards = false; -} - -/*!\internal - */ -void QMetaObject::changeGuard(QObject **ptr, QObject *o) -{ - GuardHash *hash = guardHash(); - if (!hash) { - *ptr = 0; - return; - } - QMutexLocker locker(guardHashLock()); - if (o) { - hash->insert(o, ptr); - QObjectPrivate::get(o)->hasGuards = true; - } - if (*ptr) { - bool more = false; //if the QObject has more pointer attached to it. - GuardHash::iterator it = hash->find(*ptr); - const GuardHash::iterator end = hash->end(); - for (; it.key() == *ptr && it != end; ++it) { - if (it.value() == ptr) { - it = hash->erase(it); - if (!more) more = (it != end && it.key() == *ptr); - break; - } - more = true; - } - if (!more) - QObjectPrivate::get(*ptr)->hasGuards = false; - } - *ptr = o; -} - -/*! \internal - */ -void QObjectPrivate::clearGuards(QObject *object) -{ - GuardHash *hash = 0; - QMutex *mutex = 0; - QT_TRY { - hash = guardHash(); - mutex = guardHashLock(); - } QT_CATCH(const std::bad_alloc &) { - // do nothing in case of OOM - code below is safe - } - - /* check that the hash is empty - otherwise we might detach - the shared_null hash, which will alloc, which is not nice */ - if (hash && !hash->isEmpty()) { - QMutexLocker locker(mutex); - GuardHash::iterator it = hash->find(object); - const GuardHash::iterator end = hash->end(); - while (it.key() == object && it != end) { - *it.value() = 0; - it = hash->erase(it); - } - } -} - /*! \internal */ QMetaCallEvent::QMetaCallEvent(ushort method_offset, ushort method_relative, QObjectPrivate::StaticMetaCallFunction callFunction, @@ -840,12 +732,6 @@ QObject::~QObject() d->wasDeleted = true; d->blockSig = 0; // unblock signals so we always emit destroyed() - if (d->hasGuards && !d->isWidget) { - // set all QPointers for this object to zero - note that - // ~QWidget() does this for us, so we don't have to do it twice - QObjectPrivate::clearGuards(this); - } - QtSharedPointer::ExternalRefCountData *sharedRefcount = d->sharedRefcount.load(); if (sharedRefcount) { if (sharedRefcount->strongref.load() > 0) { diff --git a/src/corelib/kernel/qobject.h b/src/corelib/kernel/qobject.h index cc2ccb9e5bc..a0d8076b735 100644 --- a/src/corelib/kernel/qobject.h +++ b/src/corelib/kernel/qobject.h @@ -105,9 +105,8 @@ public: uint receiveChildEvents : 1; uint inEventHandler : 1; //only used if QT_JAMBI_BUILD uint inThreadChangeEvent : 1; - uint hasGuards : 1; //true iff there is one or more QPointer attached to this object uint isWindow : 1; //for QWindow - uint unused : 21; + uint unused : 22; int postedEvents; QMetaObject *metaObject; // assert dynamic }; diff --git a/src/corelib/kernel/qobject_p.h b/src/corelib/kernel/qobject_p.h index 5519a69c34f..a9cac3917a9 100644 --- a/src/corelib/kernel/qobject_p.h +++ b/src/corelib/kernel/qobject_p.h @@ -183,7 +183,6 @@ public: static int *setDeleteWatch(QObjectPrivate *d, int *newWatch); static void resetDeleteWatch(QObjectPrivate *d, int *oldWatch, int deleteWatch); #endif - static void clearGuards(QObject *); static QObjectPrivate *get(QObject *o) { return o->d_func(); diff --git a/src/corelib/kernel/qobjectdefs.h b/src/corelib/kernel/qobjectdefs.h index bfb6808ad52..faadb6f48c1 100644 --- a/src/corelib/kernel/qobjectdefs.h +++ b/src/corelib/kernel/qobjectdefs.h @@ -341,11 +341,6 @@ struct Q_CORE_EXPORT QMetaObject static void activate(QObject *sender, int signal_index, void **argv); static void activate(QObject *sender, const QMetaObject *, int local_signal_index, void **argv); - // internal guarded pointers - static void addGuard(QObject **ptr); - static void removeGuard(QObject **ptr); - static void changeGuard(QObject **ptr, QObject *o); - static bool invokeMethod(QObject *obj, const char *member, Qt::ConnectionType, QGenericReturnArgument ret, diff --git a/src/corelib/kernel/qpointer.cpp b/src/corelib/kernel/qpointer.cpp index c7fcf4f1e87..51c8b2ad74d 100644 --- a/src/corelib/kernel/qpointer.cpp +++ b/src/corelib/kernel/qpointer.cpp @@ -44,7 +44,7 @@ \brief The QPointer class is a template class that provides guarded pointers to QObject. \ingroup objectmodel - + \obsolete Use QWeakPointer instead. A guarded pointer, QPointer, behaves like a normal C++ pointer \c{T *}, except that it is automatically set to 0 when the @@ -57,6 +57,24 @@ destroyed while you still hold a reference to it. You can safely test the pointer for validity. + Note that Qt 5 introduces two slight changes in behavior when using QPointer. + + \list + + \i When using QPointer on a QWidget (or a subclass of QWidget), previously + the QPointer would be cleared by the QWidget destructor. Now, the QPointer + is cleared by the QObject destructor (since this is when QWeakPointers are + cleared). Any QPointers tracking a widget will \b NOT be cleared before the + QWidget destructor destroys the children for the widget being tracked. + + \i When constructing a QSharedPointer to take ownership of an object after a + QPointer is already tracking the object. Previously, the shared pointer + construction would not be affected by the QPointer, but now that QPointer + is implemented using QWeakPoiner, constructing the QSharedPointer will + cause an \c abort(). + + \endlist + Qt also provides QSharedPointer, an implementation of a reference-counted shared pointer object, which can be used to maintain a collection of references to an individual pointer. diff --git a/src/corelib/kernel/qpointer.h b/src/corelib/kernel/qpointer.h index e9f302e317b..a1d1f2037f4 100644 --- a/src/corelib/kernel/qpointer.h +++ b/src/corelib/kernel/qpointer.h @@ -42,7 +42,7 @@ #ifndef QPOINTER_H #define QPOINTER_H -#include +#include QT_BEGIN_HEADER @@ -50,34 +50,35 @@ QT_BEGIN_NAMESPACE QT_MODULE(Core) +#if QT_DEPRECATED_SINCE(5,0) + template -class QPointer +class QT_DEPRECATED QPointer { - QObject *o; + QWeakPointer wp; + public: - inline QPointer() : o(0) {} - inline QPointer(T *p) : o(p) - { QMetaObject::addGuard(&o); } - inline QPointer(const QPointer &p) : o(p.o) - { QMetaObject::addGuard(&o); } - inline ~QPointer() - { QMetaObject::removeGuard(&o); } + inline QPointer() : wp() { } + inline QPointer(T *p) : wp(p) { } + inline QPointer(const QPointer &p) : wp(p.wp) { } + inline ~QPointer() { } + inline QPointer &operator=(const QPointer &p) - { if (this != &p) QMetaObject::changeGuard(&o, p.o); return *this; } + { wp = p.wp; return *this; } inline QPointer &operator=(T* p) - { if (o != p) QMetaObject::changeGuard(&o, p); return *this; } + { wp = p; return *this; } inline bool isNull() const - { return !o; } + { return wp.isNull(); } inline T* operator->() const - { return static_cast(const_cast(o)); } + { return wp.data(); } inline T& operator*() const - { return *static_cast(const_cast(o)); } + { return *wp.data(); } inline operator T*() const - { return static_cast(const_cast(o)); } + { return wp.data(); } inline T* data() const - { return static_cast(const_cast(o)); } + { return wp.data(); } }; @@ -161,6 +162,8 @@ inline bool operator!= (int i, const QPointer &p) { Q_ASSERT(i == 0); return !i && !p.isNull(); } #endif +#endif // QT_DEPRECATED_SINCE(5,0) + QT_END_NAMESPACE QT_END_HEADER diff --git a/src/widgets/kernel/qaction.cpp b/src/widgets/kernel/qaction.cpp index 22162895f7b..15954abf8c3 100644 --- a/src/widgets/kernel/qaction.cpp +++ b/src/widgets/kernel/qaction.cpp @@ -1185,22 +1185,19 @@ void QAction::activate(ActionEvent event) { Q_D(QAction); if(event == Trigger) { - QObject *guard = this; - QMetaObject::addGuard(&guard); + QWeakPointer guard = this; if(d->checkable) { // the checked action of an exclusive group cannot be unchecked if (d->checked && (d->group && d->group->isExclusive() && d->group->checkedAction() == this)) { - if (guard) + if (!guard.isNull()) emit triggered(true); - QMetaObject::removeGuard(&guard); return; } setChecked(!d->checked); } - if (guard) + if (!guard.isNull()) emit triggered(d->checked); - QMetaObject::removeGuard(&guard); } else if(event == Hover) { emit hovered(); } diff --git a/src/widgets/kernel/qwidget.cpp b/src/widgets/kernel/qwidget.cpp index 7ec37bb929a..d20e979cfb9 100644 --- a/src/widgets/kernel/qwidget.cpp +++ b/src/widgets/kernel/qwidget.cpp @@ -1469,10 +1469,6 @@ QWidget::~QWidget() delete d->needsFlush; d->needsFlush = 0; - // set all QPointers for this object to zero - if (d->hasGuards) - QObjectPrivate::clearGuards(this); - if (d->declarativeData) { QAbstractDeclarativeData::destroyed(d->declarativeData, this); d->declarativeData = 0; // don't activate again in ~QObject diff --git a/tests/auto/corelib/kernel/qpointer/tst_qpointer.cpp b/tests/auto/corelib/kernel/qpointer/tst_qpointer.cpp index fed98ec86a8..c3888022e66 100644 --- a/tests/auto/corelib/kernel/qpointer/tst_qpointer.cpp +++ b/tests/auto/corelib/kernel/qpointer/tst_qpointer.cpp @@ -241,8 +241,8 @@ public: ChildWidget::~ChildWidget() { - QCOMPARE(static_cast(guardedPointer), static_cast(0)); - QCOMPARE(qobject_cast(guardedPointer), static_cast(0)); + QCOMPARE(static_cast(guardedPointer), parentWidget()); + QCOMPARE(qobject_cast(guardedPointer), parentWidget()); } class DerivedChild; diff --git a/tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp b/tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp index 7eb1bd2deec..649842f9460 100644 --- a/tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp +++ b/tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp @@ -767,7 +767,6 @@ void tst_QSharedPointer::objectCast() { OtherObject *data = new OtherObject; - QPointer qptr = data; QSharedPointer ptr = QSharedPointer(data); QWeakPointer weakptr = ptr; @@ -788,7 +787,6 @@ void tst_QSharedPointer::objectCast() // drop the reference: ptr.clear(); QVERIFY(ptr.isNull()); - QVERIFY(qptr.isNull()); QVERIFY(weakptr.toStrongRef().isNull()); // verify that the object casts fail without crash