shiboken: Refactor Generator::minimalConstructor(AbstractMetaClass)

Add some convenience functions returning the status of
default expressions to AbstractMetaArgument.

Rewrite the function to only insert suitable candidates into
an ordered multimap instead of looping twice over all candidates.

This unearthed a bug in the old algorithm trying to find the maximum
number of arguments: When no candidates were found, maxArgs was left
at 0, which caused it to assume default constructible. This triggered
for the QMatrixNxN classes inheriting QGenericMatrix<int, int, Type>
with (unsupported) non-type template parameters. For these, the
default constructor needs to be specified now.

Task-number: PYSIDE-62
Change-Id: I5ce2bed43001780553048d8af0addaba2b22410b
Reviewed-by: Christian Tismer <tismer@stackless.com>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>

4 files changed, 52 insertions, 91 deletions

diff --git a/sources/pyside2/PySide2/QtGui/typesystem_gui_common.xml b/sources/pyside2/PySide2/QtGui/typesystem_gui_common.xml
index 665750946..24ee2985f 100644
--- a/sources/pyside2/PySide2/QtGui/typesystem_gui_common.xml
+++ b/sources/pyside2/PySide2/QtGui/typesystem_gui_common.xml
@@ -2380,7 +2380,10 @@
     </modify-function>
   </object-type>
 
-  <value-type name="QMatrix2x2" since="4.6">
+  <!-- The matrix classes need a default constructor specification since
+       they inherit template class QGenericMatrix<int, int, Type> with (unsupported)
+       non-type template parameters -->
+  <value-type name="QMatrix2x2" since="4.6" default-constructor="QMatrix2x2()">
     <add-function signature="__repr__" return-type="PyObject*">
         <inject-code class="target" position="beginning">
             <insert-template name="repr_code_matrix">
@@ -2468,7 +2471,7 @@
     <add-function signature="operator==(const QMatrix2x2&amp;)" return-type="bool" />
   </value-type>
 
-  <value-type name="QMatrix2x3" since="4.6">
+  <value-type name="QMatrix2x3" since="4.6" default-constructor="QMatrix2x3()">
     <add-function signature="__repr__" return-type="PyObject*">
         <inject-code class="target" position="beginning">
             <insert-template name="repr_code_matrix">
@@ -2538,7 +2541,7 @@
     <add-function signature="operator==(const QMatrix2x3&amp;)" return-type="bool" />
   </value-type>
 
-  <value-type name="QMatrix2x4" since="4.6">
+  <value-type name="QMatrix2x4" since="4.6" default-constructor="QMatrix2x4()">
     <add-function signature="__repr__" return-type="PyObject*">
         <inject-code class="target" position="beginning">
             <insert-template name="repr_code_matrix">
@@ -2608,7 +2611,7 @@
     <add-function signature="operator==(const QMatrix2x4&amp;)" return-type="bool" />
   </value-type>
 
-  <value-type name="QMatrix3x2" since="4.6">
+  <value-type name="QMatrix3x2" since="4.6" default-constructor="QMatrix3x2()">
     <add-function signature="__repr__" return-type="PyObject*">
         <inject-code class="target" position="beginning">
             <insert-template name="repr_code_matrix">
@@ -2678,7 +2681,7 @@
     <add-function signature="operator==(const QMatrix3x2&amp;)" return-type="bool" />
   </value-type>
 
-  <value-type name="QMatrix3x3" since="4.6">
+  <value-type name="QMatrix3x3" since="4.6" default-constructor="QMatrix3x3()">
     <add-function signature="__repr__" return-type="PyObject*">
         <inject-code class="target" position="beginning">
             <insert-template name="repr_code_matrix">
@@ -2748,7 +2751,7 @@
     <add-function signature="operator==(const QMatrix3x3&amp;)" return-type="bool" />
   </value-type>
 
-  <value-type name="QMatrix3x4" since="4.6">
+  <value-type name="QMatrix3x4" since="4.6" default-constructor="QMatrix3x4()">
     <add-function signature="__repr__" return-type="PyObject*">
         <inject-code class="target" position="beginning">
             <insert-template name="repr_code_matrix">
@@ -2818,7 +2821,7 @@
     <add-function signature="operator==(const QMatrix3x4&amp;)" return-type="bool" />
   </value-type>
 
-  <value-type name="QMatrix4x2" since="4.6">
+  <value-type name="QMatrix4x2" since="4.6" default-constructor="QMatrix4x2()">
     <add-function signature="__repr__" return-type="PyObject*">
         <inject-code class="target" position="beginning">
             <insert-template name="repr_code_matrix">
@@ -2888,7 +2891,7 @@
     <add-function signature="operator==(const QMatrix4x2&amp;)" return-type="bool" />
   </value-type>
 
-  <value-type name="QMatrix4x3" since="4.6">
+  <value-type name="QMatrix4x3" since="4.6" default-constructor="QMatrix4x3()">
     <add-function signature="__repr__" return-type="PyObject*">
         <inject-code class="target" position="beginning">
             <insert-template name="repr_code_matrix">
@@ -2958,7 +2961,7 @@
     <add-function signature="operator==(const QMatrix4x3&amp;)" return-type="bool" />
   </value-type>
 
-  <value-type name="QMatrix4x4" since="4.6">
+  <value-type name="QMatrix4x4" since="4.6" default-constructor="QMatrix4x4()">
 
 
     <!-- Qt5: HAIRY TRICK ALERT ahead!
diff --git a/sources/shiboken2/ApiExtractor/abstractmetalang.h b/sources/shiboken2/ApiExtractor/abstractmetalang.h
index f583d2c98..08ab49d91 100644
--- a/sources/shiboken2/ApiExtractor/abstractmetalang.h
+++ b/sources/shiboken2/ApiExtractor/abstractmetalang.h
@@ -668,6 +668,13 @@ public:
         m_originalExpression = expr;
     }
 
+    bool hasDefaultValueExpression() const
+    { return !m_originalExpression.isEmpty() || !m_expression.isEmpty(); }
+    bool hasUnmodifiedDefaultValueExpression() const
+    { return !m_originalExpression.isEmpty() && m_originalExpression == m_expression; }
+    bool hasModifiedDefaultValueExpression() const
+    { return !m_expression.isEmpty() && m_originalExpression != m_expression; }
+
     QString toString() const
     {
         return type()->name() + QLatin1Char(' ') + AbstractMetaVariable::name() +
diff --git a/sources/shiboken2/generator/generator.cpp b/sources/shiboken2/generator/generator.cpp
index cf850356c..7ae5b3fd1 100644
--- a/sources/shiboken2/generator/generator.cpp
+++ b/sources/shiboken2/generator/generator.cpp
@@ -760,99 +760,50 @@ DefaultValue Generator::minimalConstructor(const AbstractMetaClass* metaClass) c
     if (cType->hasDefaultConstructor())
         return DefaultValue(DefaultValue::Custom, cType->defaultConstructor());
 
+    const QString qualifiedCppName = cType->qualifiedCppName();
+    // Obtain a list of constructors sorted by complexity and number of arguments
+    QMultiMap<int, const AbstractMetaFunction *> candidates;
     const AbstractMetaFunctionList &constructors = metaClass->queryFunctions(AbstractMetaClass::Constructors);
-    int maxArgs = 0;
     for (const AbstractMetaFunction *ctor : constructors) {
-        if (ctor->isUserAdded() || ctor->isPrivate() || ctor->functionType() != AbstractMetaFunction::ConstructorFunction)
-            continue;
-
-        int numArgs = ctor->arguments().size();
-        if (numArgs == 0) {
-            maxArgs = 0;
-            break;
-        }
-        if (numArgs > maxArgs)
-            maxArgs = numArgs;
-    }
-
-    QString qualifiedCppName = metaClass->typeEntry()->qualifiedCppName();
-    QStringList templateTypes;
-    const QVector<TypeEntry *> &templateArguments = metaClass->templateArguments();
-    for (TypeEntry *templateType : templateArguments)
-        templateTypes << templateType->qualifiedCppName();
-
-    // Empty constructor.
-    if (maxArgs == 0)
-        return DefaultValue(DefaultValue::DefaultConstructor, QLatin1String("::") + qualifiedCppName);
-
-    QVector<const AbstractMetaFunction *> candidates;
-
-    // Constructors with C++ primitive types, enums or pointers only.
-    // Start with the ones with fewer arguments.
-    for (int i = 1; i <= maxArgs; ++i) {
-        for (const AbstractMetaFunction *ctor : constructors) {
-            if (ctor->isUserAdded() || ctor->isPrivate() || ctor->functionType() != AbstractMetaFunction::ConstructorFunction)
-                continue;
-
-            const AbstractMetaArgumentList &arguments = ctor->arguments();
-            if (arguments.size() != i)
-                continue;
-
-            QStringList args;
-            for (const AbstractMetaArgument *arg : arguments) {
-                const TypeEntry* type = arg->type()->typeEntry();
-                if (type == metaClass->typeEntry()) {
-                    args.clear();
-                    break;
-                }
-
-                if (!arg->originalDefaultValueExpression().isEmpty()) {
-                    if (!arg->defaultValueExpression().isEmpty()
-                        && arg->defaultValueExpression() != arg->originalDefaultValueExpression()) {
-                        args << arg->defaultValueExpression();
-                    }
-                    break;
-                }
-
-                if (type->isCppPrimitive() || type->isEnum() || isPointer(arg->type())) {
-                    auto argValue = minimalConstructor(arg->type());
-                    if (!argValue.isValid()) {
-                        args.clear();
-                        break;
-                    }
-                    args << argValue.constructorParameter();
-                } else {
-                    args.clear();
-                    break;
-                }
+        if (!ctor->isUserAdded() && !ctor->isPrivate()
+            && ctor->functionType() == AbstractMetaFunction::ConstructorFunction) {
+            // No arguments: Default constructible
+            const auto &arguments = ctor->arguments();
+            if (arguments.isEmpty()) {
+                return DefaultValue(DefaultValue::DefaultConstructor,
+                                    QLatin1String("::") + qualifiedCppName);
             }
-
-            if (!args.isEmpty())
-                return DefaultValue(DefaultValue::Custom, constructorCall(qualifiedCppName, args));
-
-            candidates << ctor;
+            // Examine arguments, exclude functions taking a self parameter
+            bool simple = true;
+            bool suitable = true;
+            for (int i = 0, size = arguments.size();
+                 suitable && i < size && !arguments.at(i)->hasDefaultValueExpression(); ++i) {
+                const AbstractMetaArgument *arg = arguments.at(i);
+                const TypeEntry *aType = arg->type()->typeEntry();
+                suitable &= aType != cType;
+                simple &= aType->isCppPrimitive() || aType->isEnum() || isPointer(arg->type());
+            }
+            if (suitable)
+                candidates.insert(arguments.size() + (simple ? 0 : 100), ctor);
         }
     }
 
-    // Constructors with C++ primitive types, enums, pointers, value types,
-    // and user defined primitive types.
-    // Builds the minimal constructor recursively.
-    for (const AbstractMetaFunction *ctor : qAsConst(candidates)) {
+    for (auto it = candidates.cbegin(), end = candidates.cend(); it != end; ++it) {
+        const AbstractMetaArgumentList &arguments = it.value()->arguments();
         QStringList args;
-        const AbstractMetaArgumentList &arguments = ctor->arguments();
-        for (const AbstractMetaArgument *arg : arguments) {
-            if (arg->type()->typeEntry() == metaClass->typeEntry()) {
-                args.clear();
+        bool ok = true;
+        for (int i =0, size = arguments.size(); ok && i < size; ++i) {
+            const AbstractMetaArgument *arg = arguments.at(i);
+            if (arg->hasDefaultValueExpression()) {
+                if (arg->hasModifiedDefaultValueExpression())
+                    args << arg->defaultValueExpression(); // Spell out modified values
                 break;
             }
             auto argValue = minimalConstructor(arg->type());
-            if (!argValue.isValid()) {
-                args.clear();
-                break;
-            }
+            ok &= argValue.isValid();
             args << argValue.constructorParameter();
         }
-        if (!args.isEmpty())
+        if (ok)
             return DefaultValue(DefaultValue::Custom, constructorCall(qualifiedCppName, args));
     }
 
diff --git a/sources/shiboken2/generator/shiboken2/cppgenerator.cpp b/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
index 4bb1e02b6..dfe23a598 100644
--- a/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
+++ b/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
@@ -3173,7 +3173,7 @@ void CppGenerator::writeMethodCall(QTextStream &s, const AbstractMetaFunction *f
             bool argsClear = true;
             for (int i = func->arguments().size() - 1; i >= maxArgs + removedArgs; i--) {
                 const AbstractMetaArgument* arg = func->arguments().at(i);
-                bool defValModified = arg->defaultValueExpression() != arg->originalDefaultValueExpression();
+                const bool defValModified = arg->hasModifiedDefaultValueExpression();
                 bool hasConversionRule = !func->conversionRule(TypeSystem::NativeCode, arg->argumentIndex() + 1).isEmpty();
                 if (argsClear && !defValModified && !hasConversionRule)
                     continue;