From 4a702e580eec2d6efc4f80664725bd90bfcaa4e0 Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Thu, 27 Feb 2020 14:11:51 +0100 Subject: Use QTaggedPointer in QPropertyObserver This replaces the private tagged pointer and the use of enums for the tag makes the observer handling code more readable. The pointer-to-tagged-pointer class remains in qpropertyprivate.h due to its exoticness. Change-Id: Icc88799136c6839426d994b42368526463265e66 Reviewed-by: Fabian Kosmale --- src/corelib/kernel/qproperty.cpp | 59 ++++++++++++++++++----------- src/corelib/kernel/qproperty.h | 14 ++++++- src/corelib/kernel/qproperty_p.h | 5 ++- src/corelib/kernel/qpropertyprivate.h | 70 +++++++++-------------------------- src/corelib/tools/qtaggedpointer.h | 6 +-- 5 files changed, 73 insertions(+), 81 deletions(-) diff --git a/src/corelib/kernel/qproperty.cpp b/src/corelib/kernel/qproperty.cpp index ba91db3b5d..acb2de5369 100644 --- a/src/corelib/kernel/qproperty.cpp +++ b/src/corelib/kernel/qproperty.cpp @@ -51,7 +51,7 @@ QPropertyBase::QPropertyBase(QPropertyBase &&other, void *propertyDataPtr) { std::swap(d_ptr, other.d_ptr); QPropertyBasePointer d{this}; - d.firstObserverPtr().set(nullptr); + d.setFirstObserver(nullptr); if (auto binding = d.bindingPtr()) binding->propertyDataPtr = propertyDataPtr; } @@ -63,7 +63,7 @@ void QPropertyBase::moveAssign(QPropertyBase &&other, void *propertyDataPtr) QPropertyBasePointer d{this}; auto observer = d.firstObserver(); - d.firstObserverPtr().set(nullptr); + d.setFirstObserver(nullptr); if (auto binding = d.bindingPtr()) { binding->unlinkAndDeref(); @@ -75,7 +75,7 @@ void QPropertyBase::moveAssign(QPropertyBase &&other, void *propertyDataPtr) if (auto binding = d.bindingPtr()) binding->propertyDataPtr = propertyDataPtr; - d.firstObserverPtr().set(const_cast(observer.ptr)); + d.setFirstObserver(observer.ptr); // The caller will have to notify observers. } @@ -137,11 +137,31 @@ QPropertyBindingPrivate *QPropertyBasePointer::bindingPtr() const return nullptr; } -QtPrivate::QPropertyTagPreservingPointerToPointer QPropertyBasePointer::firstObserverPtr() const +void QPropertyBasePointer::addObserver(QPropertyObserver *observer) { - if (auto *binding = bindingPtr()) - return const_cast(&binding->firstObserver.ptr); - return &ptr->d_ptr; + if (auto *binding = bindingPtr()) { + observer->prev = &binding->firstObserver.ptr; + observer->next = binding->firstObserver.ptr; + if (observer->next) + observer->next->prev = &observer->next; + binding->firstObserver.ptr = observer; + } else { + auto firstObserver = reinterpret_cast(ptr->d_ptr & ~QPropertyBase::FlagMask); + observer->prev = reinterpret_cast(&ptr->d_ptr); + observer->next = firstObserver; + if (observer->next) + observer->next->prev = &observer->next; + } + setFirstObserver(observer); +} + +void QPropertyBasePointer::setFirstObserver(QPropertyObserver *observer) +{ + if (auto *binding = bindingPtr()) { + binding->firstObserver.ptr = observer; + return; + } + ptr->d_ptr = reinterpret_cast(observer) | (ptr->d_ptr & QPropertyBase::FlagMask); } QPropertyObserverPointer QPropertyBasePointer::firstObserver() const @@ -249,9 +269,9 @@ QPropertyObserver::QPropertyObserver(QPropertyObserver &&other) std::swap(next, other.next); std::swap(prev, other.prev); if (next) - next->prev = reinterpret_cast(&next); + next->prev = &next; if (prev) - prev.set(this); + prev.setPointer(this); } QPropertyObserver &QPropertyObserver::operator=(QPropertyObserver &&other) @@ -267,9 +287,9 @@ QPropertyObserver &QPropertyObserver::operator=(QPropertyObserver &&other) std::swap(next, other.next); std::swap(prev, other.prev); if (next) - next->prev = reinterpret_cast(&next); + next->prev = &next; if (prev) - prev.set(this); + prev.setPointer(this); return *this; } @@ -279,7 +299,7 @@ void QPropertyObserverPointer::unlink() if (ptr->next) ptr->next->prev = ptr->prev; if (ptr->prev) - ptr->prev.set(ptr->next.data()); + ptr->prev.setPointer(ptr->next.pointer()); ptr->next = nullptr; ptr->prev.clear(); } @@ -287,13 +307,13 @@ void QPropertyObserverPointer::unlink() void QPropertyObserverPointer::setChangeHandler(void (*changeHandler)(QPropertyObserver *)) { ptr->changeHandler = changeHandler; - ptr->next.setFlag(true); + ptr->next.setTag(QPropertyObserver::ObserverNotifiesChangeHandler); } void QPropertyObserverPointer::setBindingToMarkDirty(QPropertyBindingPrivate *binding) { ptr->bindingToMarkDirty = binding; - ptr->next.setFlag(false); + ptr->next.setTag(QPropertyObserver::ObserverNotifiesBinding); } void QPropertyObserverPointer::notify(QPropertyBindingPrivate *triggeringBinding) @@ -303,8 +323,8 @@ void QPropertyObserverPointer::notify(QPropertyBindingPrivate *triggeringBinding auto observer = const_cast(ptr); while (observer) { - auto * const next = observer->next.data(); - if (observer->next.flag()) { + auto * const next = observer->next.pointer(); + if (observer->next.tag() == QPropertyObserver::ObserverNotifiesChangeHandler) { if (!knownIfPropertyChanged && triggeringBinding) { knownIfPropertyChanged = true; @@ -328,12 +348,7 @@ void QPropertyObserverPointer::notify(QPropertyBindingPrivate *triggeringBinding void QPropertyObserverPointer::observeProperty(QPropertyBasePointer property) { unlink(); - auto firstObserverPtr = property.firstObserverPtr(); - ptr->prev = firstObserverPtr; - ptr->next = firstObserverPtr.get(); - if (ptr->next) - ptr->next->prev = &ptr->next; - firstObserverPtr.set(ptr); + property.addObserver(ptr); } void QPropertyObserverPointer::prependToBinding(QPropertyBindingPrivate *binding) diff --git a/src/corelib/kernel/qproperty.h b/src/corelib/kernel/qproperty.h index 5e1d8ec2f6..21b2ed0839 100644 --- a/src/corelib/kernel/qproperty.h +++ b/src/corelib/kernel/qproperty.h @@ -387,6 +387,13 @@ struct QPropertyObserverPointer; struct Q_CORE_EXPORT QPropertyObserver { + // Internal + enum ObserverTag { + ObserverNotifiesBinding = 0x0, + ObserverNotifiesChangeHandler = 0x1, + }; + Q_DECLARE_FLAGS(ObserverTags, ObserverTag) + QPropertyObserver() = default; QPropertyObserver(QPropertyObserver &&other); QPropertyObserver &operator=(QPropertyObserver &&other); @@ -402,10 +409,10 @@ protected: private: void setSource(QtPrivate::QPropertyBase &property); - QtPrivate::QTaggedPointer next; + QTaggedPointer next; // prev is a pointer to the "next" element within the previous node, or to the "firstObserverPtr" if it is the // first node. - QtPrivate::QPropertyTagPreservingPointerToPointer prev; + QtPrivate::QTagPreservingPointerToPointer prev; union { QPropertyBindingPrivate *bindingToMarkDirty = nullptr; @@ -416,8 +423,11 @@ private: QPropertyObserver &operator=(const QPropertyObserver &) = delete; friend struct QPropertyObserverPointer; + friend struct QPropertyBasePointer; }; +Q_DECLARE_OPERATORS_FOR_FLAGS(QPropertyObserver::ObserverTags) + template class QPropertyChangeHandler : public QPropertyObserver { diff --git a/src/corelib/kernel/qproperty_p.h b/src/corelib/kernel/qproperty_p.h index 1b96e09d58..8815e66b69 100644 --- a/src/corelib/kernel/qproperty_p.h +++ b/src/corelib/kernel/qproperty_p.h @@ -65,7 +65,8 @@ struct Q_AUTOTEST_EXPORT QPropertyBasePointer QPropertyBindingPrivate *bindingPtr() const; - QtPrivate::QPropertyTagPreservingPointerToPointer firstObserverPtr() const; + void addObserver(QPropertyObserver *observer); + void setFirstObserver(QPropertyObserver *observer); QPropertyObserverPointer firstObserver() const; int observerCount() const; @@ -93,7 +94,7 @@ struct QPropertyObserverPointer explicit operator bool() const { return ptr != nullptr; } - QPropertyObserverPointer nextObserver() const { return {ptr->next.data()}; } + QPropertyObserverPointer nextObserver() const { return {ptr->next.pointer()}; } }; class QPropertyBindingErrorPrivate : public QSharedData diff --git a/src/corelib/kernel/qpropertyprivate.h b/src/corelib/kernel/qpropertyprivate.h index 6d4a729845..22c5416197 100644 --- a/src/corelib/kernel/qpropertyprivate.h +++ b/src/corelib/kernel/qpropertyprivate.h @@ -53,6 +53,7 @@ #include #include +#include QT_BEGIN_NAMESPACE @@ -161,85 +162,50 @@ struct QPropertyValueStorage } }; -template class QTaggedPointer; - -template -class QPropertyTagPreservingPointerToPointer +template +class QTagPreservingPointerToPointer { - quintptr *data = nullptr; - public: - QPropertyTagPreservingPointerToPointer() = default; - QPropertyTagPreservingPointerToPointer(T **ptr) - : data(reinterpret_cast(ptr)) - {} - QPropertyTagPreservingPointerToPointer(quintptr *ptr) - : data(ptr) + QTagPreservingPointerToPointer() = default; + + QTagPreservingPointerToPointer(T **ptr) + : d(reinterpret_cast(ptr)) {} - QPropertyTagPreservingPointerToPointer &operator=(T **ptr) + QTagPreservingPointerToPointer &operator=(T **ptr) { - data = reinterpret_cast(ptr); + d = reinterpret_cast(ptr); return *this; } - QPropertyTagPreservingPointerToPointer &operator=(QTaggedPointer *ptr) + QTagPreservingPointerToPointer &operator=(QTaggedPointer *ptr) { - data = reinterpret_cast(ptr); + d = reinterpret_cast(ptr); return *this; } void clear() { - data = nullptr; + d = nullptr; } - void set(T *ptr) + void setPointer(T *ptr) { - *data = (*data & Mask) | (reinterpret_cast(ptr) & ~Mask); + *d = (reinterpret_cast(ptr) & QTaggedPointer::pointerMask()) | (*d & QTaggedPointer::tagMask()); } T *get() const { - return reinterpret_cast(*data & ~Mask); + return reinterpret_cast(*d & QTaggedPointer::pointerMask()); } - explicit operator bool() const { return data != nullptr; } -}; - -template -class QTaggedPointer -{ -public: - QTaggedPointer() = default; - QTaggedPointer(T *ptr) : tagAndPointer(reinterpret_cast(ptr)) {} - - void setFlag(bool b) + explicit operator bool() const { - if (b) - tagAndPointer |= Flag1Mask; - else - tagAndPointer &= ~Flag1Mask; + return d != nullptr; } - bool flag() const { return tagAndPointer & Flag1Mask; } - - QTaggedPointer &operator=(T *ptr) - { - quintptr tag = tagAndPointer & TagMask; - tagAndPointer = reinterpret_cast(ptr) | tag; - return *this; - } - - T *data() const { return reinterpret_cast(tagAndPointer & ~TagMask); } - - explicit operator bool() const { return tagAndPointer & ~TagMask; } - T *operator->() const { return data(); } - private: - quintptr tagAndPointer = 0; - static const quintptr Flag1Mask = 0x1; - static const quintptr TagMask = 0x3; + quintptr *d = nullptr; }; } // namespace QtPrivate diff --git a/src/corelib/tools/qtaggedpointer.h b/src/corelib/tools/qtaggedpointer.h index 6fa1e67acb..2d4d688824 100644 --- a/src/corelib/tools/qtaggedpointer.h +++ b/src/corelib/tools/qtaggedpointer.h @@ -73,13 +73,13 @@ namespace QtPrivate { template ::TagType> class QTaggedPointer { - static constexpr quintptr tagMask() { return QtPrivate::TagInfo::alignment - 1; } - static constexpr quintptr pointerMask() { return ~tagMask(); } - public: using Type = T; using TagType = Tag; + static constexpr quintptr tagMask() { return QtPrivate::TagInfo::alignment - 1; } + static constexpr quintptr pointerMask() { return ~tagMask(); } + explicit QTaggedPointer(Type *pointer = nullptr, TagType tag = TagType()) noexcept : d(quintptr(pointer)) { -- cgit v1.2.3