Fix and compactify QNotifiedProperty

The static observer can live in a union with the inline observers. We
only need to take care of calling the ctors and dtors manually then.

In order for any observers to be called in the presence of a static
observer, the static observer has to be called after the other
observers.

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

3 files changed, 82 insertions, 14 deletions

diff --git a/src/corelib/kernel/qproperty.h b/src/corelib/kernel/qproperty.h
index e2a6172d84..f0156fac02 100644
--- a/src/corelib/kernel/qproperty.h
+++ b/src/corelib/kernel/qproperty.h
@@ -590,6 +590,17 @@ public:
     {
         setSource(property);
     }
+
+    template <typename PropertyType, typename Class, void(Class::*Callback)()>
+    QPropertyChangeHandler(const QNotifiedProperty<PropertyType, Callback> &property, Functor handler)
+        : QPropertyObserver([](QPropertyObserver *self, void *) {
+              auto This = static_cast<QPropertyChangeHandler<Functor>*>(self);
+              This->m_handler();
+          })
+        , m_handler(handler)
+    {
+        setSource(property);
+    }
 };
 
 template <typename T>
@@ -613,6 +624,27 @@ QPropertyChangeHandler<Functor> QProperty<T>::subscribe(Functor f)
     return onValueChanged(f);
 }
 
+template <typename T, typename Class, void(Class::*Callback)()>
+template<typename Functor>
+QPropertyChangeHandler<Functor> QNotifiedProperty<T, Callback>::onValueChanged(Functor f)
+{
+#if defined(__cpp_lib_is_invocable) && (__cpp_lib_is_invocable >= 201703L)
+    static_assert(std::is_invocable_v<Functor>, "Functor callback must be callable without any parameters");
+#endif
+    return QPropertyChangeHandler<Functor>(*this, f);
+}
+
+template <typename T, typename Class, void(Class::*Callback)()>
+template<typename Functor>
+QPropertyChangeHandler<Functor> QNotifiedProperty<T, Callback>::subscribe(Functor f)
+{
+#if defined(__cpp_lib_is_invocable) && (__cpp_lib_is_invocable >= 201703L)
+    static_assert(std::is_invocable_v<Functor>, "Functor callback must be callable without any parameters");
+#endif
+    f();
+    return onValueChanged(f);
+}
+
 template <auto propertyMember, auto callbackMember>
 struct QPropertyMemberChangeHandler;
 
diff --git a/src/corelib/kernel/qpropertybinding.cpp b/src/corelib/kernel/qpropertybinding.cpp
index 55f2fe913d..9c46d63998 100644
--- a/src/corelib/kernel/qpropertybinding.cpp
+++ b/src/corelib/kernel/qpropertybinding.cpp
@@ -51,6 +51,8 @@ QPropertyBindingPrivate::~QPropertyBindingPrivate()
 {
     if (firstObserver)
         firstObserver.unlink();
+    if (!hasStaticObserver)
+        inlineDependencyObservers.~ObserverArray(); // Explicit because of union.
 }
 
 void QPropertyBindingPrivate::unlinkAndDeref()
@@ -63,10 +65,10 @@ void QPropertyBindingPrivate::unlinkAndDeref()
 void QPropertyBindingPrivate::markDirtyAndNotifyObservers()
 {
     dirty = true;
-    if (staticObserver)
-        staticObserverCallback(staticObserver);
     if (firstObserver)
         firstObserver.notify(this, propertyDataPtr);
+    if (hasStaticObserver)
+        staticObserverCallback(staticObserver);
 }
 
 bool QPropertyBindingPrivate::evaluateIfDirtyAndReturnTrueIfValueChanged()
diff --git a/src/corelib/kernel/qpropertybinding_p.h b/src/corelib/kernel/qpropertybinding_p.h
index a2c733abd9..e03a23109a 100644
--- a/src/corelib/kernel/qpropertybinding_p.h
+++ b/src/corelib/kernel/qpropertybinding_p.h
@@ -66,14 +66,24 @@ class Q_CORE_EXPORT QPropertyBindingPrivate : public QSharedData
 private:
     friend struct QPropertyBasePointer;
 
+    using ObserverArray = std::array<QPropertyObserver, 4>;
+
+    // QSharedData is 4 bytes. Use the padding for the bools as we need 8 byte alignment below.
+    bool dirty = false;
+    bool updating = false;
+    bool hasStaticObserver = false;
+
     QUntypedPropertyBinding::BindingEvaluationFunction evaluationFunction;
 
     QPropertyObserverPointer firstObserver;
-    std::array<QPropertyObserver, 4> inlineDependencyObservers;
+    union {
+        ObserverArray inlineDependencyObservers;
+        struct {
+            void *staticObserver;
+            void (*staticObserverCallback)(void*);
+        };
+    };
     QScopedPointer<std::vector<QPropertyObserver>> heapObservers;
-    // ### merge with inline dependency observer storage
-    void *staticObserver = nullptr;
-    void (*staticObserverCallback)(void*) = nullptr;
 
     void *propertyDataPtr = nullptr;
 
@@ -82,9 +92,6 @@ private:
 
     QMetaType metaType;
 
-    bool dirty = false;
-    bool updating = false;
-
 public:
     // public because the auto-tests access it, too.
     size_t dependencyObserverCount = 0;
@@ -92,6 +99,7 @@ public:
     QPropertyBindingPrivate(const QMetaType &metaType, QUntypedPropertyBinding::BindingEvaluationFunction evaluationFunction,
                             const QPropertyBindingSourceLocation &location)
         : evaluationFunction(std::move(evaluationFunction))
+        , inlineDependencyObservers() // Explicit initialization required because of union
         , location(location)
         , metaType(metaType)
     {}
@@ -99,7 +107,31 @@ public:
 
     void setDirty(bool d) { dirty = d; }
     void setProperty(void *propertyPtr) { propertyDataPtr = propertyPtr; }
-    void setStaticObserver(void *observer, void (*callback)(void*)) { staticObserver = observer; staticObserverCallback = callback; }
+    void setStaticObserver(void *observer, void (*callback)(void*))
+    {
+        if (observer) {
+            if (!hasStaticObserver) {
+                if (dependencyObserverCount > 0) {
+                    if (!heapObservers)
+                        heapObservers.reset(new std::vector<QPropertyObserver>());
+                    for (int i = 0, end = qMin(dependencyObserverCount, inlineDependencyObservers.size()); i < end; ++i)
+                        heapObservers->push_back(std::move(inlineDependencyObservers[i]));
+                }
+                inlineDependencyObservers.~ObserverArray();
+            }
+
+            hasStaticObserver = true;
+            staticObserver = observer;
+            staticObserverCallback = callback;
+        } else if (hasStaticObserver) {
+            hasStaticObserver = false;
+            new (&inlineDependencyObservers) ObserverArray();
+            for (int i = 0, end = qMin(dependencyObserverCount, inlineDependencyObservers.size()); i < end; ++i) {
+                inlineDependencyObservers[i] = std::move(heapObservers->back());
+                heapObservers->pop_back();
+            }
+        }
+    }
     void prependObserver(QPropertyObserverPointer observer) {
         observer.ptr->prev = const_cast<QPropertyObserver **>(&firstObserver.ptr);
         firstObserver = observer;
@@ -113,16 +145,18 @@ public:
     }
 
     void clearDependencyObservers() {
-        for (size_t i = 0; i < inlineDependencyObservers.size(); ++i) {
-            QPropertyObserver empty;
-            qSwap(inlineDependencyObservers[i], empty);
+        if (!hasStaticObserver) {
+            for (size_t i = 0; i < inlineDependencyObservers.size(); ++i) {
+                QPropertyObserver empty;
+                qSwap(inlineDependencyObservers[i], empty);
+            }
         }
         if (heapObservers)
             heapObservers->clear();
         dependencyObserverCount = 0;
     }
     QPropertyObserverPointer allocateDependencyObserver() {
-        if (dependencyObserverCount < inlineDependencyObservers.size()) {
+        if (!hasStaticObserver && dependencyObserverCount < inlineDependencyObservers.size()) {
             ++dependencyObserverCount;
             return {&inlineDependencyObservers[dependencyObserverCount - 1]};
         }