QNotifiedProperty: pass old value to callback if requested

Check at compile time whether the static callback takes an argument
(which has to be of the same time as the type of the property). If so,
retrieve the old value and pass it to the callback.

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

5 files changed, 88 insertions, 30 deletions

diff --git a/src/corelib/kernel/qproperty.cpp b/src/corelib/kernel/qproperty.cpp
index a1b1d7b127..7ee035030f 100644
--- a/src/corelib/kernel/qproperty.cpp
+++ b/src/corelib/kernel/qproperty.cpp
@@ -95,7 +95,7 @@ QPropertyBase::~QPropertyBase()
 QUntypedPropertyBinding QPropertyBase::setBinding(const QUntypedPropertyBinding &binding,
                                                   void *propertyDataPtr,
                                                   void *staticObserver,
-                                                  void (*staticObserverCallback)(void*))
+                                                  void (*staticObserverCallback)(void*, void*))
 {
     QPropertyBindingPrivatePtr oldBinding;
     QPropertyBindingPrivatePtr newBinding = binding.d;
diff --git a/src/corelib/kernel/qproperty.h b/src/corelib/kernel/qproperty.h
index f0156fac02..b1b4dbeae1 100644
--- a/src/corelib/kernel/qproperty.h
+++ b/src/corelib/kernel/qproperty.h
@@ -374,11 +374,21 @@ namespace Qt {
     }
 }
 
-template <typename T, typename Class, void(Class::*Callback)()>
-class QNotifiedProperty<T, Callback>
+namespace detail {
+    template <typename F>
+    struct ExtractClassFromFunctionPointer;
+
+    template<typename T, typename C>
+    struct ExtractClassFromFunctionPointer<T C::*> { using Class = C; };
+}
+
+template <typename T, auto Callback>
+class QNotifiedProperty
 {
 public:
     using value_type = T;
+    using Class = typename detail::ExtractClassFromFunctionPointer<decltype(Callback)>::Class;
+    static_assert(std::is_invocable_v<decltype(Callback), Class, T> || std::is_invocable_v<decltype(Callback), Class>);
 
     QNotifiedProperty() = default;
 
@@ -420,45 +430,83 @@ public:
 
     void setValue(Class *owner, T &&newValue)
     {
-        if (d.setValueAndReturnTrueIfChanged(std::move(newValue)))
-            notify(owner);
+        if constexpr(std::is_invocable_v<decltype(Callback), Class>) {
+            if (d.setValueAndReturnTrueIfChanged(std::move(newValue)))
+                notify(owner);
+        } else {
+            T oldValue = value(); // TODO: kind of pointless if there was no change
+            if (d.setValueAndReturnTrueIfChanged(std::move(newValue)))
+                notify(owner, &oldValue);
+        }
         d.priv.removeBinding();
     }
 
     void setValue(Class *owner, const T &newValue)
     {
-        if (d.setValueAndReturnTrueIfChanged(newValue))
-            notify(owner);
+        if constexpr(std::is_invocable_v<decltype(Callback), Class>) {
+            if (d.setValueAndReturnTrueIfChanged(newValue))
+                notify(owner);
+        } else {
+            T oldValue = value();
+            if (d.setValueAndReturnTrueIfChanged(newValue))
+                notify(owner, &oldValue);
+        }
         d.priv.removeBinding();
     }
 
     QPropertyBinding<T> setBinding(Class *owner, const QPropertyBinding<T> &newBinding)
     {
-        QPropertyBinding<T> oldBinding(d.priv.setBinding(newBinding, &d, owner, [](void *o) {
-            (reinterpret_cast<Class *>(o)->*Callback)();
-        }));
-        notify(owner);
-        return oldBinding;
+        if constexpr(std::is_invocable_v<decltype(Callback), Class>) {
+            QPropertyBinding<T> oldBinding(d.priv.setBinding(newBinding, &d, owner, [](void *o) {
+                (reinterpret_cast<Class *>(o)->*Callback)();
+            }));
+            notify(owner);
+            return oldBinding;
+        } else {
+            T oldValue = value();
+            QPropertyBinding<T> oldBinding(d.priv.setBinding(newBinding, &d, owner, [](void *o, void *oldValue) {
+                (reinterpret_cast<Class *>(o)->*Callback)(*reinterpret_cast<T *>(oldValue));
+            }));
+            notify(owner, &oldValue);
+            return oldBinding;
+        }
     }
 
     QPropertyBinding<T> setBinding(Class *owner, QPropertyBinding<T> &&newBinding)
     {
         QPropertyBinding<T> b(std::move(newBinding));
-        QPropertyBinding<T> oldBinding(d.priv.setBinding(b, &d, owner, [](void *o) {
-            (reinterpret_cast<Class *>(o)->*Callback)();
-        }));
-        notify(owner);
-        return oldBinding;
+        if constexpr(std::is_invocable_v<decltype(Callback), Class>) {
+            QPropertyBinding<T> oldBinding(d.priv.setBinding(b, &d, owner, [](void *o, void *) {
+                (reinterpret_cast<Class *>(o)->*Callback)();
+            }));
+            notify(owner);
+            return oldBinding;
+        } else {
+            T oldValue = value();
+            QPropertyBinding<T> oldBinding(d.priv.setBinding(b, &d, owner, [](void *o, void *oldValue) {
+                (reinterpret_cast<Class *>(o)->*Callback)(*reinterpret_cast<T *>(oldValue));
+            }));
+            notify(owner, &oldValue);
+            return oldBinding;
+        }
     }
 
     bool setBinding(Class *owner, const QUntypedPropertyBinding &newBinding)
     {
         if (newBinding.valueMetaType().id() != qMetaTypeId<T>())
             return false;
-        d.priv.setBinding(newBinding, &d, owner, [](void *o) {
-            (reinterpret_cast<Class *>(o)->*Callback)();
-        });
-        notify(owner);
+        if constexpr(std::is_invocable_v<decltype(Callback), Class>) {
+            d.priv.setBinding(newBinding, &d, owner, [](void *o, void *) {
+                (reinterpret_cast<Class *>(o)->*Callback)();
+            });
+            notify(owner);
+        } else {
+            T oldValue = value();
+            d.priv.setBinding(newBinding, &d, owner, [](void *o, void *oldValue) {
+                (reinterpret_cast<Class *>(o)->*Callback)(*reinterpret_cast<T *>(oldValue));
+            });
+            notify(owner, &oldValue);
+        }
         return true;
     }
 
@@ -493,10 +541,13 @@ public:
     QPropertyChangeHandler<Functor> subscribe(Functor f);
 
 private:
-    void notify(Class *owner)
+    void notify(Class *owner, T *oldValue=nullptr)
     {
         d.priv.notifyObservers(&d);
-        (owner->*Callback)();
+        if constexpr(std::is_invocable_v<decltype(Callback), Class>)
+            (owner->*Callback)();
+        else
+            (owner->*Callback)(*oldValue);
     }
 
     Q_DISABLE_COPY_MOVE(QNotifiedProperty)
@@ -624,7 +675,7 @@ QPropertyChangeHandler<Functor> QProperty<T>::subscribe(Functor f)
     return onValueChanged(f);
 }
 
-template <typename T, typename Class, void(Class::*Callback)()>
+template <typename T, auto Callback>
 template<typename Functor>
 QPropertyChangeHandler<Functor> QNotifiedProperty<T, Callback>::onValueChanged(Functor f)
 {
@@ -634,7 +685,7 @@ QPropertyChangeHandler<Functor> QNotifiedProperty<T, Callback>::onValueChanged(F
     return QPropertyChangeHandler<Functor>(*this, f);
 }
 
-template <typename T, typename Class, void(Class::*Callback)()>
+template <typename T, auto Callback>
 template<typename Functor>
 QPropertyChangeHandler<Functor> QNotifiedProperty<T, Callback>::subscribe(Functor f)
 {
diff --git a/src/corelib/kernel/qpropertybinding.cpp b/src/corelib/kernel/qpropertybinding.cpp
index cbb9752e2e..3a0f54b44a 100644
--- a/src/corelib/kernel/qpropertybinding.cpp
+++ b/src/corelib/kernel/qpropertybinding.cpp
@@ -69,8 +69,15 @@ void QPropertyBindingPrivate::markDirtyAndNotifyObservers()
     dirty = true;
     if (firstObserver)
         firstObserver.notify(this, propertyDataPtr);
-    if (hasStaticObserver)
-        staticObserverCallback(staticObserver);
+    if (hasStaticObserver) {
+        if (metaType == QMetaType::fromType<bool>()) {
+            auto propertyPtr = reinterpret_cast<QPropertyBase *>(propertyDataPtr);
+            bool oldValue = propertyPtr->extraBit();
+            staticObserverCallback(staticObserver, &oldValue);
+        } else {
+            staticObserverCallback(staticObserver, propertyDataPtr);
+        }
+    }
 }
 
 bool QPropertyBindingPrivate::evaluateIfDirtyAndReturnTrueIfValueChanged()
diff --git a/src/corelib/kernel/qpropertybinding_p.h b/src/corelib/kernel/qpropertybinding_p.h
index e03a23109a..99a6b22b4b 100644
--- a/src/corelib/kernel/qpropertybinding_p.h
+++ b/src/corelib/kernel/qpropertybinding_p.h
@@ -80,7 +80,7 @@ private:
         ObserverArray inlineDependencyObservers;
         struct {
             void *staticObserver;
-            void (*staticObserverCallback)(void*);
+            void (*staticObserverCallback)(void*, void*);
         };
     };
     QScopedPointer<std::vector<QPropertyObserver>> heapObservers;
@@ -107,7 +107,7 @@ public:
 
     void setDirty(bool d) { dirty = d; }
     void setProperty(void *propertyPtr) { propertyDataPtr = propertyPtr; }
-    void setStaticObserver(void *observer, void (*callback)(void*))
+    void setStaticObserver(void *observer, void (*callback)(void*, void*))
     {
         if (observer) {
             if (!hasStaticObserver) {
diff --git a/src/corelib/kernel/qpropertyprivate.h b/src/corelib/kernel/qpropertyprivate.h
index 7a2c6a5b77..43318f25c0 100644
--- a/src/corelib/kernel/qpropertyprivate.h
+++ b/src/corelib/kernel/qpropertyprivate.h
@@ -84,7 +84,7 @@ public:
 
     QUntypedPropertyBinding setBinding(const QUntypedPropertyBinding &newBinding,
                                        void *propertyDataPtr, void *staticObserver = nullptr,
-                                       void (*staticObserverCallback)(void*) = nullptr);
+                                       void (*staticObserverCallback)(void *, void *) = nullptr);
     QPropertyBindingPrivate *binding();
 
     void evaluateIfDirty();