Add QObjectCompatProperty

Add a compatibility property class that makes porting to the new
property system as simple as possible.

Binding evaluation for those compat properties is eager, as we
do not control possible side effects of the code in the existing
setters.

Change-Id: Ic56347abb49e40631ec73e88c6d40d4bdb05ca29
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

src/corelib/kernel/qproperty.cpp

@@ -82,6 +82,10 @@ void QPropertyBindingPrivate::markDirtyAndNotifyObservers()
     if (dirty)
         return;
     dirty = true;
+    if (requiresEagerEvaluation()) {
+        // these are compat properties that we will need to evaluate eagerly
+        evaluateIfDirtyAndReturnTrueIfValueChanged(propertyDataPtr);
+    }
     if (firstObserver)
         firstObserver.notify(this, propertyDataPtr);
     if (hasStaticObserver)
@@ -109,7 +113,7 @@ bool QPropertyBindingPrivate::evaluateIfDirtyAndReturnTrueIfValueChanged(const Q
     QPropertyBindingPrivatePtr keepAlive {this};
     QScopedValueRollback<bool> updateGuard(updating, true);
 
-    QBindingEvaluationState evaluationFrame(this);
+    BindingEvaluationState evaluationFrame(this);
 
     bool changed = false;
 
@@ -117,7 +121,7 @@ bool QPropertyBindingPrivate::evaluateIfDirtyAndReturnTrueIfValueChanged(const Q
     QUntypedPropertyData *mutable_data = const_cast<QUntypedPropertyData *>(data);
 
     if (hasBindingWrapper) {
-        changed = staticBindingWrapper(metaType, propertyDataPtr, evaluationFunction);
+        changed = staticBindingWrapper(metaType, mutable_data, evaluationFunction);
     } else {
         changed = evaluationFunction(metaType, mutable_data);
     }
@@ -260,6 +264,8 @@ QUntypedPropertyBinding QPropertyBindingData::setBinding(const QUntypedPropertyB
         if (observer)
             newBinding->prependObserver(observer);
         newBinding->setStaticObserver(staticObserverCallback, guardCallback);
+        if (newBinding->requiresEagerEvaluation())
+            newBinding->evaluateIfDirtyAndReturnTrueIfValueChanged(propertyDataPtr);
     } else if (observer) {
         d.setObservers(observer.ptr);
     } else {
@@ -280,27 +286,33 @@ QPropertyBindingPrivate *QPropertyBindingData::binding() const
     return nullptr;
 }
 
-static thread_local QBindingEvaluationState *currentBindingEvaluationState = nullptr;
+static thread_local QBindingStatus bindingStatus;
 
-QBindingEvaluationState::QBindingEvaluationState(QPropertyBindingPrivate *binding)
+BindingEvaluationState::BindingEvaluationState(QPropertyBindingPrivate *binding)
     : binding(binding)
 {
     // store a pointer to the currentBindingEvaluationState to avoid a TLS lookup in
     // the destructor (as these come with a non zero cost)
-    currentState = &currentBindingEvaluationState;
+    currentState = &bindingStatus.currentlyEvaluatingBinding;
     previousState = *currentState;
     *currentState = this;
     binding->clearDependencyObservers();
 }
 
-QBindingEvaluationState::~QBindingEvaluationState()
+CurrentCompatProperty::CurrentCompatProperty(QBindingStatus *status, QUntypedPropertyData *property)
+    : property(property)
 {
-    *currentState = previousState;
+    // store a pointer to the currentBindingEvaluationState to avoid a TLS lookup in
+    // the destructor (as these come with a non zero cost)
+    currentState = &status->currentCompatProperty;
+    previousState = *currentState;
+    *currentState = this;
 }
 
 QPropertyBindingPrivate *QPropertyBindingPrivate::currentlyEvaluatingBinding()
 {
-    return currentBindingEvaluationState ? currentBindingEvaluationState->binding : nullptr;
+    auto currentState = bindingStatus.currentlyEvaluatingBinding ;
+    return currentState ? currentState->binding : nullptr;
 }
 
 void QPropertyBindingData::evaluateIfDirty(const QUntypedPropertyData *property) const
@@ -327,7 +339,7 @@ void QPropertyBindingData::removeBinding()
 
 void QPropertyBindingData::registerWithCurrentlyEvaluatingBinding() const
 {
-    auto currentState = currentBindingEvaluationState;
+    auto currentState = bindingStatus.currentlyEvaluatingBinding;
     if (!currentState)
         return;
 
@@ -1448,8 +1460,8 @@ struct QBindingStoragePrivate
 
 QBindingStorage::QBindingStorage()
 {
-    currentlyEvaluatingBinding = &currentBindingEvaluationState;
-    Q_ASSERT(currentlyEvaluatingBinding);
+    bindingStatus = &QT_PREPEND_NAMESPACE(bindingStatus);
+    Q_ASSERT(bindingStatus);
 }
 
 QBindingStorage::~QBindingStorage()
@@ -1459,9 +1471,9 @@ QBindingStorage::~QBindingStorage()
 
 void QBindingStorage::maybeUpdateBindingAndRegister(const QUntypedPropertyData *data) const
 {
-    Q_ASSERT(currentlyEvaluatingBinding);
+    Q_ASSERT(bindingStatus);
     QUntypedPropertyData *dd = const_cast<QUntypedPropertyData *>(data);
-    auto storage = *currentlyEvaluatingBinding ?
+    auto storage = bindingStatus->currentlyEvaluatingBinding ?
                 QBindingStoragePrivate(d).getAndCreate(dd) :
                 QBindingStoragePrivate(d).get(dd);
     if (!storage)

src/corelib/kernel/qproperty_p.h

@@ -1,4 +1,4 @@
-/****************************************************************************
+/***************************************************************************
 **
 ** Copyright (C) 2020 The Qt Company Ltd.
 ** Contact: https://www.qt.io/licensing/
@@ -120,13 +120,39 @@ public:
     QString description;
 };
 
-struct QBindingEvaluationState
+namespace QtPrivate {
+
+struct BindingEvaluationState
 {
-    QBindingEvaluationState(QPropertyBindingPrivate *binding);
-    ~QBindingEvaluationState();
+    BindingEvaluationState(QPropertyBindingPrivate *binding);
+    ~BindingEvaluationState()
+    {
+        *currentState = previousState;
+    }
+
     QPropertyBindingPrivate *binding;
-    QBindingEvaluationState *previousState = nullptr;
-    QBindingEvaluationState **currentState = nullptr;
+    BindingEvaluationState *previousState = nullptr;
+    BindingEvaluationState **currentState = nullptr;
+};
+
+struct CurrentCompatProperty
+{
+    Q_CORE_EXPORT CurrentCompatProperty(QBindingStatus *status, QUntypedPropertyData *property);
+    ~CurrentCompatProperty()
+    {
+        *currentState = previousState;
+    }
+    QUntypedPropertyData *property;
+    CurrentCompatProperty *previousState = nullptr;
+    CurrentCompatProperty **currentState = nullptr;
+};
+
+}
+
+struct QBindingStatus
+{
+    QtPrivate::BindingEvaluationState *currentlyEvaluatingBinding = nullptr;
+    QtPrivate::CurrentCompatProperty *currentCompatProperty = nullptr;
 };
 
 class Q_CORE_EXPORT QPropertyBindingPrivate : public QSharedData
@@ -144,12 +170,13 @@ private:
 
     QUntypedPropertyBinding::BindingEvaluationFunction evaluationFunction;
 
-    QPropertyObserverPointer firstObserver;
     union {
         QtPrivate::QPropertyObserverCallback staticObserverCallback = nullptr;
         QtPrivate::QPropertyBindingWrapper staticBindingWrapper;
     };
     ObserverArray inlineDependencyObservers;
+
+    QPropertyObserverPointer firstObserver;
     QScopedPointer<std::vector<QPropertyObserver>> heapObservers;
 
     QUntypedPropertyData *propertyDataPtr = nullptr;
@@ -174,17 +201,17 @@ public:
 
     void setDirty(bool d) { dirty = d; }
     void setProperty(QUntypedPropertyData *propertyPtr) { propertyDataPtr = propertyPtr; }
-    void setStaticObserver(QtPrivate::QPropertyObserverCallback callback, QtPrivate::QPropertyBindingWrapper guardCallback)
+    void setStaticObserver(QtPrivate::QPropertyObserverCallback callback, QtPrivate::QPropertyBindingWrapper bindingWrapper)
     {
-        Q_ASSERT(!(callback && guardCallback));
+        Q_ASSERT(!(callback && bindingWrapper));
         if (callback) {
             hasStaticObserver = true;
             hasBindingWrapper = false;
             staticObserverCallback = callback;
-        } else if (guardCallback) {
+        } else if (bindingWrapper) {
             hasStaticObserver = false;
             hasBindingWrapper = true;
-            staticBindingWrapper = guardCallback;
+            staticBindingWrapper = bindingWrapper;
         } else {
             hasStaticObserver = false;
             hasBindingWrapper = false;
@@ -245,6 +272,8 @@ public:
         clearDependencyObservers();
     }
 
+    bool requiresEagerEvaluation() const { return hasBindingWrapper; }
+
     static QPropertyBindingPrivate *currentlyEvaluatingBinding();
 };
 
@@ -264,6 +293,181 @@ inline QPropertyObserverPointer QPropertyBindingDataPointer::firstObserver() con
     return {reinterpret_cast<QPropertyObserver*>(ptr->d_ptr & ~QtPrivate::QPropertyBindingData::FlagMask)};
 }
 
+
+template<typename Class, typename T, auto Offset, auto Setter>
+class QObjectCompatProperty : public QPropertyData<T>
+{
+    using ThisType = QObjectCompatProperty<Class, T, Offset, Setter>;
+    Class *owner()
+    {
+        char *that = reinterpret_cast<char *>(this);
+        return reinterpret_cast<Class *>(that - QtPrivate::detail::getOffset(Offset));
+    }
+    const Class *owner() const
+    {
+        char *that = const_cast<char *>(reinterpret_cast<const char *>(this));
+        return reinterpret_cast<Class *>(that - QtPrivate::detail::getOffset(Offset));
+    }
+    static bool bindingWrapper(QMetaType type, QUntypedPropertyData *dataPtr, QtPrivate::QPropertyBindingFunction binding)
+    {
+        auto *thisData = static_cast<ThisType *>(dataPtr);
+        QPropertyData<T> copy;
+        binding(type, &copy);
+        if constexpr (QTypeTraits::has_operator_equal_v<T>)
+            if (copy.valueBypassingBindings() == thisData->valueBypassingBindings())
+                return false;
+        // ensure value and setValue know we're currently evaluating our binding
+        QBindingStorage *storage = qGetBindingStorage(thisData->owner());
+        QtPrivate::CurrentCompatProperty guardThis(storage->bindingStatus, thisData);
+        (thisData->owner()->*Setter)(copy.valueBypassingBindings());
+        return true;
+    }
+    inline bool inBindingWrapper(const QBindingStorage *storage) const
+    {
+        return storage->bindingStatus->currentCompatProperty &&
+               storage->bindingStatus->currentCompatProperty->property == this;
+    }
+
+public:
+    using value_type = typename QPropertyData<T>::value_type;
+    using parameter_type = typename QPropertyData<T>::parameter_type;
+    using arrow_operator_result = typename QPropertyData<T>::arrow_operator_result;
+
+    QObjectCompatProperty() = default;
+    explicit QObjectCompatProperty(const T &initialValue) : QPropertyData<T>(initialValue) {}
+    explicit QObjectCompatProperty(T &&initialValue) : QPropertyData<T>(std::move(initialValue)) {}
+
+    parameter_type value() const {
+        const QBindingStorage *storage = qGetBindingStorage(owner());
+        // make sure we don't register this binding as a dependency to itself
+        if (!inBindingWrapper(storage))
+            storage->maybeUpdateBindingAndRegister(this);
+        return this->val;
+    }
+
+    arrow_operator_result operator->() const
+    {
+        if constexpr (QTypeTraits::is_dereferenceable_v<T>) {
+            return value();
+        } else if constexpr (std::is_pointer_v<T>) {
+            value();
+            return this->val;
+        } else {
+            return;
+        }
+    }
+
+    parameter_type operator*() const
+    {
+        return value();
+    }
+
+    operator parameter_type() const
+    {
+        return value();
+    }
+
+    void setValue(parameter_type t) {
+        QBindingStorage *storage = qGetBindingStorage(owner());
+        auto *bd = storage->bindingData(this);
+        // make sure we don't remove the binding if called from the bindingWrapper
+        if (bd && !inBindingWrapper(storage))
+            bd->removeBinding();
+        if constexpr (QTypeTraits::has_operator_equal_v<T>)
+            if (this->val == t)
+                return;
+        this->val = t;
+        notify(bd);
+    }
+
+    QObjectCompatProperty &operator=(parameter_type newValue)
+    {
+        setValue(newValue);
+        return *this;
+    }
+
+    QPropertyBinding<T> setBinding(const QPropertyBinding<T> &newBinding)
+    {
+        QtPrivate::QPropertyBindingData *bd = qGetBindingStorage(owner())->bindingData(this, true);
+        QUntypedPropertyBinding oldBinding(bd->setBinding(newBinding, this, nullptr, bindingWrapper));
+        notify(bd);
+        return static_cast<QPropertyBinding<T> &>(oldBinding);
+    }
+
+    bool setBinding(const QUntypedPropertyBinding &newBinding)
+    {
+        if (!newBinding.isNull() && newBinding.valueMetaType().id() != qMetaTypeId<T>())
+            return false;
+        setBinding(static_cast<const QPropertyBinding<T> &>(newBinding));
+        return true;
+    }
+
+#ifndef Q_CLANG_QDOC
+    template <typename Functor>
+    QPropertyBinding<T> setBinding(Functor &&f,
+                                   const QPropertyBindingSourceLocation &location = QT_PROPERTY_DEFAULT_BINDING_LOCATION,
+                                   std::enable_if_t<std::is_invocable_v<Functor>> * = nullptr)
+    {
+        return setBinding(Qt::makePropertyBinding(std::forward<Functor>(f), location));
+    }
+#else
+    template <typename Functor>
+    QPropertyBinding<T> setBinding(Functor f);
+#endif
+
+    bool hasBinding() const {
+        auto *bd = qGetBindingStorage(owner())->bindingData(this);
+        return bd && bd->binding() != nullptr;
+    }
+
+    QPropertyBinding<T> binding() const
+    {
+        auto *bd = qGetBindingStorage(owner())->bindingData(this);
+        return static_cast<QPropertyBinding<T> &&>(QUntypedPropertyBinding(bd ? bd->binding() : nullptr));
+    }
+
+    QPropertyBinding<T> takeBinding()
+    {
+        return setBinding(QPropertyBinding<T>());
+    }
+
+    template<typename Functor>
+    QPropertyChangeHandler<Functor> onValueChanged(Functor f)
+    {
+        static_assert(std::is_invocable_v<Functor>, "Functor callback must be callable without any parameters");
+        return QPropertyChangeHandler<Functor>(*this, f);
+    }
+
+    template<typename Functor>
+    QPropertyChangeHandler<Functor> subscribe(Functor f)
+    {
+        static_assert(std::is_invocable_v<Functor>, "Functor callback must be callable without any parameters");
+        f();
+        return onValueChanged(f);
+    }
+
+    QtPrivate::QPropertyBindingData &bindingData() const
+    {
+        auto *storage = const_cast<QBindingStorage *>(qGetBindingStorage(owner()));
+        return *storage->bindingData(const_cast<QObjectCompatProperty *>(this), true);
+    }
+private:
+    void notify(const QtPrivate::QPropertyBindingData *binding)
+    {
+        if (binding)
+            binding->notifyObservers(this);
+    }
+};
+
+#define Q_OBJECT_COMPAT_PROPERTY(Class, Type, name,  setter) \
+    static constexpr size_t _qt_property_##name##_offset() { \
+        QT_WARNING_PUSH QT_WARNING_DISABLE_INVALID_OFFSETOF \
+        return offsetof(Class, name); \
+        QT_WARNING_POP \
+    } \
+    QObjectCompatProperty<Class, Type, Class::_qt_property_##name##_offset, setter> name;
+
+
 QT_END_NAMESPACE
 
 #endif // QPROPERTY_P_H

src/corelib/kernel/qpropertyprivate.h

@@ -76,10 +76,8 @@ namespace QtPrivate {
 
 // writes binding result into dataPtr
 using QPropertyBindingFunction = std::function<bool(QMetaType metaType, QUntypedPropertyData *dataPtr)>;
-
-using QPropertyBindingWrapper = bool(*)(QMetaType, QUntypedPropertyData *dataPtr,
-                                       QPropertyBindingFunction);
 using QPropertyObserverCallback = void (*)(QUntypedPropertyData *);
+using QPropertyBindingWrapper = bool(*)(QMetaType, QUntypedPropertyData *dataPtr, QPropertyBindingFunction);
 
 class Q_CORE_EXPORT QPropertyBindingData
 {
@@ -101,7 +99,8 @@ public:
     QUntypedPropertyBinding setBinding(const QUntypedPropertyBinding &newBinding,
                                        QUntypedPropertyData *propertyDataPtr,
                                        QPropertyObserverCallback staticObserverCallback = nullptr,
-                                       QPropertyBindingWrapper guardCallback = nullptr);
+                                       QPropertyBindingWrapper bindingWrapper = nullptr);
+
     QPropertyBindingPrivate *binding() const;
 
     void evaluateIfDirty(const QUntypedPropertyData *property) const;
@@ -188,30 +187,6 @@ namespace detail {
     }
 }
 
-// type erased guard functions, casts its arguments to the correct types
-template<typename T, typename Class, auto Guard, bool = std::is_same_v<decltype(Guard), std::nullptr_t>>
-struct QPropertyGuardFunctionHelper
-{
-    static constexpr QPropertyBindingWrapper guard = nullptr;
-};
-template<typename T, typename Class, auto Guard>
-struct QPropertyGuardFunctionHelper<T, Class, Guard, false>
-{
-    static auto guard(QMetaType metaType, QUntypedPropertyData *dataPtr,
-                      QPropertyBindingFunction eval, void *owner) -> bool
-    {
-        T t = T();
-        eval(metaType, &t);
-        if (!(static_cast<Class *>(owner)->*Guard)(t))
-            return false;
-        T *data = static_cast<T *>(dataPtr);
-        if (*data == t)
-            return false;
-        *data = std::move(t);
-        return true;
-    };
-};
-
 } // namespace QtPrivate
 
 QT_END_NAMESPACE

tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp

@@ -76,6 +76,7 @@ private slots:
 
     void testNewStuff();
     void qobjectObservers();
+    void compatBindings();
 };
 
 void tst_QProperty::functorBinding()
@@ -969,16 +970,19 @@ class MyQObject : public QObject
     Q_PROPERTY(int bar READ bar WRITE setBar NOTIFY barChanged)
     Q_PROPERTY(int read READ read NOTIFY readChanged)
     Q_PROPERTY(int computed READ computed STORED false)
+    Q_PROPERTY(int compat READ compat WRITE setCompat NOTIFY compatChanged)
 
 signals:
     void fooChanged();
     void barChanged();
     void readChanged();
+    void compatChanged();
 
 public slots:
     void fooHasChanged() { fooChangedCount++; }
     void barHasChanged() { barChangedCount++; }
     void readHasChanged() { readChangedCount++; }
+    void compatHasChanged() { compatChangedCount++; }
 
 public:
     int foo() const { return fooData.value(); }
@@ -987,21 +991,37 @@ public:
     void setBar(int i) { barData.setValue(i); }
     int read() const { return readData.value(); }
     int computed() const { return readData.value(); }
+    int compat() const { return compatData; }
+    void setCompat(int i)
+    {
+        if (compatData == i)
+            return;
+        // implement some side effect and clamping
+        ++setCompatCalled;
+        if (i < 0)
+            i = 0;
+        compatData = i;
+        emit compatChanged();
+    }
 
     QBindable<int> bindableFoo() { return QBindable<int>(&fooData); }
     QBindable<int> bindableBar() { return QBindable<int>(&barData); }
     QBindable<int> bindableRead() { return QBindable<int>(&readData); }
     QBindable<int> bindableComputed() { return QBindable<int>(&computedData); }
+    QBindable<int> bindableCompat() { return QBindable<int>(&compatData); }
 
 public:
     int fooChangedCount = 0;
     int barChangedCount = 0;
     int readChangedCount = 0;
+    int compatChangedCount = 0;
+    int setCompatCalled = 0;
 
     Q_OBJECT_BINDABLE_PROPERTY(MyQObject, int, fooData, &MyQObject::fooChanged);
     Q_OBJECT_BINDABLE_PROPERTY(MyQObject, int, barData, &MyQObject::barChanged);
     Q_OBJECT_BINDABLE_PROPERTY(MyQObject, int, readData, &MyQObject::readChanged);
     Q_OBJECT_COMPUTED_PROPERTY(MyQObject, int, computedData, &MyQObject::computed);
+    Q_OBJECT_COMPAT_PROPERTY(MyQObject, int, compatData, &MyQObject::setCompat)
 };
 
 void tst_QProperty::testNewStuff()
@@ -1086,6 +1106,62 @@ void tst_QProperty::qobjectObservers()
     QCOMPARE(onValueChangedCalled, 3);
 }
 
+void tst_QProperty::compatBindings()
+{
+    MyQObject object;
+    QObject::connect(&object, &MyQObject::fooChanged, &object, &MyQObject::fooHasChanged);
+    QObject::connect(&object, &MyQObject::barChanged, &object, &MyQObject::barHasChanged);
+    QObject::connect(&object, &MyQObject::compatChanged, &object, &MyQObject::compatHasChanged);
+
+    QCOMPARE(object.compatData, 0);
+    // setting data through the private interface should not call the changed signal or the public setter
+    object.compatData = 10;
+    QCOMPARE(object.compatChangedCount, 0);
+    QCOMPARE(object.setCompatCalled, 0);
+    // going through the public API should emit the signal
+    object.setCompat(42);
+    QCOMPARE(object.compatChangedCount, 1);
+    QCOMPARE(object.setCompatCalled, 1);
+
+    // setting the same value again does nothing
+    object.setCompat(42);
+    QCOMPARE(object.compatChangedCount, 1);
+    QCOMPARE(object.setCompatCalled, 1);
+
+    object.setFoo(111);
+    // just setting the binding. For a compat property, this should already trigger evaluation
+    object.compatData.setBinding(object.bindableFoo().makeBinding());
+    QCOMPARE(object.compatData.valueBypassingBindings(), 111);
+    QCOMPARE(object.compatChangedCount, 2);
+    QCOMPARE(object.setCompatCalled, 2);
+
+    QCOMPARE(object.compat(), 111);
+    QCOMPARE(object.compatChangedCount, 2);
+    QCOMPARE(object.setCompatCalled, 2);
+
+    object.setFoo(666);
+    QCOMPARE(object.compatData.valueBypassingBindings(), 666);
+    QCOMPARE(object.compatChangedCount, 3);
+    QCOMPARE(object.setCompatCalled, 3);
+
+    QCOMPARE(object.compat(), 666);
+    QCOMPARE(object.compatChangedCount, 3);
+    QCOMPARE(object.setCompatCalled, 3);
+
+    object.setFoo(-42);
+    QCOMPARE(object.compatChangedCount, 4);
+    QCOMPARE(object.setCompatCalled, 4);
+
+    QCOMPARE(object.compat(), 0);
+    QCOMPARE(object.compatChangedCount, 4);
+    QCOMPARE(object.setCompatCalled, 4);
+
+    object.setCompat(0);
+    QCOMPARE(object.compat(), 0);
+    QCOMPARE(object.compatChangedCount, 4);
+    QCOMPARE(object.setCompatCalled, 4);
+}
+
 QTEST_MAIN(tst_QProperty);
 
 #include "tst_qproperty.moc"