shiboken6: Generate Python override code for added virtuals

Introduce "Python override" as a special type of user-added function
which will cause a function calling a Python override into the native
wrapper. This can then be called from a virtual that has a signature
which cannot be handled in Python.

Fixes: PYSIDE-2602
Pick-to: 6.7
Change-Id: I5fc44ebe3f585078e87d3230d5e6f4faa67a4ee1
Reviewed-by: Cristian Maureira-Fredes <cristian.maureira-fredes@qt.io>

16 files changed, 198 insertions, 21 deletions

diff --git a/sources/shiboken6/ApiExtractor/abstractmetafunction.cpp b/sources/shiboken6/ApiExtractor/abstractmetafunction.cpp
index f8faba103..11a02f154 100644
--- a/sources/shiboken6/ApiExtractor/abstractmetafunction.cpp
+++ b/sources/shiboken6/ApiExtractor/abstractmetafunction.cpp
@@ -592,6 +592,11 @@ bool AbstractMetaFunction::isUserAdded() const
     return d->m_addedFunction && !d->m_addedFunction->isDeclaration();
 }
 
+bool AbstractMetaFunction::isUserAddedPythonOverride() const
+{
+    return d->m_addedFunction && d->m_addedFunction->isPythonOverride();
+}
+
 bool AbstractMetaFunction::isUserDeclared() const
 {
     return d->m_addedFunction && d->m_addedFunction->isDeclaration();
diff --git a/sources/shiboken6/ApiExtractor/abstractmetafunction.h b/sources/shiboken6/ApiExtractor/abstractmetafunction.h
index bdf5127d1..e252e439d 100644
--- a/sources/shiboken6/ApiExtractor/abstractmetafunction.h
+++ b/sources/shiboken6/ApiExtractor/abstractmetafunction.h
@@ -300,6 +300,7 @@ public:
 
     /// Returns true if the AbstractMetaFunction was added by the user via the type system description.
     bool isUserAdded() const;
+    bool isUserAddedPythonOverride() const;
     /// Returns true if the AbstractMetaFunction was declared by the user via
     /// the type system description.
     bool isUserDeclared() const;
diff --git a/sources/shiboken6/ApiExtractor/abstractmetalang.cpp b/sources/shiboken6/ApiExtractor/abstractmetalang.cpp
index 7cc036cbc..fb49cc9d0 100644
--- a/sources/shiboken6/ApiExtractor/abstractmetalang.cpp
+++ b/sources/shiboken6/ApiExtractor/abstractmetalang.cpp
@@ -102,6 +102,7 @@ public:
 
     AbstractMetaClassCPtr m_templateBaseClass;
     AbstractMetaFunctionCList m_functions;
+    AbstractMetaFunctionCList m_userAddedPythonOverrides;
     AbstractMetaFieldList m_fields;
     AbstractMetaEnumList m_enums;
     QList<QPropertySpec> m_propertySpecs;
@@ -323,6 +324,11 @@ const AbstractMetaFunctionCList &AbstractMetaClass::functions() const
     return d->m_functions;
 }
 
+const AbstractMetaFunctionCList &AbstractMetaClass::userAddedPythonOverrides() const
+{
+    return d->m_userAddedPythonOverrides;
+}
+
 void AbstractMetaClassPrivate::sortFunctions()
 {
     std::sort(m_functions.begin(), m_functions.end(), function_sorter);
@@ -390,7 +396,13 @@ void AbstractMetaClass::addFunction(const AbstractMetaClassPtr &klass,
     // to function properly. Such as function modifications
     nonConstF->setImplementingClass(klass);
 
-    klass->d->addFunction(function);
+    if (function->isUserAddedPythonOverride()) {
+        nonConstF->setConstant(false);
+        nonConstF->setCppAttribute(FunctionAttribute::Static);
+        klass->d->m_userAddedPythonOverrides.append(function);
+    } else {
+        klass->d->addFunction(function);
+    }
 }
 
 bool AbstractMetaClass::hasSignal(const AbstractMetaFunction *other) const
@@ -1452,6 +1464,12 @@ void AbstractMetaClass::fixFunctions(const AbstractMetaClassPtr &klass)
     }
 
     for (const auto &superClassC : d->m_baseClasses) {
+        for (const auto &pof : superClassC->userAddedPythonOverrides()) {
+            auto *clonedPof = pof->copy();
+            clonedPof->setOwnerClass(klass);
+            d->m_userAddedPythonOverrides.append(AbstractMetaFunctionCPtr{clonedPof});
+        }
+
         auto superClass = std::const_pointer_cast<AbstractMetaClass>(superClassC);
         AbstractMetaClass::fixFunctions(superClass);
         // Since we always traverse the complete hierarchy we are only
diff --git a/sources/shiboken6/ApiExtractor/abstractmetalang.h b/sources/shiboken6/ApiExtractor/abstractmetalang.h
index f7ae7b69f..3dc876690 100644
--- a/sources/shiboken6/ApiExtractor/abstractmetalang.h
+++ b/sources/shiboken6/ApiExtractor/abstractmetalang.h
@@ -66,6 +66,7 @@ public:
     ~AbstractMetaClass();
 
     const AbstractMetaFunctionCList &functions() const;
+    const AbstractMetaFunctionCList &userAddedPythonOverrides() const;
     void setFunctions(const AbstractMetaFunctionCList &functions);
     static void addFunction(const AbstractMetaClassPtr &klass,
                             const AbstractMetaFunctionCPtr &function);
diff --git a/sources/shiboken6/ApiExtractor/addedfunction.h b/sources/shiboken6/ApiExtractor/addedfunction.h
index 06986a47b..b8d189b7a 100644
--- a/sources/shiboken6/ApiExtractor/addedfunction.h
+++ b/sources/shiboken6/ApiExtractor/addedfunction.h
@@ -79,6 +79,9 @@ struct AddedFunction
     bool isDeclaration() const { return m_isDeclaration; } // <declare-function>
     void setDeclaration(bool value) { m_isDeclaration = value; }
 
+    bool isPythonOverride() const { return m_isPythonOverride; }
+    void setPythonOverride(bool o) { m_isPythonOverride = o; }
+
     const FunctionModificationList &modifications() const { return m_modifications; }
     FunctionModificationList &modifications() { return m_modifications; }
 
@@ -101,6 +104,7 @@ private:
     bool m_isClassMethod = false;
     bool m_isStatic = false;
     bool m_isDeclaration = false;
+    bool m_isPythonOverride = false;
 };
 
 QDebug operator<<(QDebug d, const AddedFunction::Argument &a);
diff --git a/sources/shiboken6/ApiExtractor/apiextractor.cpp b/sources/shiboken6/ApiExtractor/apiextractor.cpp
index ea45f22ba..83ee4437e 100644
--- a/sources/shiboken6/ApiExtractor/apiextractor.cpp
+++ b/sources/shiboken6/ApiExtractor/apiextractor.cpp
@@ -669,6 +669,8 @@ ApiExtractorPrivate::collectInstantiatedContainersAndSmartPointers(Instantiation
         return;
     for (const auto &func : metaClass->functions())
         collectInstantiatedContainersAndSmartPointers(context, func);
+    for (const auto &func : metaClass->userAddedPythonOverrides())
+        collectInstantiatedContainersAndSmartPointers(context, func);
     for (const AbstractMetaField &field : metaClass->fields())
         addInstantiatedContainersAndSmartPointers(context, field.type(), field.name());
 
diff --git a/sources/shiboken6/ApiExtractor/typesystemparser.cpp b/sources/shiboken6/ApiExtractor/typesystemparser.cpp
index bcae02ff4..2b686e997 100644
--- a/sources/shiboken6/ApiExtractor/typesystemparser.cpp
+++ b/sources/shiboken6/ApiExtractor/typesystemparser.cpp
@@ -93,6 +93,7 @@ constexpr auto positionAttribute = "position"_L1;
 constexpr auto preferredConversionAttribute = "preferred-conversion"_L1;
 constexpr auto preferredTargetLangTypeAttribute = "preferred-target-lang-type"_L1;
 constexpr auto pythonEnumTypeAttribute = "python-type"_L1;
+constexpr auto pythonOverrideAttribute = "python-override"_L1;
 constexpr auto cppEnumTypeAttribute = "cpp-type"_L1;
 constexpr auto qtMetaObjectFunctionsAttribute = "qt-metaobject"_L1;
 constexpr auto qtMetaTypeAttribute = "qt-register-metatype"_L1;
@@ -2601,6 +2602,7 @@ bool TypeSystemParser::parseAddFunction(const ConditionalStreamReader &,
     QString returnType;
     bool staticFunction = false;
     bool classMethod = false;
+    bool pythonOverride = false;
     QString access;
     for (auto i = attributes->size() - 1; i >= 0; --i) {
         const auto name = attributes->at(i).qualifiedName();
@@ -2616,6 +2618,9 @@ bool TypeSystemParser::parseAddFunction(const ConditionalStreamReader &,
                                             classmethodAttribute, false);
         } else if (name == accessAttribute) {
             access = attributes->takeAt(i).value().toString();
+        } else if (name == pythonOverrideAttribute) {
+            pythonOverride = convertBoolean(attributes->takeAt(i).value(),
+                                            pythonOverrideAttribute, false);
         }
     }
 
@@ -2639,6 +2644,7 @@ bool TypeSystemParser::parseAddFunction(const ConditionalStreamReader &,
 
     func->setStatic(staticFunction);
     func->setClassMethod(classMethod);
+    func->setPythonOverride(pythonOverride);
     func->setTargetLangPackage(m_defaultPackage);
 
     // Create signature for matching modifications
diff --git a/sources/shiboken6/doc/typesystem_manipulating_objects.rst b/sources/shiboken6/doc/typesystem_manipulating_objects.rst
index 89e3879b4..e024cdf00 100644
--- a/sources/shiboken6/doc/typesystem_manipulating_objects.rst
+++ b/sources/shiboken6/doc/typesystem_manipulating_objects.rst
@@ -282,6 +282,7 @@ logic. This can be done using the :ref:`inject-code` node.
                        access="public | protected"
                        overload-number="number"
                        static="yes | no" classmethod="yes | no"
+                       python-override ="yes | no"
                        since="..."/>
      </object-type>
 
@@ -320,6 +321,10 @@ within the `signature` field
 
 See :ref:`sequence-protocol` for adding the respective functions.
 
+The *optional* attribute ``python-override`` indicates a special type
+of added function, a python-override that will be generated into
+the native wrapper (see :ref:`modifying-virtual-functions`).
+
 .. _declare-function:
 
 declare-function
@@ -500,3 +505,52 @@ configuration (see also option :ref:`drop-type-entries`) intended
 for building several configurations from one generated source tree,
 but still requires listing the correct source files in the
 ``CMakeLists.txt`` file.
+
+.. _modifying-virtual-functions:
+
+Modifying virtual functions
+^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Some C++ virtual functions are unsuitable for Python bindings:
+
+.. code-block:: c
+
+    virtual void getInt(int *result) const;
+
+In that case, you would modify it to return the integer instead (or a tuple
+in case of several out-parameters):
+
+.. code-block:: c
+
+    virtual int getInt() const;
+
+For the binding itself, use the common argument modifications (removing
+arguments, modifying return types with injected code snippets) to modify the
+signature.
+
+To make it possible to reimplement the function in Python with the modified
+signature, add a ``python-override`` function with that signature, using an
+arbitrary name for disambiguation:
+
+.. code-block:: xml
+
+       <add-function signature="getIntPyOverride()"
+                     return-type="int" python-override="true"/>
+
+This causes a static function performing the call into Python for the override
+to be generated into the native wrapper.
+
+In the existing virtual function, inject a code snippet at the ``shell`` /
+``override`` position which calls the newly added function. The first 2
+arguments are the `Global interpreter lock handle` (``Shiboken::GilState``) and
+the Python method determined by the override check (``PyObject *``). The
+snippet then converts the arguments and return values and returns after that:
+
+.. code-block:: xml
+
+       <modify-function signature="getInt(int*)const">
+           <inject-code class="shell" position="override">
+           *result = getIntPyOverride(gil, pyOverride.object());
+           return;
+           </inject-code>
+       </modify-function>
diff --git a/sources/shiboken6/generator/shiboken/cppgenerator.cpp b/sources/shiboken6/generator/shiboken/cppgenerator.cpp
index 4ac6492e9..059ab7be3 100644
--- a/sources/shiboken6/generator/shiboken/cppgenerator.cpp
+++ b/sources/shiboken6/generator/shiboken/cppgenerator.cpp
@@ -624,6 +624,9 @@ void CppGenerator::generateClass(TextStream &s, const GeneratorContext &classCon
             writeDestructorNative(s, classContext);
     }
 
+    for (const auto &f : metaClass->userAddedPythonOverrides())
+        writeUserAddedPythonOverride(s, f);
+
     StringStream smd(TextStream::Language::Cpp);
     StringStream md(TextStream::Language::Cpp);
     StringStream signatureStream(TextStream::Language::Cpp);
@@ -1129,6 +1132,29 @@ static inline void writeVirtualMethodStaticReturnVar(TextStream &s, const Abstra
         << virtualMethodStaticReturnVar << ";\n";
 }
 
+static void writeFuncNameVar(TextStream &s, const AbstractMetaFunctionCPtr &func,
+                             const QString &funcName)
+{
+    // PYSIDE-1019: Add info about properties
+    int propFlag = 0;
+    if (func->isPropertyReader())
+        propFlag |= 1;
+    if (func->isPropertyWriter())
+        propFlag |= 2;
+    if (propFlag && func->isStatic())
+        propFlag |= 4;
+    QString propStr;
+    if (propFlag != 90)
+        propStr = QString::number(propFlag) + u':';
+
+    if (propFlag != 0)
+        s << "// This method belongs to a property.\n";
+    s << "static const char *funcName = \"";
+    if (propFlag != 0)
+        s << propFlag << ':';
+    s << funcName << "\";\n";
+}
+
 void CppGenerator::writeVirtualMethodNative(TextStream &s,
                                             const AbstractMetaFunctionCPtr &func,
                                             int cacheIndex) const
@@ -1191,23 +1217,9 @@ void CppGenerator::writeVirtualMethodNative(TextStream &s,
     s << "if (" << shibokenErrorsOccurred << ")\n" << indent
         << returnStatement.statement << '\n' << outdent;
 
-    // PYSIDE-1019: Add info about properties
-    int propFlag = 0;
-    if (func->isPropertyReader())
-        propFlag |= 1;
-    if (func->isPropertyWriter())
-        propFlag |= 2;
-    if (propFlag && func->isStatic())
-        propFlag |= 4;
-    QString propStr;
-    if (propFlag)
-        propStr = QString::number(propFlag) + u':';
-
     s << "static PyObject *nameCache[2] = {};\n";
-    if (propFlag)
-        s << "// This method belongs to a property.\n";
-    s << "static const char *funcName = \"" << propStr << funcName << "\";\n"
-        << "Shiboken::AutoDecRef " << PYTHON_OVERRIDE_VAR
+    writeFuncNameVar(s, func, funcName);
+    s << "Shiboken::AutoDecRef " << PYTHON_OVERRIDE_VAR
         << "(Shiboken::BindingManager::instance().getOverride(this, nameCache, funcName));\n"
         << "if (" << PYTHON_OVERRIDE_VAR << ".isNull()) {\n" << indent;
     if (useOverrideCaching(func->ownerClass()))
@@ -1430,6 +1442,28 @@ void CppGenerator::writeVirtualMethodPythonOverride(TextStream &s,
     s << outdent << "}\n\n";
 }
 
+void CppGenerator::writeUserAddedPythonOverride(TextStream &s,
+                                                const AbstractMetaFunctionCPtr &func) const
+{
+    TypeEntryCPtr retType = func->type().typeEntry();
+    const QString funcName = func->isOperatorOverload()
+        ? pythonOperatorFunctionName(func) : func->definitionNames().constFirst();
+
+    const CodeSnipList snips = func->hasInjectedCode()
+        ? func->injectedCodeSnips() : CodeSnipList();
+
+    QString prefix = wrapperName(func->ownerClass()) + u"::"_s;
+    s << '\n' << functionSignature(func, prefix, QString(), Generator::SkipDefaultValues |
+                                   Generator::OriginalTypeDescription)
+      << "\n{\n" << indent << sbkUnusedVariableCast("gil");
+
+    writeFuncNameVar(s, func, funcName);
+
+    const auto returnStatement = virtualMethodReturn(api(), func,
+                                                     func->modifications());
+    writeVirtualMethodPythonOverride(s, func, snips, returnStatement);
+}
+
 void CppGenerator::writeMetaObjectMethod(TextStream &s,
                                          const GeneratorContext &classContext) const
 {
diff --git a/sources/shiboken6/generator/shiboken/cppgenerator.h b/sources/shiboken6/generator/shiboken/cppgenerator.h
index 7e87fd9f3..00ae31f9a 100644
--- a/sources/shiboken6/generator/shiboken/cppgenerator.h
+++ b/sources/shiboken6/generator/shiboken/cppgenerator.h
@@ -87,6 +87,8 @@ private:
                                           const AbstractMetaFunctionCPtr &func,
                                           const CodeSnipList &snips,
                                           const VirtualMethodReturn &returnStatement) const;
+    void writeUserAddedPythonOverride(TextStream &s,
+                                      const AbstractMetaFunctionCPtr &func) const;
     void writeVirtualMethodCppCall(TextStream &s, const AbstractMetaFunctionCPtr &func,
                                    const QString &funcName, const QList<CodeSnip> &snips,
                                    const AbstractMetaArgument *lastArg, const TypeEntryCPtr &retType,
diff --git a/sources/shiboken6/generator/shiboken/headergenerator.cpp b/sources/shiboken6/generator/shiboken/headergenerator.cpp
index c0aa2e129..1f574b47c 100644
--- a/sources/shiboken6/generator/shiboken/headergenerator.cpp
+++ b/sources/shiboken6/generator/shiboken/headergenerator.cpp
@@ -274,8 +274,15 @@ void *qt_metacast(const char *_clname) override;
         s << "static void pysideInitQtMetaTypes();\n";
 
     s << "void resetPyMethodCache();\n"
-      << outdent << "private:\n" << indent
-      << "mutable bool m_PyMethodCache[" << maxOverrides << "];\n"
+        << outdent << "private:\n" << indent;
+
+    if (!metaClass->userAddedPythonOverrides().isEmpty()) {
+        for (const auto &f : metaClass->userAddedPythonOverrides())
+            s << functionSignature(f, {}, {}, Generator::OriginalTypeDescription) << ";\n";
+        s << '\n';
+    }
+
+    s << "mutable bool m_PyMethodCache[" << maxOverrides << "];\n"
       << outdent << "};\n\n";
 }
 
@@ -286,8 +293,6 @@ void HeaderGenerator::writeMemberFunctionWrapper(TextStream &s,
 {
     Q_ASSERT(!func->isConstructor() && !func->isOperatorOverload());
     s << "inline ";
-    if (func->isStatic())
-        s << "static ";
     s << functionSignature(func, {}, postfix, Generator::OriginalTypeDescription)
       << " { ";
     if (!func->isVoid())
diff --git a/sources/shiboken6/generator/shiboken/shibokengenerator.cpp b/sources/shiboken6/generator/shiboken/shibokengenerator.cpp
index ac098f394..a1417e5d9 100644
--- a/sources/shiboken6/generator/shiboken/shibokengenerator.cpp
+++ b/sources/shiboken6/generator/shiboken/shibokengenerator.cpp
@@ -1147,6 +1147,10 @@ void ShibokenGenerator::writeFunctionArguments(TextStream &s,
                                                Options options) const
 {
     int argUsed = 0;
+    if (func->isUserAddedPythonOverride()) {
+        s << "Shiboken::GilState &gil, PyObject *" << PYTHON_OVERRIDE_VAR;
+        argUsed += 2;
+    }
     for (const auto &arg : func->arguments()) {
         if (options.testFlag(Generator::SkipRemovedArguments) && arg.isModifiedRemoved())
             continue;
@@ -1183,6 +1187,8 @@ QString ShibokenGenerator::functionSignature(const AbstractMetaFunctionCPtr &fun
 {
     StringStream s(TextStream::Language::Cpp);
     // The actual function
+    if (!options.testFlag(Option::SkipDefaultValues) && func->isStatic()) // Declaration
+        s << "static ";
     if (func->isEmptyFunction() || func->needsReturnType())
         s << functionReturnType(func, options) << ' ';
     else
diff --git a/sources/shiboken6/tests/libsample/abstract.cpp b/sources/shiboken6/tests/libsample/abstract.cpp
index 648cedcd0..0d67d8630 100644
--- a/sources/shiboken6/tests/libsample/abstract.cpp
+++ b/sources/shiboken6/tests/libsample/abstract.cpp
@@ -49,6 +49,18 @@ void Abstract::show(PrintFormat format) const
     std::cout << '>';
 }
 
+void Abstract::virtualWithOutParameter(int &x) const
+{
+    x = 42;
+}
+
+int Abstract::callVirtualWithOutParameter() const
+{
+    int x;
+    virtualWithOutParameter(x);
+    return x;
+}
+
 void Abstract::callVirtualGettingEnum(PrintFormat p)
 {
     virtualGettingAEnum(p);
diff --git a/sources/shiboken6/tests/libsample/abstract.h b/sources/shiboken6/tests/libsample/abstract.h
index 1e62a9c51..4c1b98d90 100644
--- a/sources/shiboken6/tests/libsample/abstract.h
+++ b/sources/shiboken6/tests/libsample/abstract.h
@@ -74,6 +74,9 @@ public:
 
     virtual void hideFunction(HideType *arg) = 0;
 
+    virtual void virtualWithOutParameter(int &x) const;
+    int callVirtualWithOutParameter() const;
+
 protected:
     virtual const char *className() const { return "Abstract"; }
 
diff --git a/sources/shiboken6/tests/samplebinding/derived_test.py b/sources/shiboken6/tests/samplebinding/derived_test.py
index 418c990d3..346f29136 100644
--- a/sources/shiboken6/tests/samplebinding/derived_test.py
+++ b/sources/shiboken6/tests/samplebinding/derived_test.py
@@ -33,6 +33,15 @@ class Deviant(Derived):
         return 'Deviant'
 
 
+class ImplementVirtualWithOutParameter(Derived):
+    def __init__(self, value):
+        super().__init__()
+        self._value = value
+
+    def virtualWithOutParameter(self):
+        return self._value
+
+
 class DerivedTest(unittest.TestCase):
     '''Test case for Derived class'''
 
@@ -122,6 +131,13 @@ class DerivedTest(unittest.TestCase):
         obj = DerivedUsingCt(42)
         self.assertEqual(obj.value(), 42)
 
+    def testVirtualWithOutParameter(self):
+        d = Derived()
+        self.assertEqual(d.callVirtualWithOutParameter(), 42)
+
+        d = ImplementVirtualWithOutParameter(1)
+        self.assertEqual(d.callVirtualWithOutParameter(), 1)
+
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/sources/shiboken6/tests/samplebinding/typesystem_sample.xml b/sources/shiboken6/tests/samplebinding/typesystem_sample.xml
index 36134e649..e315e599e 100644
--- a/sources/shiboken6/tests/samplebinding/typesystem_sample.xml
+++ b/sources/shiboken6/tests/samplebinding/typesystem_sample.xml
@@ -571,6 +571,14 @@
         <modify-function signature="hideFunction(HideType*)" remove="all"/>
         <modify-field name="toBeRenamedField" rename="renamedField"/>
         <modify-field name="readOnlyField" write="false"/>
+        <modify-function signature="virtualWithOutParameter(int&amp;)const">
+            <inject-code class="shell" position="override">
+            x = virtualWithOutParameterPyOverride(gil, pyOverride.object());
+            return;
+            </inject-code>
+        </modify-function>
+        <add-function signature="virtualWithOutParameterPyOverride()"
+                      return-type="int" python-override="true"/>
     </object-type>
 
     <object-type name="Derived" polymorphic-id-expression="%1->type() == Derived::TpDerived">