libpyside: Split DynamicSlotDataV2 into separate classes

DynamicSlotDataV2 provided the callable for the slot invocation and
contained switches depending on whether the callable was a method or a
plain callable.

Turn it into a virtual base class providing a call()
method and a factory to cleanly separate the code paths for method and
plain callables. Introduce an enumeration of slot types for clarity.
Expose it in globalreceiverv2.h so that it can be used
for PySideQSlotObject.

Task-number: PYSIDE-2810
Change-Id: I4bdfb58a430c39aba2cc531c28129f71ebeb498c
Reviewed-by: Christian Tismer <tismer@stackless.com>

5 files changed, 282 insertions, 149 deletions

diff --git a/sources/pyside6/libpyside/CMakeLists.txt b/sources/pyside6/libpyside/CMakeLists.txt
index ebfe123dd..b14a0ab52 100644
--- a/sources/pyside6/libpyside/CMakeLists.txt
+++ b/sources/pyside6/libpyside/CMakeLists.txt
@@ -10,6 +10,7 @@ set(CMAKE_AUTOMOC ON)
 set(libpyside_HEADERS # installed below
     class_property.h
     dynamicqmetaobject.h
+    dynamicslot_p.h
     feature_select.h
     globalreceiverv2.h
     pysideclassdecorator_p.h
@@ -46,6 +47,7 @@ set(libpyside_HEADERS # installed below
 set(libpyside_SRC
     class_property.cpp
     dynamicqmetaobject.cpp
+    dynamicslot.cpp
     feature_select.cpp
     signalmanager.cpp
     globalreceiverv2.cpp
diff --git a/sources/pyside6/libpyside/dynamicslot.cpp b/sources/pyside6/libpyside/dynamicslot.cpp
new file mode 100644
index 000000000..182c2949f
--- /dev/null
+++ b/sources/pyside6/libpyside/dynamicslot.cpp
@@ -0,0 +1,220 @@
+// Copyright (C) 2024 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only
+
+#include "dynamicslot_p.h"
+#include "globalreceiverv2.h" // for GlobalReceiverMethodSlot
+#include "pysidestaticstrings.h"
+#include "pysideutils.h"
+#include "pysideweakref.h"
+#include "signalmanager.h"
+
+#include <autodecref.h>
+#include <gilstate.h>
+#include <pep384ext.h>
+
+#include <QtCore/QDebug>
+
+namespace PySide
+{
+
+DynamicSlot::SlotType DynamicSlot::slotType(PyObject *callback)
+{
+    if (PyMethod_Check(callback) != 0)
+        return SlotType::Method;
+    if (PySide::isCompiledMethod(callback) != 0)
+        return SlotType::CompiledMethod;
+    return SlotType::Callable;
+}
+
+// Simple callable slot.
+class CallbackDynamicSlot : public DynamicSlot
+{
+    Q_DISABLE_COPY_MOVE(CallbackDynamicSlot)
+public:
+    explicit CallbackDynamicSlot(PyObject *callback) noexcept;
+    ~CallbackDynamicSlot() override;
+
+    void call(const QByteArrayList &parameterTypes, const char *returnType,
+              void **cppArgs) override;
+    void formatDebug(QDebug &debug) const override;
+
+private:
+    PyObject *m_callback;
+};
+
+CallbackDynamicSlot::CallbackDynamicSlot(PyObject *callback) noexcept :
+    m_callback(callback)
+{
+    Py_INCREF(m_callback);
+}
+
+CallbackDynamicSlot::~CallbackDynamicSlot()
+{
+    Shiboken::GilState gil;
+    Py_DECREF(m_callback);
+}
+
+void CallbackDynamicSlot::call(const QByteArrayList &parameterTypes, const char *returnType,
+                               void **cppArgs)
+{
+    SignalManager::callPythonMetaMethod(parameterTypes, returnType, cppArgs, m_callback);
+}
+
+void CallbackDynamicSlot::formatDebug(QDebug &debug) const
+{
+    debug << "CallbackDynamicSlot(" << PySide::debugPyObject(m_callback) << ')';
+}
+
+// A method given by "signal.connect(foo.method)" is a temporarily created
+// callable/partial function where self is bound as a first parameter.
+// It can be split into self and the function. Keeping a reference on
+// the callable itself would prevent object deletion. Instead, keep a
+// reference on the function.
+class MethodDynamicSlot : public DynamicSlot
+{
+    Q_DISABLE_COPY_MOVE(MethodDynamicSlot)
+public:
+
+    explicit MethodDynamicSlot(PyObject *function, PyObject *pythonSelf);
+    ~MethodDynamicSlot() override;
+
+    void call(const QByteArrayList &parameterTypes, const char *returnType,
+              void **cppArgs) override;
+    void formatDebug(QDebug &debug) const override;
+
+private:
+    PyObject *m_function;
+    PyObject *m_pythonSelf;
+};
+
+MethodDynamicSlot::MethodDynamicSlot(PyObject *function, PyObject *pythonSelf) :
+    m_function(function),
+    m_pythonSelf(pythonSelf)
+{
+}
+
+MethodDynamicSlot::~MethodDynamicSlot()
+{
+    Shiboken::GilState gil;
+    Py_DECREF(m_function);
+}
+
+void MethodDynamicSlot::call(const QByteArrayList &parameterTypes, const char *returnType,
+                             void **cppArgs)
+{
+    // create a callback based on method data
+    Shiboken::AutoDecRef callable(PepExt_Type_CallDescrGet(m_function,
+                                                           m_pythonSelf, nullptr));
+    SignalManager::callPythonMetaMethod(parameterTypes, returnType,
+                                        cppArgs, callable.object());
+}
+
+void MethodDynamicSlot::formatDebug(QDebug &debug) const
+{
+    debug << "MethodDynamicSlot(self=" << PySide::debugPyObject(m_pythonSelf)
+        << ", function=" << PySide::debugPyObject(m_function) << ')';
+}
+
+// Store a weak reference on pythonSelf.
+class TrackingMethodDynamicSlot : public MethodDynamicSlot
+{
+    Q_DISABLE_COPY_MOVE(TrackingMethodDynamicSlot)
+public:
+    explicit TrackingMethodDynamicSlot(PyObject *function, PyObject *pythonSelf,
+                                       PyObject *weakRef);
+    ~TrackingMethodDynamicSlot() override;
+
+    void releaseWeakRef() { m_weakRef = nullptr; }
+
+private:
+    PyObject *m_weakRef;
+};
+
+TrackingMethodDynamicSlot::TrackingMethodDynamicSlot(PyObject *function, PyObject *pythonSelf,
+                                                     PyObject *weakRef) :
+    MethodDynamicSlot(function, pythonSelf),
+    m_weakRef(weakRef)
+{
+}
+
+TrackingMethodDynamicSlot::~TrackingMethodDynamicSlot()
+{
+    Shiboken::GilState gil;
+    Py_XDECREF(m_weakRef);
+    m_weakRef = nullptr;
+}
+
+// Delete the GlobalReceiver on pythonSelf deletion
+class GlobalReceiverMethodSlot : public TrackingMethodDynamicSlot
+{
+    Q_DISABLE_COPY_MOVE(GlobalReceiverMethodSlot)
+public:
+    explicit GlobalReceiverMethodSlot(PyObject *function, PyObject *pythonSelf,
+                                      GlobalReceiverV2 *parent);
+    ~GlobalReceiverMethodSlot() override = default;
+
+    GlobalReceiverV2 *parent() const { return m_parent; }
+
+private:
+    GlobalReceiverV2 *m_parent;
+};
+
+static void onGlobalReceiverSlotDestroyed(void *data)
+{
+    auto *self = reinterpret_cast<GlobalReceiverMethodSlot *>(data);
+    self->releaseWeakRef();
+    Py_BEGIN_ALLOW_THREADS
+    SignalManager::deleteGlobalReceiver(self->parent());
+    Py_END_ALLOW_THREADS
+}
+
+// monitor class from method lifetime
+GlobalReceiverMethodSlot::GlobalReceiverMethodSlot(PyObject *function, PyObject *pythonSelf,
+                                                   GlobalReceiverV2 *parent) :
+    TrackingMethodDynamicSlot(function, pythonSelf,
+                               WeakRef::create(pythonSelf, onGlobalReceiverSlotDestroyed, this)),
+    m_parent(parent)
+{
+}
+
+DynamicSlot* DynamicSlot::create(PyObject *callback, GlobalReceiverV2 *parent)
+{
+    Shiboken::GilState gil;
+    switch (slotType(callback)) {
+    case SlotType::Method: {
+        PyObject *function = PyMethod_GET_FUNCTION(callback);
+        Py_INCREF(function);
+        PyObject *pythonSelf = PyMethod_GET_SELF(callback);
+        if (parent != nullptr)
+            return new GlobalReceiverMethodSlot(function, pythonSelf, parent);
+        return new MethodDynamicSlot(function, pythonSelf);
+    }
+    case SlotType::CompiledMethod: {
+        // PYSIDE-1523: PyMethod_Check is not accepting compiled form, we just go by attributes.
+        PyObject *function = PyObject_GetAttr(callback, PySide::PySideName::im_func());
+        Py_DECREF(function);
+        PyObject *pythonSelf = PyObject_GetAttr(callback, PySide::PySideName::im_self());
+        Py_DECREF(pythonSelf);
+        if (parent != nullptr)
+            return new GlobalReceiverMethodSlot(function, pythonSelf, parent);
+        return new MethodDynamicSlot(function, pythonSelf);
+    }
+    case SlotType::Callable:
+        break;
+    }
+    return new CallbackDynamicSlot(callback);
+}
+
+QDebug operator<<(QDebug debug, const DynamicSlot *ds)
+{
+    QDebugStateSaver saver(debug);
+    debug.noquote();
+    debug.nospace();
+    if (ds != nullptr)
+        ds->formatDebug(debug);
+    else
+        debug << "DynamicSlot(0";
+    return debug;
+}
+
+} // namespace PySide
diff --git a/sources/pyside6/libpyside/dynamicslot_p.h b/sources/pyside6/libpyside/dynamicslot_p.h
new file mode 100644
index 000000000..7f35b7d7b
--- /dev/null
+++ b/sources/pyside6/libpyside/dynamicslot_p.h
@@ -0,0 +1,46 @@
+// Copyright (C) 2024 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only
+
+#ifndef DYNAMICSLOT_P_H
+#define DYNAMICSLOT_P_H
+
+#include <sbkpython.h>
+
+#include <QtCore/QtCompare>
+
+QT_FORWARD_DECLARE_CLASS(QDebug)
+
+namespace PySide
+{
+
+class GlobalReceiverV2;
+
+class DynamicSlot
+{
+    Q_DISABLE_COPY_MOVE(DynamicSlot)
+public:
+    enum SlotType
+    {
+        Callable,
+        Method,
+        CompiledMethod
+    };
+
+    virtual ~DynamicSlot() = default;
+
+    virtual void call(const QByteArrayList &parameterTypes, const char *returnType,
+                      void **cppArgs) = 0;
+    virtual void formatDebug(QDebug &debug) const = 0;
+
+    static SlotType slotType(PyObject *callback);
+    static DynamicSlot *create(PyObject *callback, GlobalReceiverV2 *parent = nullptr);
+
+protected:
+    DynamicSlot() noexcept = default;
+};
+
+QDebug operator<<(QDebug debug, const DynamicSlot *ds);
+
+}
+
+#endif // DYNAMICSLOT_P_H
diff --git a/sources/pyside6/libpyside/globalreceiverv2.cpp b/sources/pyside6/libpyside/globalreceiverv2.cpp
index 4a061d566..41ed35263 100644
--- a/sources/pyside6/libpyside/globalreceiverv2.cpp
+++ b/sources/pyside6/libpyside/globalreceiverv2.cpp
@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only
 
 #include "globalreceiverv2.h"
+#include "dynamicslot_p.h"
 #include "pysideweakref.h"
 #include "pysidestaticstrings.h"
 #include "pysideutils.h"
@@ -16,157 +17,38 @@
 #include <QtCore/QDebug>
 
 #include <cstring>
+#include <utility>
 
 namespace PySide
 {
 
-class DynamicSlotDataV2
-{
-    Q_DISABLE_COPY_MOVE(DynamicSlotDataV2)
-    public:
-        DynamicSlotDataV2(PyObject *callback, GlobalReceiverV2 *parent);
-        ~DynamicSlotDataV2();
-
-        PyObject *callback();
-        GlobalReceiverKey key() const { return {m_pythonSelf, m_callback}; }
-
-        static void onCallbackDestroyed(void *data);
-        static GlobalReceiverKey key(PyObject *callback);
-
-        void formatDebug(QDebug &debug) const;
-
-    private:
-        bool m_isMethod;
-        PyObject *m_callback;
-        PyObject *m_pythonSelf = nullptr;
-        PyObject *m_pyClass = nullptr;
-        PyObject *m_weakRef = nullptr;
-        GlobalReceiverV2 *m_parent;
-};
-
-void DynamicSlotDataV2::formatDebug(QDebug &debug) const
-{
-    debug << "method=" << m_isMethod << ", m_callback=" << m_callback;
-    if (m_callback != nullptr)
-        debug << '/' << Py_TYPE(m_callback)->tp_name;
-    debug << ", self=" << m_pythonSelf;
-    if (m_pythonSelf != nullptr)
-        debug << '/' << Py_TYPE(m_pythonSelf)->tp_name;
-    debug << ", m_pyClass=" << m_pyClass;
-    if (m_pyClass != nullptr)
-        debug << '/' << Py_TYPE(m_pyClass)->tp_name;
-}
-
-QDebug operator<<(QDebug debug, const DynamicSlotDataV2 *d)
-{
-    QDebugStateSaver saver(debug);
-    debug.noquote();
-    debug.nospace();
-    debug << "DynamicSlotDataV2(";
-    if (d)
-        d->formatDebug(debug);
-    else
-        debug << '0';
-    debug << ')';
-    return debug;
-}
-
-using namespace PySide;
-
-DynamicSlotDataV2::DynamicSlotDataV2(PyObject *callback, GlobalReceiverV2 *parent) :
-    m_parent(parent)
-{
-    Shiboken::GilState gil;
-
-    if (PyMethod_Check(callback)) {
-        m_isMethod = true;
-        // A method given by "signal.connect(foo.method)" is a temporarily created
-        // callable/partial function where self is bound as a first parameter.
-        // It can be split into self and the function. Keeping a reference on
-        // the callable itself would prevent object deletion. Instead, keep a
-        // reference on the function and listen for destruction of the object
-        // using a weak reference with notification.
-        m_callback = PyMethod_GET_FUNCTION(callback);
-        Py_INCREF(m_callback);
-        m_pythonSelf = PyMethod_GET_SELF(callback);
-
-        m_weakRef = WeakRef::create(m_pythonSelf, DynamicSlotDataV2::onCallbackDestroyed, this);
-    } else if (PySide::isCompiledMethod(callback)) {
-        // PYSIDE-1523: PyMethod_Check is not accepting compiled form, we just go by attributes.
-        m_isMethod = true;
-
-        m_callback = PyObject_GetAttr(callback, PySide::PySideName::im_func());
-        Py_DECREF(m_callback);
-
-        m_pythonSelf = PyObject_GetAttr(callback, PySide::PySideName::im_self());
-        Py_DECREF(m_pythonSelf);
-
-        //monitor class from method lifetime
-        m_weakRef = WeakRef::create(m_pythonSelf, DynamicSlotDataV2::onCallbackDestroyed, this);
-    } else {
-        m_isMethod = false;
-
-        m_callback = callback;
-        Py_INCREF(m_callback);
-    }
-}
-
-GlobalReceiverKey DynamicSlotDataV2::key(PyObject *callback)
+GlobalReceiverKey GlobalReceiverV2::key(PyObject *callback)
 {
     Shiboken::GilState gil;
-    if (PyMethod_Check(callback)) {
+    switch (DynamicSlot::slotType(callback)) {
+    case DynamicSlot::SlotType::Method:
         // PYSIDE-1422: Avoid hash on self which might be unhashable.
         return {PyMethod_GET_SELF(callback), PyMethod_GET_FUNCTION(callback)};
-    }
-    if (PySide::isCompiledMethod(callback)) {
+    case DynamicSlot::SlotType::CompiledMethod: {
         // PYSIDE-1589: Fix for slots in compiled functions
         Shiboken::AutoDecRef self(PyObject_GetAttr(callback, PySide::PySideName::im_self()));
         Shiboken::AutoDecRef func(PyObject_GetAttr(callback, PySide::PySideName::im_func()));
         return {self, func};
     }
+    case DynamicSlot::SlotType::Callable:
+        break;
+    }
     return {nullptr, callback};
 }
 
-PyObject *DynamicSlotDataV2::callback()
-{
-    PyObject *callback = m_callback;
-
-    //create a callback based on method data
-    if (m_isMethod)
-        callback = PepExt_Type_CallDescrGet(m_callback, m_pythonSelf, nullptr);
-    else
-        Py_INCREF(callback);
-
-    return callback;
-}
-
-void DynamicSlotDataV2::onCallbackDestroyed(void *data)
-{
-    auto *self = reinterpret_cast<DynamicSlotDataV2 *>(data);
-    self->m_weakRef = nullptr;
-    Py_BEGIN_ALLOW_THREADS
-    SignalManager::deleteGlobalReceiver(self->m_parent);
-    Py_END_ALLOW_THREADS
-}
-
-DynamicSlotDataV2::~DynamicSlotDataV2()
-{
-    Shiboken::GilState gil;
-
-    Py_XDECREF(m_weakRef);
-    m_weakRef = nullptr;
-
-    Py_DECREF(m_callback);
-}
-
 const char *GlobalReceiverV2::senderDynamicProperty = "_q_pyside_sender";
 
 GlobalReceiverV2::GlobalReceiverV2(PyObject *callback, QObject *receiver) :
     QObject(nullptr),
     m_metaObject("__GlobalReceiver__", &QObject::staticMetaObject),
+    m_data(DynamicSlot::create(callback, this)),
     m_receiver(receiver)
 {
-    m_data = new DynamicSlotDataV2(callback, this);
 }
 
 GlobalReceiverV2::~GlobalReceiverV2()
@@ -180,9 +62,7 @@ GlobalReceiverV2::~GlobalReceiverV2()
     // Callback is deleted, hence the last reference is decremented,
     // leading to the object being deleted, which emits destroyed(), which would try to invoke
     // the already deleted callback, and also try to delete the object again.
-    DynamicSlotDataV2 *data = m_data;
-    m_data = nullptr;
-    delete data;
+    delete std::exchange(m_data, nullptr);
 }
 
 int GlobalReceiverV2::addSlot(const QByteArray &signature)
@@ -227,16 +107,6 @@ bool GlobalReceiverV2::isEmpty() const
     return std::all_of(m_refs.cbegin(), m_refs.cend(), isNull);
 }
 
-GlobalReceiverKey GlobalReceiverV2::key() const
-{
-    return m_data->key();
-}
-
-GlobalReceiverKey GlobalReceiverV2::key(PyObject *callback)
-{
-    return DynamicSlotDataV2::key(callback);
-}
-
 const QMetaObject *GlobalReceiverV2::metaObject() const
 {
     return const_cast<GlobalReceiverV2 *>(this)->m_metaObject.update();
@@ -262,8 +132,7 @@ int GlobalReceiverV2::qt_metacall(QMetaObject::Call call, int id, void **args)
     if (setSenderDynamicProperty)
         m_receiver->setProperty(senderDynamicProperty, QVariant::fromValue(sender()));
 
-    Shiboken::AutoDecRef callback(m_data->callback());
-    SignalManager::callPythonMetaMethod(slot, args, callback);
+    m_data->call(slot.parameterTypes(), slot.typeName(), args);
 
     if (setSenderDynamicProperty)
         m_receiver->setProperty(senderDynamicProperty, QVariant{});
diff --git a/sources/pyside6/libpyside/globalreceiverv2.h b/sources/pyside6/libpyside/globalreceiverv2.h
index d2c09cf1f..351007596 100644
--- a/sources/pyside6/libpyside/globalreceiverv2.h
+++ b/sources/pyside6/libpyside/globalreceiverv2.h
@@ -22,7 +22,7 @@ QT_FORWARD_DECLARE_CLASS(QDebug);
 namespace PySide
 {
 
-class DynamicSlotDataV2;
+class DynamicSlot;
 class GlobalReceiverV2;
 
 struct GlobalReceiverKey
@@ -82,10 +82,6 @@ public:
     /// Returns whether any senders are registered.
     bool isEmpty() const;
 
-    /// Use to retrieve the unique hash of this GlobalReceiver object
-    /// @return hash key
-    GlobalReceiverKey key() const;
-
     /// Use to retrieve the unique hash of the PyObject based on GlobalReceiver rules
     /// @param callback The Python callable object used to calculate the id
     /// @return hash key
@@ -102,7 +98,7 @@ private:
     void purgeDeletedSenders();
 
     MetaObjectBuilder m_metaObject;
-    DynamicSlotDataV2 *m_data;
+    DynamicSlot *m_data;
     using QObjectPointer = QPointer<const QObject>;
     QList<QObjectPointer> m_refs;
     QPointer<QObject> m_receiver;