diff --git a/src/corelib/kernel/qproperty.cpp b/src/corelib/kernel/qproperty.cpp index b55069d1ae..103c3d8482 100644 --- a/src/corelib/kernel/qproperty.cpp +++ b/src/corelib/kernel/qproperty.cpp @@ -47,6 +47,24 @@ QT_BEGIN_NAMESPACE using namespace QtPrivate; +QPropertyBindingPrivatePtr::~QPropertyBindingPrivatePtr() +{ + if (d && (--d->ref == 0)) + QPropertyBindingPrivate::destroyAndFreeMemory(static_cast(d)); +} + +void QPropertyBindingPrivatePtr::reset(QtPrivate::RefCounted *ptr) noexcept +{ + if (ptr != d) { + if (ptr) + ptr->ref++; + auto *old = qExchange(d, ptr); + if (old && (--old->ref == 0)) + QPropertyBindingPrivate::destroyAndFreeMemory(static_cast(d)); + } +} + + void QPropertyBindingDataPointer::addObserver(QPropertyObserver *observer) { if (auto *binding = bindingPtr()) { @@ -69,13 +87,15 @@ QPropertyBindingPrivate::~QPropertyBindingPrivate() { if (firstObserver) firstObserver.unlink(); + if (vtable->size) + vtable->destroy(reinterpret_cast(this) + sizeof(QPropertyBindingPrivate)); } void QPropertyBindingPrivate::unlinkAndDeref() { propertyDataPtr = nullptr; - if (!ref.deref()) - delete this; + if (--ref == 0) + destroyAndFreeMemory(this); } void QPropertyBindingPrivate::markDirtyAndNotifyObservers() @@ -129,9 +149,9 @@ bool QPropertyBindingPrivate::evaluateIfDirtyAndReturnTrueIfValueChanged(const Q QUntypedPropertyData *mutable_data = const_cast(data); if (hasBindingWrapper) { - changed = staticBindingWrapper(metaType, mutable_data, evaluationFunction); + changed = staticBindingWrapper(metaType, mutable_data, {vtable, reinterpret_cast(this)+QPropertyBindingPrivate::getSizeEnsuringAlignment()}); } else { - changed = evaluationFunction(metaType, mutable_data); + changed = vtable->call(metaType, mutable_data, reinterpret_cast(this)+ QPropertyBindingPrivate::getSizeEnsuringAlignment()); } dirty = false; @@ -140,10 +160,12 @@ bool QPropertyBindingPrivate::evaluateIfDirtyAndReturnTrueIfValueChanged(const Q QUntypedPropertyBinding::QUntypedPropertyBinding() = default; -QUntypedPropertyBinding::QUntypedPropertyBinding(QMetaType metaType, QUntypedPropertyBinding::BindingEvaluationFunction function, +QUntypedPropertyBinding::QUntypedPropertyBinding(QMetaType metaType, const BindingFunctionVTable *vtable, void *function, const QPropertyBindingSourceLocation &location) - : d(new QPropertyBindingPrivate(metaType, std::move(function), std::move(location))) { + std::byte *mem = new std::byte[QPropertyBindingPrivate::getSizeEnsuringAlignment() + vtable->size](); + d = new(mem) QPropertyBindingPrivate(metaType, vtable, std::move(location)); + vtable->moveConstruct(mem+sizeof(QPropertyBindingPrivate), function); } QUntypedPropertyBinding::QUntypedPropertyBinding(QUntypedPropertyBinding &&other) @@ -186,14 +208,14 @@ QPropertyBindingError QUntypedPropertyBinding::error() const { if (!d) return QPropertyBindingError(); - return d->bindingError(); + return static_cast(d.get())->bindingError(); } QMetaType QUntypedPropertyBinding::valueMetaType() const { if (!d) return QMetaType(); - return d->valueMetaType(); + return static_cast(d.get())->valueMetaType(); } QPropertyBindingData::~QPropertyBindingData() @@ -221,28 +243,29 @@ QUntypedPropertyBinding QPropertyBindingData::setBinding(const QUntypedPropertyB if (auto *existingBinding = d.bindingPtr()) { if (existingBinding == newBinding.data()) - return QUntypedPropertyBinding(oldBinding.data()); + return QUntypedPropertyBinding(static_cast(oldBinding.data())); oldBinding = QPropertyBindingPrivatePtr(existingBinding); - observer = oldBinding->takeObservers(); - oldBinding->unlinkAndDeref(); + observer = static_cast(oldBinding.data())->takeObservers(); + static_cast(oldBinding.data())->unlinkAndDeref(); d_ptr &= FlagMask; } else { observer = d.firstObserver(); } if (newBinding) { - newBinding.data()->ref.ref(); + newBinding.data()->addRef(); d_ptr = (d_ptr & FlagMask) | reinterpret_cast(newBinding.data()); d_ptr |= BindingBit; - newBinding->setDirty(true); - newBinding->setProperty(propertyDataPtr); + auto newBindingRaw = static_cast(newBinding.data()); + newBindingRaw->setDirty(true); + newBindingRaw->setProperty(propertyDataPtr); if (observer) - newBinding->prependObserver(observer); - newBinding->setStaticObserver(staticObserverCallback, guardCallback); - if (newBinding->requiresEagerEvaluation()) { - auto changed = newBinding->evaluateIfDirtyAndReturnTrueIfValueChanged(propertyDataPtr); + newBindingRaw->prependObserver(observer); + newBindingRaw->setStaticObserver(staticObserverCallback, guardCallback); + if (newBindingRaw->requiresEagerEvaluation()) { + auto changed = newBindingRaw->evaluateIfDirtyAndReturnTrueIfValueChanged(propertyDataPtr); if (changed) - observer.notify(newBinding.data(), propertyDataPtr, /*alreadyKnownToHaveChanged=*/true); + observer.notify(newBindingRaw, propertyDataPtr, /*alreadyKnownToHaveChanged=*/true); } } else if (observer) { d.setObservers(observer.ptr); @@ -251,9 +274,9 @@ QUntypedPropertyBinding QPropertyBindingData::setBinding(const QUntypedPropertyB } if (oldBinding) - oldBinding->detachFromProperty(); + static_cast(oldBinding.data())->detachFromProperty(); - return QUntypedPropertyBinding(oldBinding.data()); + return QUntypedPropertyBinding(static_cast(oldBinding.data())); } QPropertyBindingData::QPropertyBindingData(QPropertyBindingData &&other) : d_ptr(std::exchange(other.d_ptr, 0)) diff --git a/src/corelib/kernel/qproperty.h b/src/corelib/kernel/qproperty.h index 8b400e7285..d612486e2c 100644 --- a/src/corelib/kernel/qproperty.h +++ b/src/corelib/kernel/qproperty.h @@ -141,10 +141,16 @@ class Q_CORE_EXPORT QUntypedPropertyBinding { public: // writes binding result into dataPtr - using BindingEvaluationFunction = QtPrivate::QPropertyBindingFunction; + using BindingFunctionVTable = QtPrivate::BindingFunctionVTable; QUntypedPropertyBinding(); - QUntypedPropertyBinding(QMetaType metaType, BindingEvaluationFunction function, const QPropertyBindingSourceLocation &location); + QUntypedPropertyBinding(QMetaType metaType, const BindingFunctionVTable *vtable, void *function, const QPropertyBindingSourceLocation &location); + + template + QUntypedPropertyBinding(QMetaType metaType, Functor &&f, const QPropertyBindingSourceLocation &location) + : QUntypedPropertyBinding(metaType, &QtPrivate::bindingFunctionVTable>, &f, location) + {} + QUntypedPropertyBinding(QUntypedPropertyBinding &&other); QUntypedPropertyBinding(const QUntypedPropertyBinding &other); QUntypedPropertyBinding &operator=(const QUntypedPropertyBinding &other); @@ -168,29 +174,13 @@ private: template class QPropertyBinding : public QUntypedPropertyBinding { - template - struct BindingAdaptor - { - Functor impl; - bool operator()(QMetaType /*metaType*/, QUntypedPropertyData *dataPtr) - { - QPropertyData *propertyPtr = static_cast *>(dataPtr); - PropertyType newValue = impl(); - if constexpr (QTypeTraits::has_operator_equal_v) { - if (newValue == propertyPtr->valueBypassingBindings()) - return false; - } - propertyPtr->setValueBypassingBindings(std::move(newValue)); - return true; - } - }; public: QPropertyBinding() = default; template QPropertyBinding(Functor &&f, const QPropertyBindingSourceLocation &location) - : QUntypedPropertyBinding(QMetaType::fromType(), BindingAdaptor{std::forward(f)}, location) + : QUntypedPropertyBinding(QMetaType::fromType(), &QtPrivate::bindingFunctionVTable, PropertyType>, &f, location) {} template diff --git a/src/corelib/kernel/qproperty_p.h b/src/corelib/kernel/qproperty_p.h index fdb5f14ccd..99a145c65a 100644 --- a/src/corelib/kernel/qproperty_p.h +++ b/src/corelib/kernel/qproperty_p.h @@ -54,11 +54,9 @@ #include #include -#include #include #include - QT_BEGIN_NAMESPACE // Keep all classes related to QProperty in one compilation unit. Performance of this code is crucial and @@ -156,14 +154,15 @@ struct QBindingStatus QtPrivate::CurrentCompatProperty *currentCompatProperty = nullptr; }; -class Q_CORE_EXPORT QPropertyBindingPrivate : public QSharedData +class Q_CORE_EXPORT QPropertyBindingPrivate : public QtPrivate::RefCounted { private: friend struct QPropertyBindingDataPointer; + friend class QPropertyBindingPrivatePtr; using ObserverArray = std::array; - // QSharedData is 4 bytes. Use the padding for the bools as we need 8 byte alignment below. +private: // a dependent property has changed, and the binding needs to be reevaluated on access bool dirty = false; @@ -174,7 +173,7 @@ private: // used to detect binding loops for eagerly evaluated properties bool eagerlyUpdating:1; - QUntypedPropertyBinding::BindingEvaluationFunction evaluationFunction; + const QtPrivate::BindingFunctionVTable *vtable; union { QtPrivate::QPropertyObserverCallback staticObserverCallback = nullptr; @@ -193,19 +192,29 @@ private: QMetaType metaType; public: + static constexpr size_t getSizeEnsuringAlignment() { + constexpr auto align = alignof (std::max_align_t) - 1; + constexpr size_t sizeEnsuringAlignment = (sizeof(QPropertyBindingPrivate) + align) & ~align; + static_assert (sizeEnsuringAlignment % alignof (std::max_align_t) == 0, + "Required for placement new'ing the function behind it."); + return sizeEnsuringAlignment; + } + + // public because the auto-tests access it, too. size_t dependencyObserverCount = 0; - QPropertyBindingPrivate(QMetaType metaType, QUntypedPropertyBinding::BindingEvaluationFunction evaluationFunction, + QPropertyBindingPrivate(QMetaType metaType, const QtPrivate::BindingFunctionVTable *vtable, const QPropertyBindingSourceLocation &location) : hasBindingWrapper(false) , eagerlyUpdating(false) - , evaluationFunction(std::move(evaluationFunction)) + , vtable(vtable) , inlineDependencyObservers() // Explicit initialization required because of union , location(location) , metaType(metaType) {} - virtual ~QPropertyBindingPrivate(); + ~QPropertyBindingPrivate(); + void setDirty(bool d) { dirty = d; } void setProperty(QUntypedPropertyData *propertyPtr) { propertyDataPtr = propertyPtr; } @@ -277,7 +286,7 @@ public: bool evaluateIfDirtyAndReturnTrueIfValueChanged(const QUntypedPropertyData *data); static QPropertyBindingPrivate *get(const QUntypedPropertyBinding &binding) - { return binding.d.data(); } + { return static_cast(binding.d.data()); } void setError(QPropertyBindingError &&e) { error = std::move(e); } @@ -293,6 +302,17 @@ public: bool requiresEagerEvaluation() const { return hasBindingWrapper; } static QPropertyBindingPrivate *currentlyEvaluatingBinding(); + + static void destroyAndFreeMemory(QPropertyBindingPrivate *priv) { + if (priv->vtable->size == 0) { + // special hack for QQmlPropertyBinding which has a + // different memory layout than normal QPropertyBindings + priv->vtable->destroy(priv); + } else{ + priv->~QPropertyBindingPrivate(); + delete[] reinterpret_cast(priv); + } + } }; inline void QPropertyBindingDataPointer::setFirstObserver(QPropertyObserver *observer) @@ -337,11 +357,11 @@ class QObjectCompatProperty : public QPropertyData char *that = const_cast(reinterpret_cast(this)); return reinterpret_cast(that - QtPrivate::detail::getOffset(Offset)); } - static bool bindingWrapper(QMetaType type, QUntypedPropertyData *dataPtr, const QtPrivate::QPropertyBindingFunction &binding) + static bool bindingWrapper(QMetaType type, QUntypedPropertyData *dataPtr, QtPrivate::QPropertyBindingFunction binding) { auto *thisData = static_cast(dataPtr); QPropertyData copy; - binding(type, ©); + binding.vtable->call(type, ©, binding.functor); if constexpr (QTypeTraits::has_operator_equal_v) if (copy.valueBypassingBindings() == thisData->valueBypassingBindings()) return false; diff --git a/src/corelib/kernel/qpropertyprivate.h b/src/corelib/kernel/qpropertyprivate.h index a0dfbd97c9..df12c1fdb3 100644 --- a/src/corelib/kernel/qpropertyprivate.h +++ b/src/corelib/kernel/qpropertyprivate.h @@ -60,9 +60,87 @@ QT_BEGIN_NAMESPACE +namespace QtPrivate { +// QPropertyBindingPrivatePtr operates on a RefCountingMixin solely so that we can inline +// the constructor and copy constructor +struct RefCounted { + int ref = 0; + void addRef() {++ref;} + bool deref() {--ref; return ref;} +}; +} + +class QPropertyBindingPrivate; +class QPropertyBindingPrivatePtr +{ +public: + using T = QtPrivate::RefCounted; + T &operator*() const { return *d; } + T *operator->() noexcept { return d; } + T *operator->() const noexcept { return d; } + explicit operator T *() { return d; } + explicit operator const T *() const noexcept { return d; } + T *data() const noexcept { return d; } + T *get() const noexcept { return d; } + const T *constData() const noexcept { return d; } + T *take() noexcept { T *x = d; d = nullptr; return x; } + + QPropertyBindingPrivatePtr() noexcept : d(nullptr) { } + Q_CORE_EXPORT ~QPropertyBindingPrivatePtr(); + + explicit QPropertyBindingPrivatePtr(T *data) noexcept : d(data) { if (d) d->addRef(); } + QPropertyBindingPrivatePtr(const QPropertyBindingPrivatePtr &o) noexcept + : d(o.d) { if (d) d->addRef(); } + + void reset(T *ptr = nullptr) noexcept; + + QPropertyBindingPrivatePtr &operator=(const QPropertyBindingPrivatePtr &o) noexcept + { + reset(o.d); + return *this; + } + QPropertyBindingPrivatePtr &operator=(T *o) noexcept + { + reset(o); + return *this; + } + QPropertyBindingPrivatePtr(QPropertyBindingPrivatePtr &&o) noexcept : d(qExchange(o.d, nullptr)) {} + QT_MOVE_ASSIGNMENT_OPERATOR_IMPL_VIA_MOVE_AND_SWAP(QPropertyBindingPrivatePtr) + + operator bool () const noexcept { return d != nullptr; } + bool operator!() const noexcept { return d == nullptr; } + + void swap(QPropertyBindingPrivatePtr &other) noexcept + { qSwap(d, other.d); } + + friend bool operator==(const QPropertyBindingPrivatePtr &p1, const QPropertyBindingPrivatePtr &p2) noexcept + { return p1.d == p2.d; } + friend bool operator!=(const QPropertyBindingPrivatePtr &p1, const QPropertyBindingPrivatePtr &p2) noexcept + { return p1.d != p2.d; } + friend bool operator==(const QPropertyBindingPrivatePtr &p1, const T *ptr) noexcept + { return p1.d == ptr; } + friend bool operator!=(const QPropertyBindingPrivatePtr &p1, const T *ptr) noexcept + { return p1.d != ptr; } + friend bool operator==(const T *ptr, const QPropertyBindingPrivatePtr &p2) noexcept + { return ptr == p2.d; } + friend bool operator!=(const T *ptr, const QPropertyBindingPrivatePtr &p2) noexcept + { return ptr != p2.d; } + friend bool operator==(const QPropertyBindingPrivatePtr &p1, std::nullptr_t) noexcept + { return !p1; } + friend bool operator!=(const QPropertyBindingPrivatePtr &p1, std::nullptr_t) noexcept + { return p1; } + friend bool operator==(std::nullptr_t, const QPropertyBindingPrivatePtr &p2) noexcept + { return !p2; } + friend bool operator!=(std::nullptr_t, const QPropertyBindingPrivatePtr &p2) noexcept + { return p2; } + +private: + QtPrivate::RefCounted *d; +}; + + class QUntypedPropertyBinding; class QPropertyBindingPrivate; -using QPropertyBindingPrivatePtr = QExplicitlySharedDataPointer; struct QPropertyBindingDataPointer; class QUntypedPropertyData @@ -72,12 +150,66 @@ public: struct InheritsQUntypedPropertyData {}; }; +template +class QPropertyData; + namespace QtPrivate { +struct BindingFunctionVTable +{ + using CallFn = bool(*)(QMetaType, QUntypedPropertyData *, void *); + using DtorFn = void(*)(void *); + using MoveCtrFn = void(*)(void *, void *); + const CallFn call; + const DtorFn destroy; + const MoveCtrFn moveConstruct; + const qsizetype size; + + template + static constexpr BindingFunctionVTable createFor() + { + static_assert (alignof(Callable) <= alignof(std::max_align_t), "Bindings do not support overaligned functors!"); + return { + /*call=*/[](QMetaType metaType, QUntypedPropertyData *dataPtr, void *f){ + if constexpr (!std::is_invocable_v) { + // we got an untyped callable + static_assert (std::is_invocable_r_v ); + auto untypedEvaluationFunction = static_cast(f); + return std::invoke(*untypedEvaluationFunction, metaType, dataPtr); + } else { + QPropertyData *propertyPtr = static_cast *>(dataPtr); + // That is allowed by POSIX even if Callable is a function pointer + auto evaluationFunction = static_cast(f); + PropertyType newValue = std::invoke(*evaluationFunction); + if constexpr (QTypeTraits::has_operator_equal_v) { + if (newValue == propertyPtr->valueBypassingBindings()) + return false; + } + propertyPtr->setValueBypassingBindings(std::move(newValue)); + return true; + } + }, + /*destroy*/[](void *f){ static_cast(f)->~Callable(); }, + /*moveConstruct*/[](void *addr, void *other){ + new (addr) Callable(std::move(*static_cast(other))); + }, + /*size*/sizeof(Callable) + }; + } +}; + +template +inline constexpr BindingFunctionVTable bindingFunctionVTable = BindingFunctionVTable::createFor(); + + // writes binding result into dataPtr -using QPropertyBindingFunction = std::function; +struct QPropertyBindingFunction { + const QtPrivate::BindingFunctionVTable *vtable; + void *functor; +}; + using QPropertyObserverCallback = void (*)(QUntypedPropertyData *); -using QPropertyBindingWrapper = bool(*)(QMetaType, QUntypedPropertyData *dataPtr, const QPropertyBindingFunction &); +using QPropertyBindingWrapper = bool(*)(QMetaType, QUntypedPropertyData *dataPtr, QPropertyBindingFunction); class Q_CORE_EXPORT QPropertyBindingData { diff --git a/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp b/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp index 21e0955ba5..472ba6031f 100644 --- a/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp +++ b/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp @@ -33,6 +33,13 @@ using namespace QtPrivate; + +struct DtorCounter { + static inline int counter = 0; + bool shouldIncrement = false; + ~DtorCounter() {if (shouldIncrement) ++counter;} +}; + class tst_QProperty : public QObject { Q_OBJECT @@ -43,6 +50,7 @@ private slots: void bindingWithDeletedDependency(); void recursiveDependency(); void bindingAfterUse(); + void bindingFunctionDtorCalled(); void switchBinding(); void avoidDependencyAllocationAfterFirstEval(); void boolProperty(); @@ -201,6 +209,20 @@ void tst_QProperty::bindingAfterUse() QCOMPARE(QPropertyBindingDataPointer::get(propWithBindingLater).observerCount(), 1); } +void tst_QProperty::bindingFunctionDtorCalled() +{ + DtorCounter dc; + { + QProperty prop; + prop.setBinding([dc]() mutable { + dc.shouldIncrement = true; + return 42; + }); + QCOMPARE(prop.value(), 42); + } + QCOMPARE(DtorCounter::counter, 1); +} + void tst_QProperty::switchBinding() { QProperty first(1);