Introduce a common base class for all QProperty types

Add an empty QUntypedPropertyData class. This allows making
a couple of places where the system is currently using a
void * more type safe.

Also add a QPropertyData<T> as an intermediate class between
QUntypedPropertyData and QProperty. This class will get used
in a future commit to simplify storing property data separately
from the possible binding data.

Also simplify the static observer handling a bit by always
passing it a pointer to the QUntypedPropertyData instead of
some other void * that could point to anything.

Change-Id: I1f8144ea717815b1bc6f034d1ac883c13af5aaf8
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

4 files changed, 139 insertions, 70 deletions

diff --git a/src/corelib/kernel/qproperty.cpp b/src/corelib/kernel/qproperty.cpp
index e3724f8f4a..2028bcf404 100644
--- a/src/corelib/kernel/qproperty.cpp
+++ b/src/corelib/kernel/qproperty.cpp
@@ -87,7 +87,7 @@ void QPropertyBindingPrivate::markDirtyAndNotifyObservers()
     if (firstObserver)
         firstObserver.notify(this, propertyDataPtr);
     if (hasStaticObserver)
-        staticObserverCallback(staticObserver, propertyDataPtr);
+        staticObserverCallback(propertyDataPtr);
 }
 
 bool QPropertyBindingPrivate::evaluateIfDirtyAndReturnTrueIfValueChanged()
@@ -116,7 +116,7 @@ bool QPropertyBindingPrivate::evaluateIfDirtyAndReturnTrueIfValueChanged()
     bool changed = false;
 
     if (hasStaticObserver && staticGuardCallback) {
-        changed = staticGuardCallback(metaType, propertyDataPtr, evaluationFunction, staticObserver);
+        changed = staticGuardCallback(metaType, propertyDataPtr, evaluationFunction);
     } else {
         changed = evaluationFunction(metaType, propertyDataPtr);
     }
@@ -183,7 +183,7 @@ QMetaType QUntypedPropertyBinding::valueMetaType() const
     return d->valueMetaType();
 }
 
-QPropertyBindingData::QPropertyBindingData(QPropertyBindingData &&other, void *propertyDataPtr)
+QPropertyBindingData::QPropertyBindingData(QPropertyBindingData &&other, QUntypedPropertyData *propertyDataPtr)
 {
     std::swap(d_ptr, other.d_ptr);
     QPropertyBindingDataPointer d{this};
@@ -192,7 +192,7 @@ QPropertyBindingData::QPropertyBindingData(QPropertyBindingData &&other, void *p
         binding->setProperty(propertyDataPtr);
 }
 
-void QPropertyBindingData::moveAssign(QPropertyBindingData &&other, void *propertyDataPtr)
+void QPropertyBindingData::moveAssign(QPropertyBindingData &&other, QUntypedPropertyData *propertyDataPtr)
 {
     if (&other == this)
         return;
@@ -229,8 +229,7 @@ QPropertyBindingData::~QPropertyBindingData()
 }
 
 QUntypedPropertyBinding QPropertyBindingData::setBinding(const QUntypedPropertyBinding &binding,
-                                                  void *propertyDataPtr,
-                                                  void *staticObserver,
+                                                  QUntypedPropertyData *propertyDataPtr,
                                                   QPropertyObserverCallback staticObserverCallback,
                                                   QtPrivate::QPropertyGuardFunction guardCallback)
 {
@@ -259,7 +258,7 @@ QUntypedPropertyBinding QPropertyBindingData::setBinding(const QUntypedPropertyB
         newBinding->setProperty(propertyDataPtr);
         if (observer)
             newBinding->prependObserver(observer);
-        newBinding->setStaticObserver(staticObserver, staticObserverCallback, guardCallback);
+        newBinding->setStaticObserver(staticObserverCallback, guardCallback);
     } else if (observer) {
         d.setObservers(observer.ptr);
     } else {
@@ -335,7 +334,7 @@ void QPropertyBindingData::registerWithCurrentlyEvaluatingBinding() const
     dependencyObserver.observeProperty(d);
 }
 
-void QPropertyBindingData::notifyObservers(void *propertyDataPtr) const
+void QPropertyBindingData::notifyObservers(QUntypedPropertyData *propertyDataPtr) const
 {
     QPropertyBindingDataPointer d{this};
     if (QPropertyObserverPointer observer = d.firstObserver())
@@ -350,13 +349,13 @@ int QPropertyBindingDataPointer::observerCount() const
     return count;
 }
 
-QPropertyObserver::QPropertyObserver(void (*callback)(QPropertyObserver *, void *))
+QPropertyObserver::QPropertyObserver(ChangeHandler changeHandler)
 {
     QPropertyObserverPointer d{this};
-    d.setChangeHandler(callback);
+    d.setChangeHandler(changeHandler);
 }
 
-QPropertyObserver::QPropertyObserver(void *aliasedPropertyPtr)
+QPropertyObserver::QPropertyObserver(QUntypedPropertyData *aliasedPropertyPtr)
 {
     QPropertyObserverPointer d{this};
     d.setAliasedProperty(aliasedPropertyPtr);
@@ -410,7 +409,7 @@ QPropertyObserver &QPropertyObserver::operator=(QPropertyObserver &&other)
 void QPropertyObserverPointer::unlink()
 {
     if (ptr->next.tag() & QPropertyObserver::ObserverNotifiesAlias)
-        ptr->aliasedPropertyPtr = 0;
+        ptr->aliasedPropertyData = nullptr;
     if (ptr->next)
         ptr->next->prev = ptr->prev;
     if (ptr->prev)
@@ -419,15 +418,15 @@ void QPropertyObserverPointer::unlink()
     ptr->prev.clear();
 }
 
-void QPropertyObserverPointer::setChangeHandler(void (*changeHandler)(QPropertyObserver *, void *))
+void QPropertyObserverPointer::setChangeHandler(QPropertyObserver::ChangeHandler changeHandler)
 {
     ptr->changeHandler = changeHandler;
     ptr->next.setTag(QPropertyObserver::ObserverNotifiesChangeHandler);
 }
 
-void QPropertyObserverPointer::setAliasedProperty(void *propertyPtr)
+void QPropertyObserverPointer::setAliasedProperty(QUntypedPropertyData *property)
 {
-    ptr->aliasedPropertyPtr = quintptr(propertyPtr);
+    ptr->aliasedPropertyData = property;
     ptr->next.setTag(QPropertyObserver::ObserverNotifiesAlias);
 }
 
@@ -437,7 +436,7 @@ void QPropertyObserverPointer::setBindingToMarkDirty(QPropertyBindingPrivate *bi
     ptr->next.setTag(QPropertyObserver::ObserverNotifiesBinding);
 }
 
-void QPropertyObserverPointer::notify(QPropertyBindingPrivate *triggeringBinding, void *propertyDataPtr)
+void QPropertyObserverPointer::notify(QPropertyBindingPrivate *triggeringBinding, QUntypedPropertyData *propertyDataPtr)
 {
     bool knownIfPropertyChanged = false;
     bool propertyChanged = true;
@@ -532,9 +531,54 @@ QString QPropertyBindingError::description() const
 }
 
 /*!
+  \class QPropertyData
+  \inmodule QtCore
+  \brief The QPropertyData class is a helper class for properties with automatic property bindings.
+  \since 6.0
+
+  \ingroup tools
+
+  QPropertyData\<T\> is a common base class for classes that can hold properties with automatic
+  data bindings. It mainly wraps the stored data, and offers low level access to that data.
+
+  The low level access to the data provided by this class bypasses the binding mechanism, and should be
+  used with care, as updates to the values will not get propagated to any bindings that depend on this
+  property.
+
+  You should usually call value() and setValue() on QProperty<T> or QBindablePropertyData<T>, not use
+  the low level mechanisms provided in this class.
+*/
+
+/*! \fn QPropertyData<T>::parameter_type QPropertyData<T>::valueBypassingBindings() const
+
+    \returns the data stored in this property.
+
+    \note As this will bypass any binding evaluation it might return an outdated value if a
+    binding is set on this property. Using this method will also not register the property
+    access with any currently executing binding.
+*/
+
+/*! \fn void QPropertyData<T>::setValueBypassingBindings(parameter_type v)
+
+    Sets the data value stored in this property to \a v.
+
+    \note Using this method will bypass any potential binding registered for this property.
+*/
+
+/*! \fn void QPropertyData<T>::setValueBypassingBindings(rvalue_ref v)
+    \overload
+
+    Sets the data value stored in this property to \a v.
+
+    \note Using this method will bypass any potential binding registered for this property.
+*/
+
+
+/*!
   \class QProperty
   \inmodule QtCore
   \brief The QProperty class is a template class that enables automatic property bindings.
+  \since 6.0
 
   \ingroup tools
 
diff --git a/src/corelib/kernel/qproperty.h b/src/corelib/kernel/qproperty.h
index ff2a5c883f..7b63a46665 100644
--- a/src/corelib/kernel/qproperty.h
+++ b/src/corelib/kernel/qproperty.h
@@ -63,6 +63,32 @@
 
 QT_BEGIN_NAMESPACE
 
+template <typename T>
+class QPropertyData : public QUntypedPropertyData
+{
+protected:
+    T val = T();
+private:
+    class DisableRValueRefs {};
+protected:
+    static constexpr bool UseReferences = !(std::is_arithmetic_v<T> || std::is_enum_v<T> || std::is_pointer_v<T>);
+public:
+    using value_type = T;
+    using parameter_type = std::conditional_t<UseReferences, const T &, T>;
+    using rvalue_ref = typename std::conditional_t<UseReferences, T &&, DisableRValueRefs>;
+    using arrow_operator_result = std::conditional_t<std::is_pointer_v<T>, const T &,
+                                        std::conditional_t<QTypeTraits::is_dereferenceable_v<T>, const T &, void>>;
+
+    QPropertyData() = default;
+    QPropertyData(parameter_type t) : val(t) {}
+    QPropertyData(rvalue_ref t) : val(std::move(t)) {}
+    ~QPropertyData() = default;
+
+    parameter_type valueBypassingBindings() const { return val; }
+    void setValueBypassingBindings(parameter_type v) { val = v; }
+    void setValueBypassingBindings(rvalue_ref v) { val = std::move(v); }
+};
+
 struct Q_CORE_EXPORT QPropertyBindingSourceLocation
 {
     const char *fileName = nullptr;
@@ -146,15 +172,15 @@ class QPropertyBinding : public QUntypedPropertyBinding
     struct BindingAdaptor
     {
         Functor impl;
-        bool operator()(QMetaType /*metaType*/, void *dataPtr)
+        bool operator()(QMetaType /*metaType*/, QUntypedPropertyData *dataPtr)
         {
-            PropertyType *propertyPtr = static_cast<PropertyType *>(dataPtr);
+            QPropertyData<PropertyType> *propertyPtr = static_cast<QPropertyData<PropertyType> *>(dataPtr);
             PropertyType newValue = impl();
             if constexpr (QTypeTraits::has_operator_equal_v<PropertyType>) {
-                if (newValue == *propertyPtr)
+                if (newValue == propertyPtr->valueBypassingBindings())
                     return false;
             }
-            *propertyPtr = std::move(newValue);
+            propertyPtr->setValueBypassingBindings(std::move(newValue));
             return true;
         }
     };
@@ -167,7 +193,7 @@ public:
         : QUntypedPropertyBinding(QMetaType::fromType<PropertyType>(), BindingAdaptor<Functor>{std::forward<Functor>(f)}, location)
     {}
 
-    template<typename Property, typename = std::void_t<decltype(&Property::bindingData)>>
+    template<typename Property, typename = typename Property::InheritsQUntypedPropertyData>
     QPropertyBinding(const Property &property)
         : QUntypedPropertyBinding(property.bindingData().binding())
     {}
@@ -187,36 +213,30 @@ namespace Qt {
     }
 }
 
-struct QPropertyBindingDataPointer;
-
 template <typename T>
-class QProperty
+class QProperty : public QPropertyData<T>
 {
-    T val = T();
     QtPrivate::QPropertyBindingData d;
     bool is_equal(const T &v)
     {
         if constexpr (QTypeTraits::has_operator_equal_v<T>) {
-            if (v == val)
+            if (v == this->val)
                 return true;
         }
         return false;
     }
 
-    class DisableRValueRefs {};
-    static constexpr bool UseReferences = !(std::is_arithmetic_v<T> || std::is_enum_v<T> || std::is_pointer_v<T>);
 public:
-    using value_type = T;
-    using parameter_type = std::conditional_t<UseReferences, const T &, T>;
-    using rvalue_ref = typename std::conditional_t<UseReferences, T &&, DisableRValueRefs>;
-    using arrow_operator_result = std::conditional_t<std::is_pointer_v<T>, const T &,
-                                        std::conditional_t<QTypeTraits::is_dereferenceable_v<T>, const T &, void>>;
+    using value_type = typename QPropertyData<T>::value_type;
+    using parameter_type = typename QPropertyData<T>::parameter_type;
+    using rvalue_ref = typename QPropertyData<T>::rvalue_ref;
+    using arrow_operator_result = typename QPropertyData<T>::arrow_operator_result;
 
     QProperty() = default;
-    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; }
+    explicit QProperty(parameter_type initialValue) : QPropertyData<T>(initialValue) {}
+    explicit QProperty(rvalue_ref initialValue) : QPropertyData<T>(std::move(initialValue)) {}
+    QProperty(QProperty &&other) : QPropertyData<T>(std::move(other.val)), d(std::move(other.d), this) { notify(); }
+    QProperty &operator=(QProperty &&other) { this->val = std::move(other.val); d.moveAssign(std::move(other.d), this); notify(); return *this; }
     QProperty(const QPropertyBinding<T> &binding)
         : QProperty()
     { operator=(binding); }
@@ -240,7 +260,7 @@ public:
         if (d.hasBinding())
             d.evaluateIfDirty();
         d.registerWithCurrentlyEvaluatingBinding();
-        return val;
+        return this->val;
     }
 
     arrow_operator_result operator->() const
@@ -270,7 +290,7 @@ public:
         d.removeBinding();
         if (is_equal(newValue))
             return;
-        val = std::move(newValue);
+        this->val = std::move(newValue);
         notify();
     }
 
@@ -279,7 +299,7 @@ public:
         d.removeBinding();
         if (is_equal(newValue))
             return;
-        val = newValue;
+        this->val = newValue;
         notify();
     }
 
@@ -303,7 +323,7 @@ public:
 
     QPropertyBinding<T> setBinding(const QPropertyBinding<T> &newBinding)
     {
-        QPropertyBinding<T> oldBinding(d.setBinding(newBinding, &val));
+        QPropertyBinding<T> oldBinding(d.setBinding(newBinding, this));
         notify();
         return oldBinding;
     }
@@ -338,7 +358,7 @@ public:
 
     QPropertyBinding<T> takeBinding()
     {
-        return QPropertyBinding<T>(d.setBinding(QUntypedPropertyBinding(), &d));
+        return QPropertyBinding<T>(d.setBinding(QUntypedPropertyBinding(), this));
     }
 
     template<typename Functor>
@@ -350,7 +370,7 @@ public:
 private:
     void notify()
     {
-        d.notifyObservers(&d);
+        d.notifyObservers(this);
     }
 
     Q_DISABLE_COPY(QProperty)
@@ -389,13 +409,14 @@ public:
     { setSource(property.bindingData()); }
 
 protected:
-    QPropertyObserver(void (*callback)(QPropertyObserver*, void *));
-    QPropertyObserver(void *aliasedPropertyPtr);
+    using ChangeHandler = void (*)(QPropertyObserver*, QUntypedPropertyData *);
+    QPropertyObserver(ChangeHandler changeHandler);
+    QPropertyObserver(QUntypedPropertyData *aliasedPropertyPtr);
 
     template<typename PropertyType>
     QProperty<PropertyType> *aliasedProperty() const
     {
-        return reinterpret_cast<QProperty<PropertyType> *>(aliasedPropertyPtr);
+        return static_cast<QProperty<PropertyType> *>(aliasedPropertyData);
     }
 
 private:
@@ -408,8 +429,8 @@ private:
 
     union {
         QPropertyBindingPrivate *bindingToMarkDirty = nullptr;
-        void (*changeHandler)(QPropertyObserver*, void *);
-        quintptr aliasedPropertyPtr;
+        ChangeHandler changeHandler;
+        QUntypedPropertyData *aliasedPropertyData;
     };
 
     QPropertyObserver(const QPropertyObserver &) = delete;
@@ -426,7 +447,7 @@ class QPropertyChangeHandler : public QPropertyObserver
     Functor m_handler;
 public:
     QPropertyChangeHandler(Functor handler)
-        : QPropertyObserver([](QPropertyObserver *self, void *) {
+        : QPropertyObserver([](QPropertyObserver *self, QUntypedPropertyData *) {
               auto This = static_cast<QPropertyChangeHandler<Functor>*>(self);
               This->m_handler();
           })
@@ -434,9 +455,9 @@ public:
     {
     }
 
-    template<typename Property, typename = std::void_t<decltype(&Property::bindingData)>>
+    template<typename Property, typename = typename Property::InheritsQUntypedPropertyData>
     QPropertyChangeHandler(const Property &property, Functor handler)
-        : QPropertyObserver([](QPropertyObserver *self, void *) {
+        : QPropertyObserver([](QPropertyObserver *self, QUntypedPropertyData *) {
               auto This = static_cast<QPropertyChangeHandler<Functor>*>(self);
               This->m_handler();
           })
diff --git a/src/corelib/kernel/qproperty_p.h b/src/corelib/kernel/qproperty_p.h
index 6e8f4b4516..d93db988f9 100644
--- a/src/corelib/kernel/qproperty_p.h
+++ b/src/corelib/kernel/qproperty_p.h
@@ -102,10 +102,10 @@ struct QPropertyObserverPointer
     void unlink();
 
     void setBindingToMarkDirty(QPropertyBindingPrivate *binding);
-    void setChangeHandler(void (*changeHandler)(QPropertyObserver *, void *));
-    void setAliasedProperty(void *propertyPtr);
+    void setChangeHandler(QPropertyObserver::ChangeHandler changeHandler);
+    void setAliasedProperty(QUntypedPropertyData *propertyPtr);
 
-    void notify(QPropertyBindingPrivate *triggeringBinding, void *propertyDataPtr);
+    void notify(QPropertyBindingPrivate *triggeringBinding, QUntypedPropertyData *propertyDataPtr);
     void observeProperty(QPropertyBindingDataPointer property);
 
     explicit operator bool() const { return ptr != nullptr; }
@@ -147,14 +147,13 @@ private:
     union {
         ObserverArray inlineDependencyObservers;
         struct {
-            void *staticObserver;
             QtPrivate::QPropertyObserverCallback staticObserverCallback;
             QtPrivate::QPropertyGuardFunction staticGuardCallback;
         };
     };
     QScopedPointer<std::vector<QPropertyObserver>> heapObservers;
 
-    void *propertyDataPtr = nullptr;
+    QUntypedPropertyData *propertyDataPtr = nullptr;
 
     QPropertyBindingSourceLocation location;
     QPropertyBindingError error;
@@ -175,11 +174,10 @@ public:
     virtual ~QPropertyBindingPrivate();
 
     void setDirty(bool d) { dirty = d; }
-    void setProperty(void *propertyPtr) { propertyDataPtr = propertyPtr; }
-    void setStaticObserver(void *observer, QtPrivate::QPropertyObserverCallback callback,
-                           QtPrivate::QPropertyGuardFunction guardCallback)
+    void setProperty(QUntypedPropertyData *propertyPtr) { propertyDataPtr = propertyPtr; }
+    void setStaticObserver(QtPrivate::QPropertyObserverCallback callback, QtPrivate::QPropertyGuardFunction guardCallback)
     {
-        if (observer) {
+        if (callback) {
             if (!hasStaticObserver) {
                 if (dependencyObserverCount > 0) {
                     if (!heapObservers)
@@ -191,7 +189,6 @@ public:
             }
 
             hasStaticObserver = true;
-            staticObserver = observer;
             staticObserverCallback = callback;
             staticGuardCallback = guardCallback;
         } else if (hasStaticObserver) {
diff --git a/src/corelib/kernel/qpropertyprivate.h b/src/corelib/kernel/qpropertyprivate.h
index bab4162472..33ecb405f2 100644
--- a/src/corelib/kernel/qpropertyprivate.h
+++ b/src/corelib/kernel/qpropertyprivate.h
@@ -65,14 +65,21 @@ class QPropertyBindingPrivate;
 using QPropertyBindingPrivatePtr = QExplicitlySharedDataPointer<QPropertyBindingPrivate>;
 struct QPropertyBindingDataPointer;
 
+class QUntypedPropertyData
+{
+public:
+    // sentinel to check whether a class inherits QUntypedPropertyData
+    struct InheritsQUntypedPropertyData {};
+};
+
 namespace QtPrivate {
 
 // writes binding result into dataPtr
-using QPropertyBindingFunction = std::function<bool(QMetaType metaType, void *dataPtr)>;
+using QPropertyBindingFunction = std::function<bool(QMetaType metaType, QUntypedPropertyData *dataPtr)>;
 
-using QPropertyGuardFunction = bool(*)(QMetaType, void *dataPtr,
-                                       QPropertyBindingFunction, void *owner);
-using QPropertyObserverCallback = void (*)(void *, void *);
+using QPropertyGuardFunction = bool(*)(QMetaType, QUntypedPropertyData *dataPtr,
+                                       QPropertyBindingFunction);
+using QPropertyObserverCallback = void (*)(QUntypedPropertyData *);
 
 class Q_CORE_EXPORT QPropertyBindingData
 {
@@ -84,16 +91,16 @@ public:
     QPropertyBindingData() = default;
     Q_DISABLE_COPY(QPropertyBindingData)
     QPropertyBindingData(QPropertyBindingData &&other) = delete;
-    QPropertyBindingData(QPropertyBindingData &&other, void *propertyDataPtr);
+    QPropertyBindingData(QPropertyBindingData &&other, QUntypedPropertyData *propertyDataPtr);
     QPropertyBindingData &operator=(QPropertyBindingData &&other) = delete;
     ~QPropertyBindingData();
 
-    void moveAssign(QPropertyBindingData &&other, void *propertyDataPtr);
+    void moveAssign(QPropertyBindingData &&other, QUntypedPropertyData *propertyDataPtr);
 
     bool hasBinding() const { return d_ptr & BindingBit; }
 
     QUntypedPropertyBinding setBinding(const QUntypedPropertyBinding &newBinding,
-                                       void *propertyDataPtr, void *staticObserver = nullptr,
+                                       QUntypedPropertyData *propertyDataPtr,
                                        QPropertyObserverCallback staticObserverCallback = nullptr,
                                        QPropertyGuardFunction guardCallback = nullptr);
     QPropertyBindingPrivate *binding() const;
@@ -102,7 +109,7 @@ public:
     void removeBinding();
 
     void registerWithCurrentlyEvaluatingBinding() const;
-    void notifyObservers(void *propertyDataPtr) const;
+    void notifyObservers(QUntypedPropertyData *propertyDataPtr) const;
 
     void setExtraBit(bool b)
     {
@@ -182,7 +189,7 @@ struct QPropertyGuardFunctionHelper
 template<typename T, typename Class, auto Guard>
 struct QPropertyGuardFunctionHelper<T, Class, Guard, false>
 {
-    static auto guard(const QMetaType metaType, void *dataPtr,
+    static auto guard(QMetaType metaType, QUntypedPropertyData *dataPtr,
                       QPropertyBindingFunction eval, void *owner) -> bool
     {
         T t = T();