Support the qApp macro in "scriptable application"

Renamed from "Fix scriptable application to support the qApp macro"
because qApp was improved instead of scriptable application.

The qApp macro needed some extra effort to support the
qApp "macro" which is only defined in the Python wrappers.

I took some generated code, created a QApplication instance
in Python and used then reduced generated code to get at the
object and adjust the refcount.

This solution was then rejected, because I can do better,
and in fact, scriptable application now has a correct qApp
macro too, without any change to scriptable application.

The central idea was to look into the module init function
at import time and to see if a Q*Application already exists.
I was not aware of that import. Many thanks for the rejection! :-)

Update..
--------
After many attempts to make the qApp variable correctly behave
like always, I recognized that pre-existing Q*Application instances
have no wrappers or constructors at all! With that, it is not
possible to create a sophisticated qApp macro as a singleton
variable in the desired way.

Fortunately, this is also not necessary, because a C++ Q*Application
cannot be deleted from Python, and there is no point in supporting
more that a simple variable. So in case of a pre-existing instance,
the qApp variable now gets redirected to that instance.

A small test was added to application_test.py that is triggered by
an import.

A weird effect when "qApp" was typed interactively before calling
"QApplication()" was fixed, too.

Change-Id: Ic69dd6a21c964838a90f63e316d299b62a54d612
Fixes: PYSIDE-571
Reviewed-by: Cristian Maureira-Fredes <cristian.maureira-fredes@qt.io>
 
 
 
 

5 files changed, 47 insertions, 15 deletions

diff --git a/sources/pyside2/PySide2/glue/qtcore.cpp b/sources/pyside2/PySide2/glue/qtcore.cpp
index f30607f6b..9db4e2e82 100644
--- a/sources/pyside2/PySide2/glue/qtcore.cpp
+++ b/sources/pyside2/PySide2/glue/qtcore.cpp
@@ -1340,18 +1340,17 @@ if (!PyTuple_SetItem(empty, 0, PyList_New(0))) {
 // @snippet qcoreapplication-2
 
 // @snippet qcoreapplication-instance
-QCoreApplication *app = QCoreApplication::instance();
 PyObject *pyApp = Py_None;
-if (app) {
+if (qApp) {
     pyApp = reinterpret_cast<PyObject*>(
-        Shiboken::BindingManager::instance().retrieveWrapper(app));
+        Shiboken::BindingManager::instance().retrieveWrapper(qApp));
     if (!pyApp)
-        pyApp = %CONVERTTOPYTHON[QCoreApplication*](app);
+        pyApp = %CONVERTTOPYTHON[QCoreApplication*](qApp);
         // this will keep app live after python exit (extra ref)
 }
 // PYSIDE-571: make sure that we return the singleton "None"
 if (pyApp == Py_None)
-    Py_DECREF(MakeSingletonQAppWrapper(0)); // here qApp and instance() diverge
+    Py_DECREF(MakeSingletonQAppWrapper(nullptr)); // here qApp and instance() diverge
 %PYARG_0 = pyApp;
 Py_XINCREF(%PYARG_0);
 // @snippet qcoreapplication-instance
diff --git a/sources/pyside2/tests/QtWidgets/application_test.py b/sources/pyside2/tests/QtWidgets/application_test.py
index bd0f94125..0b8f73cd6 100644
--- a/sources/pyside2/tests/QtWidgets/application_test.py
+++ b/sources/pyside2/tests/QtWidgets/application_test.py
@@ -31,19 +31,25 @@
 import unittest
 from testbinding import TestObject
 from PySide2.QtWidgets import QApplication
+from PySide2 import __all__ as all
 
 class QApplicationInstance(unittest.TestCase):
 
     def appDestroyed(self):
-        sefl.assertTrue(False)
+        self.assertTrue(False)
 
     def testInstanceObject(self):
+        self.assertEqual(type(qApp), type(None))
         TestObject.createApp()
         app1 = QApplication.instance()
         app2 = QApplication.instance()
         app1.setObjectName("MyApp")
         self.assertEqual(app1, app2)
         self.assertEqual(app2.objectName(), app1.objectName())
+        if len(all) > 3:
+            # an import triggers qApp initialization
+            __import__("PySide2." + all[-1])
+            self.assertEqual(app1, qApp)
         app1.destroyed.connect(self.appDestroyed)
 
 if __name__ == '__main__':
diff --git a/sources/shiboken2/generator/shiboken2/cppgenerator.cpp b/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
index 8ee0a9cf2..d1da3a4ba 100644
--- a/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
+++ b/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
@@ -5806,7 +5806,7 @@ bool CppGenerator::finishGeneration()
 
     if (usePySideExtensions()) {
         // initialize the qApp module.
-        s << INDENT << "NotifyModuleForQApp(module);" << endl;
+        s << INDENT << "NotifyModuleForQApp(module, qApp);" << endl;
     }
     s << endl;
     s << "SBK_MODULE_INIT_FUNCTION_END" << endl;
diff --git a/sources/shiboken2/libshiboken/qapp_macro.cpp b/sources/shiboken2/libshiboken/qapp_macro.cpp
index 19e985b20..ea9cf0c86 100644
--- a/sources/shiboken2/libshiboken/qapp_macro.cpp
+++ b/sources/shiboken2/libshiboken/qapp_macro.cpp
@@ -38,6 +38,7 @@
 ****************************************************************************/
 
 #include "basewrapper.h"
+#include "autodecref.h"
 
 extern "C"
 {
@@ -93,13 +94,14 @@ static int qApp_var_ref = 0;
 static int qApp_content_ref = 0;
 
 static int
-reset_qApp_var()
+reset_qApp_var(void)
 {
     PyObject **mod_ptr;
 
-    for (mod_ptr = qApp_moduledicts; *mod_ptr != NULL; mod_ptr++) {
+    for (mod_ptr = qApp_moduledicts; *mod_ptr != nullptr; mod_ptr++) {
         // We respect whatever the user may have set.
-        if (PyDict_GetItem(*mod_ptr, qApp_var) == NULL) {
+        PyObject *existing = PyDict_GetItem(*mod_ptr, qApp_var);
+        if (existing == nullptr || Py_TYPE(existing) == Py_NONE_TYPE) {
             if (PyDict_SetItem(*mod_ptr, qApp_var, qApp_content) < 0)
                 return -1;
         }
@@ -135,8 +137,13 @@ MakeSingletonQAppWrapper(PyTypeObject *type)
     if (Py_REFCNT(qApp_content) > qApp_content_ref)
         qApp_content_ref = Py_REFCNT(qApp_content);
 
-    if (Py_TYPE(qApp_content) != Py_NONE_TYPE)
+    if (Py_TYPE(qApp_content) != Py_NONE_TYPE) {
+        // Remove the "_" variable which might hold a reference to qApp.
+        Shiboken::AutoDecRef pymain(PyImport_ImportModule("__main__"));
+        if (pymain.object() && PyObject_HasAttrString(pymain.object(), "_"))
+            PyObject_DelAttrString(pymain.object(), "_");
         Py_REFCNT(qApp_var) = 1; // fuse is armed...
+    }
     if (type == Py_NONE_TYPE) {
         // Debug mode showed that we need to do more than just remove the
         // reference. To keep everything in the right order, it is easiest
@@ -149,8 +156,8 @@ MakeSingletonQAppWrapper(PyTypeObject *type)
         Py_TYPE(qApp_content) = Py_NONE_TYPE;
         Py_REFCNT(qApp_var) = qApp_var_ref;
         Py_REFCNT(qApp_content) = Py_REFCNT(Py_None);
-        if (__moduleShutdown != NULL)
-            Py_DECREF(PyObject_CallFunction(__moduleShutdown, (char *)"()"));
+        if (__moduleShutdown != nullptr)
+            Py_XDECREF(PyObject_CallFunction(__moduleShutdown, const_cast<char *>("()")));
     }
     else
         (void)PyObject_INIT(qApp_content, type);
@@ -216,9 +223,29 @@ setup_qApp_var(PyObject *module)
 }
 
 void
-NotifyModuleForQApp(PyObject *module)
+NotifyModuleForQApp(PyObject *module, void *qApp)
 {
     setup_qApp_var(module);
+    /*
+     * PYSIDE-571: Check if an QApplication instance exists before the import.
+     * This happens in scriptableapplication and application_test.py .
+     *
+     * Crucial Observation
+     * ===================
+     *
+     * A Q*Application object from C++ does not have a wrapper or constructor
+     * like instances created by Python. It makes no sense to support
+     * deletion or special features like qApp resurrection.
+     *
+     * Therefore, the implementation is very simple and just redirects the
+     * qApp_contents variable and assigns the instance, instead of vice-versa.
+     */
+    if (qApp != nullptr) {
+        Shiboken::AutoDecRef pycore(PyImport_ImportModule("PySide2.QtCore"));
+        Shiboken::AutoDecRef coreapp(PyObject_GetAttrString(pycore, "QCoreApplication"));
+        qApp_content = PyObject_CallMethod(coreapp, "instance", "");
+        reset_qApp_var();
+    }
 }
 
 
diff --git a/sources/shiboken2/libshiboken/qapp_macro.h b/sources/shiboken2/libshiboken/qapp_macro.h
index ded892383..be45241de 100644
--- a/sources/shiboken2/libshiboken/qapp_macro.h
+++ b/sources/shiboken2/libshiboken/qapp_macro.h
@@ -46,7 +46,7 @@ extern "C"
 {
 
 LIBSHIBOKEN_API PyObject *MakeSingletonQAppWrapper(PyTypeObject *type);
-LIBSHIBOKEN_API void NotifyModuleForQApp(PyObject *module);
+LIBSHIBOKEN_API void NotifyModuleForQApp(PyObject *module, void *qApp);
 
 } // extern "C"