From 0c89721716d9be43675358cbdb9c3170fd5936af Mon Sep 17 00:00:00 2001 From: Lars Knoll Date: Wed, 12 Aug 2020 12:23:16 +0200 Subject: Get rid of QPropertyValueStorage This simplifies and cleans up the code. Change-Id: Ic811925d644466ff298f1109efcda0537e52ce0d Reviewed-by: Fabian Kosmale --- src/corelib/kernel/qproperty.h | 124 ++++++++++++++++++++-------------- src/corelib/kernel/qpropertyprivate.h | 38 ----------- 2 files changed, 72 insertions(+), 90 deletions(-) diff --git a/src/corelib/kernel/qproperty.h b/src/corelib/kernel/qproperty.h index ff61596fb6..e120e834b2 100644 --- a/src/corelib/kernel/qproperty.h +++ b/src/corelib/kernel/qproperty.h @@ -192,9 +192,19 @@ struct QPropertyBasePointer; template class QProperty { + T val = T(); + QtPrivate::QPropertyBase d; + bool is_equal(const T &v) + { + if constexpr (QTypeTraits::has_operator_equal_v) { + if (v == val) + return true; + } + return false; + } + class DisableRValueRefs {}; static constexpr bool UseReferences = !(std::is_arithmetic_v || std::is_enum_v || std::is_pointer_v); - public: using value_type = T; using parameter_type = std::conditional_t; @@ -203,10 +213,10 @@ public: std::conditional_t, const T &, void>>; QProperty() = default; - explicit QProperty(const T &initialValue) : d(initialValue) {} - explicit QProperty(T &&initialValue) : d(std::move(initialValue)) {} - QProperty(QProperty &&other) : d(std::move(other.d)) { notify(); } - QProperty &operator=(QProperty &&other) { d = std::move(other.d); notify(); return *this; } + explicit QProperty(const T &initialValue) : val(initialValue) {} + explicit QProperty(T &&initialValue) : val(std::move(initialValue)) {} + QProperty(QProperty &&other) : val(std::move(other.val)), d(std::move(other.d), &val) { notify(); } + QProperty &operator=(QProperty &&other) { val = std::move(other.val); d.moveAssign(std::move(other.d), &val); notify(); return *this; } QProperty(const QPropertyBinding &binding) : QProperty() { operator=(binding); } @@ -227,10 +237,10 @@ public: parameter_type value() const { - if (d.priv.hasBinding()) - d.priv.evaluateIfDirty(); - d.priv.registerWithCurrentlyEvaluatingBinding(); - return d.getValue(); + if (d.hasBinding()) + d.evaluateIfDirty(); + d.registerWithCurrentlyEvaluatingBinding(); + return val; } arrow_operator_result operator->() const @@ -257,16 +267,20 @@ public: void setValue(rvalue_ref newValue) { - d.priv.removeBinding(); - if (d.setValueAndReturnTrueIfChanged(std::move(newValue))) - notify(); + d.removeBinding(); + if (is_equal(newValue)) + return; + val = std::move(newValue); + notify(); } void setValue(parameter_type newValue) { - d.priv.removeBinding(); - if (d.setValueAndReturnTrueIfChanged(newValue)) - notify(); + d.removeBinding(); + if (is_equal(newValue)) + return; + val = newValue; + notify(); } QProperty &operator=(rvalue_ref newValue) @@ -289,7 +303,7 @@ public: QPropertyBinding setBinding(const QPropertyBinding &newBinding) { - QPropertyBinding oldBinding(d.priv.setBinding(newBinding, &d)); + QPropertyBinding oldBinding(d.setBinding(newBinding, &val)); notify(); return oldBinding; } @@ -315,7 +329,7 @@ public: QPropertyBinding setBinding(Functor f); #endif - bool hasBinding() const { return d.priv.hasBinding(); } + bool hasBinding() const { return d.hasBinding(); } QPropertyBinding binding() const { @@ -324,7 +338,7 @@ public: QPropertyBinding takeBinding() { - return QPropertyBinding(d.priv.setBinding(QUntypedPropertyBinding(), &d)); + return QPropertyBinding(d.setBinding(QUntypedPropertyBinding(), &d)); } template @@ -332,18 +346,14 @@ public: template QPropertyChangeHandler subscribe(Functor f); - const QtPrivate::QPropertyBase &propertyBase() const { return d.priv; } + const QtPrivate::QPropertyBase &propertyBase() const { return d; } private: void notify() { - d.priv.notifyObservers(&d); + d.notifyObservers(&d); } Q_DISABLE_COPY(QProperty) - - // Mutable because querying for the value may require evalating the binding expression, calling - // non-const functions on QPropertyBase. - mutable QtPrivate::QPropertyValueStorage d; }; namespace Qt { @@ -359,6 +369,17 @@ namespace Qt { template class QNotifiedProperty { + T val = T(); + QtPrivate::QPropertyBase d; + bool is_equal(const T &v) + { + if constexpr (QTypeTraits::has_operator_equal_v) { + if (v == val) + return true; + } + return false; + } + public: using value_type = T; using Class = typename QtPrivate::detail::ExtractClassFromFunctionPointer::Class; @@ -380,8 +401,8 @@ public: QNotifiedProperty() = default; - explicit QNotifiedProperty(const T &initialValue) : d(initialValue) {} - explicit QNotifiedProperty(T &&initialValue) : d(std::move(initialValue)) {} + explicit QNotifiedProperty(const T &initialValue) : val(initialValue) {} + explicit QNotifiedProperty(T &&initialValue) : val(std::move(initialValue)) {} QNotifiedProperty(Class *owner, const QPropertyBinding &binding) : QNotifiedProperty() @@ -405,10 +426,10 @@ public: T value() const { - if (d.priv.hasBinding()) - d.priv.evaluateIfDirty(); - d.priv.registerWithCurrentlyEvaluatingBinding(); - return d.getValue(); + if (d.hasBinding()) + d.evaluateIfDirty(); + d.registerWithCurrentlyEvaluatingBinding(); + return val; } operator T() const @@ -423,15 +444,17 @@ public: if (!(owner->*ValueGuard)(newValue)) return; } + if (is_equal(newValue)) + return; if constexpr (CallbackAcceptsOldValue) { - T oldValue = value(); // TODO: kind of pointless if there was no change - if (d.setValueAndReturnTrueIfChanged(std::move(newValue))) - notify(owner, &oldValue); + T oldValue = value(); + val = std::move(newValue); + notify(owner, &oldValue); } else { - if (d.setValueAndReturnTrueIfChanged(std::move(newValue))) - notify(owner); + val = std::move(newValue); + notify(owner); } - d.priv.removeBinding(); + d.removeBinding(); } void setValue(Class *owner, std::conditional_t newValue) @@ -440,29 +463,30 @@ public: if (!(owner->*ValueGuard)(newValue)) return; } + if (is_equal(newValue)) + return; if constexpr (CallbackAcceptsOldValue) { - // When newValue is T, we move it, if it's const T& it stays const T& and won't get moved T oldValue = value(); - if (d.setValueAndReturnTrueIfChanged(std::move(newValue))) - notify(owner, &oldValue); + val = newValue; + notify(owner, &oldValue); } else { - if (d.setValueAndReturnTrueIfChanged(std::move(newValue))) - notify(owner); + val = newValue; + notify(owner); } - d.priv.removeBinding(); + d.removeBinding(); } QPropertyBinding setBinding(Class *owner, const QPropertyBinding &newBinding) { if constexpr (CallbackAcceptsOldValue) { T oldValue = value(); - QPropertyBinding oldBinding(d.priv.setBinding(newBinding, &d, owner, [](void *o, void *oldVal) { + QPropertyBinding oldBinding(d.setBinding(newBinding, &val, owner, [](void *o, void *oldVal) { (reinterpret_cast(o)->*Callback)(*reinterpret_cast(oldVal)); }, GuardTE)); notify(owner, &oldValue); return oldBinding; } else { - QPropertyBinding oldBinding(d.priv.setBinding(newBinding, &d, owner, [](void *o, void *) { + QPropertyBinding oldBinding(d.setBinding(newBinding, &val, owner, [](void *o, void *) { (reinterpret_cast(o)->*Callback)(); }, GuardTE)); notify(owner); @@ -491,7 +515,7 @@ public: QPropertyBinding setBinding(Class *owner, Functor f); #endif - bool hasBinding() const { return d.priv.hasBinding(); } + bool hasBinding() const { return d.hasBinding(); } QPropertyBinding binding() const { @@ -500,7 +524,7 @@ public: QPropertyBinding takeBinding() { - return QPropertyBinding(d.priv.setBinding(QUntypedPropertyBinding(), &d)); + return QPropertyBinding(d.setBinding(QUntypedPropertyBinding(), &d)); } template @@ -508,11 +532,11 @@ public: template QPropertyChangeHandler subscribe(Functor f); - const QtPrivate::QPropertyBase &propertyBase() const { return d.priv; } + const QtPrivate::QPropertyBase &propertyBase() const { return d; } private: void notify(Class *owner, T *oldValue=nullptr) { - d.priv.notifyObservers(&d); + d.notifyObservers(&d); if constexpr (std::is_invocable_v) { Q_UNUSED(oldValue); (owner->*Callback)(); @@ -522,10 +546,6 @@ private: } Q_DISABLE_COPY_MOVE(QNotifiedProperty) - - // Mutable because querying for the value may require evalating the binding expression, calling - // non-const functions on QPropertyBase. - mutable QtPrivate::QPropertyValueStorage d; }; struct QPropertyObserverPrivate; diff --git a/src/corelib/kernel/qpropertyprivate.h b/src/corelib/kernel/qpropertyprivate.h index fe6895d953..acdf405648 100644 --- a/src/corelib/kernel/qpropertyprivate.h +++ b/src/corelib/kernel/qpropertyprivate.h @@ -119,44 +119,6 @@ public: static const quintptr FlagMask = BindingBit | ExtraBit; }; -template -struct QPropertyValueStorage -{ -private: - T value; -public: - QPropertyBase priv; - - QPropertyValueStorage() : value() {} - Q_DISABLE_COPY(QPropertyValueStorage) - explicit QPropertyValueStorage(const T &initialValue) : value(initialValue) {} - QPropertyValueStorage &operator=(const T &newValue) { value = newValue; return *this; } - explicit QPropertyValueStorage(T &&initialValue) : value(std::move(initialValue)) {} - QPropertyValueStorage &operator=(T &&newValue) { value = std::move(newValue); return *this; } - QPropertyValueStorage(QPropertyValueStorage &&other) : value(std::move(other.value)), priv(std::move(other.priv), this) {} - QPropertyValueStorage &operator=(QPropertyValueStorage &&other) { value = std::move(other.value); priv.moveAssign(std::move(other.priv), &value); return *this; } - - T const& getValue() const { return value; } - bool setValueAndReturnTrueIfChanged(T &&v) - { - if constexpr (QTypeTraits::has_operator_equal_v) { - if (v == value) - return false; - } - value = std::move(v); - return true; - } - bool setValueAndReturnTrueIfChanged(const T &v) - { - if constexpr (QTypeTraits::has_operator_equal_v) { - if (v == value) - return false; - } - value = v; - return true; - } -}; - template class QTagPreservingPointerToPointer { -- cgit v1.2.3