shiboken: Make constructor checks more fine-grained

ShibokenGenerator::isValueTypeWithCopyConstructorOnly()
returned false for QWebEngineHistoryItem since the check
for AbstractMetaAttributes::HasRejectedConstructor triggered
on WebEngineHistoryItem(QWebEngineHistoryItemPrivate *priv),
causing the function to bail out.

To prevent this, add a new
AbstractMetaAttributes::HasRejectedDefaultConstructor attribute
and use that in isValueTypeWithCopyConstructorOnly().

Task-number: PYSIDE-906
Change-Id: I4fee83b89f0a4c44e7e8d69e118ed7f2b03ceee1
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>

6 files changed, 78 insertions, 31 deletions

diff --git a/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp b/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp
index f6724e61d..7498d3a18 100644
--- a/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp
+++ b/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp
@@ -1367,17 +1367,21 @@ static bool _compareAbstractMetaFunctions(const AbstractMetaFunction* func, cons
 }
 
 AbstractMetaFunctionList AbstractMetaBuilderPrivate::classFunctionList(const ScopeModelItem &scopeItem,
-                                                                       bool *constructorRejected)
+                                                                       AbstractMetaClass::Attributes *constructorAttributes)
 {
-    *constructorRejected = false;
+    *constructorAttributes = 0;
     AbstractMetaFunctionList result;
     const FunctionList &scopeFunctionList = scopeItem->functions();
     result.reserve(scopeFunctionList.size());
     for (const FunctionModelItem &function : scopeFunctionList) {
-        if (AbstractMetaFunction *metaFunction = traverseFunction(function))
+        if (AbstractMetaFunction *metaFunction = traverseFunction(function)) {
             result.append(metaFunction);
-        else if (function->functionType() == CodeModel::Constructor)
-            *constructorRejected = true;
+        } else if (function->functionType() == CodeModel::Constructor) {
+            auto arguments = function->arguments();
+            *constructorAttributes |= AbstractMetaAttributes::HasRejectedConstructor;
+            if (arguments.isEmpty() || arguments.constFirst()->defaultValue())
+                *constructorAttributes |= AbstractMetaAttributes::HasRejectedDefaultConstructor;
+        }
     }
     return result;
 }
@@ -1408,12 +1412,10 @@ private:
 void AbstractMetaBuilderPrivate::traverseFunctions(ScopeModelItem scopeItem,
                                                    AbstractMetaClass *metaClass)
 {
-    bool constructorRejected = false;
+    AbstractMetaAttributes::Attributes constructorAttributes;
     const AbstractMetaFunctionList functions =
-        classFunctionList(scopeItem, &constructorRejected);
-
-    if (constructorRejected)
-        *metaClass += AbstractMetaAttributes::HasRejectedConstructor;
+        classFunctionList(scopeItem, &constructorAttributes);
+    metaClass->setAttributes(metaClass->attributes() | constructorAttributes);
 
     for (AbstractMetaFunction *metaFunction : functions){
         metaFunction->setOriginalAttributes(metaFunction->attributes());
diff --git a/sources/shiboken2/ApiExtractor/abstractmetabuilder_p.h b/sources/shiboken2/ApiExtractor/abstractmetabuilder_p.h
index ec55d1b47..1633bd868 100644
--- a/sources/shiboken2/ApiExtractor/abstractmetabuilder_p.h
+++ b/sources/shiboken2/ApiExtractor/abstractmetabuilder_p.h
@@ -80,7 +80,7 @@ public:
     void traverseEnums(const ScopeModelItem &item, AbstractMetaClass *parent,
                        const QStringList &enumsDeclarations);
     AbstractMetaFunctionList classFunctionList(const ScopeModelItem &scopeItem,
-                                               bool *constructorRejected);
+                                               AbstractMetaClass::Attributes *constructorAttributes);
     AbstractMetaFunctionList templateClassFunctionList(const ScopeModelItem &scopeItem,
                                                        AbstractMetaClass *metaClass,
                                                        bool *constructorRejected);
diff --git a/sources/shiboken2/ApiExtractor/abstractmetalang.h b/sources/shiboken2/ApiExtractor/abstractmetalang.h
index aaefa32d5..bb17ad8d7 100644
--- a/sources/shiboken2/ApiExtractor/abstractmetalang.h
+++ b/sources/shiboken2/ApiExtractor/abstractmetalang.h
@@ -133,13 +133,14 @@ public:
         Invokable                   = 0x00040000,
 
         HasRejectedConstructor      = 0x00080000,
+        HasRejectedDefaultConstructor = 0x00100000,
 
-        FinalCppClass               = 0x00100000,
-        VirtualCppMethod            = 0x00200000,
-        OverriddenCppMethod         = 0x00400000,
-        FinalCppMethod              = 0x00800000,
+        FinalCppClass               = 0x00200000,
+        VirtualCppMethod            = 0x00400000,
+        OverriddenCppMethod         = 0x00800000,
+        FinalCppMethod              = 0x01000000,
         // Add by meta builder (implicit constructors, inherited methods, etc)
-        AddedMethod                 = 0x01000000
+        AddedMethod                 = 0x02000000
     };
     Q_DECLARE_FLAGS(Attributes, Attribute)
     Q_FLAG(Attribute)
diff --git a/sources/shiboken2/generator/shiboken2/shibokengenerator.cpp b/sources/shiboken2/generator/shiboken2/shibokengenerator.cpp
index ac0b2ffaf..a226f2c00 100644
--- a/sources/shiboken2/generator/shiboken2/shibokengenerator.cpp
+++ b/sources/shiboken2/generator/shiboken2/shibokengenerator.cpp
@@ -1055,12 +1055,26 @@ bool ShibokenGenerator::isValueTypeWithCopyConstructorOnly(const AbstractMetaCla
 {
     if (!metaClass || !metaClass->typeEntry()->isValue())
         return false;
-    if ((metaClass->attributes() & AbstractMetaAttributes::HasRejectedConstructor) != 0)
+    if (metaClass->attributes().testFlag(AbstractMetaAttributes::HasRejectedDefaultConstructor))
         return false;
-    AbstractMetaFunctionList ctors = metaClass->queryFunctions(AbstractMetaClass::Constructors);
-    if (ctors.count() != 1)
-        return false;
-    return ctors.constFirst()->functionType() == AbstractMetaFunction::CopyConstructorFunction;
+    const AbstractMetaFunctionList ctors =
+        metaClass->queryFunctions(AbstractMetaClass::Constructors);
+    bool copyConstructorFound = false;
+    for (auto ctor : ctors) {
+        switch (ctor->functionType()) {
+        case AbstractMetaFunction::ConstructorFunction:
+            return false;
+        case AbstractMetaFunction::CopyConstructorFunction:
+            copyConstructorFound = true;
+            break;
+        case AbstractMetaFunction::MoveConstructorFunction:
+            break;
+        default:
+            Q_ASSERT(false);
+            break;
+        }
+    }
+    return copyConstructorFound;
 }
 
 bool ShibokenGenerator::isValueTypeWithCopyConstructorOnly(const TypeEntry* type) const
diff --git a/sources/shiboken2/tests/libsample/onlycopy.cpp b/sources/shiboken2/tests/libsample/onlycopy.cpp
index 75bf23b5c..cfc7c9d99 100644
--- a/sources/shiboken2/tests/libsample/onlycopy.cpp
+++ b/sources/shiboken2/tests/libsample/onlycopy.cpp
@@ -28,18 +28,44 @@
 
 #include "onlycopy.h"
 
-OnlyCopy::OnlyCopy(const OnlyCopy& other)
+class OnlyCopyPrivate
+{
+public:
+    explicit OnlyCopyPrivate(int v = 0) : value(v) {}
+
+    int value;
+};
+
+OnlyCopy::OnlyCopy(int value) : d(new OnlyCopyPrivate(value))
+{
+
+}
+
+OnlyCopy::OnlyCopy(OnlyCopyPrivate *dIn) : d(dIn)
+{
+}
+
+OnlyCopy::~OnlyCopy()
+{
+    delete d;
+}
+
+OnlyCopy::OnlyCopy(const OnlyCopy& other) : d(new OnlyCopyPrivate(other.value()))
 {
-    m_value = other.m_value;
 }
 
 OnlyCopy&
 OnlyCopy::operator=(const OnlyCopy& other)
 {
-    m_value = other.m_value;
+    d->value = other.d->value;
     return *this;
 }
 
+int OnlyCopy::value() const
+{
+    return d->value;
+}
+
 OnlyCopy
 FriendOfOnlyCopy::createOnlyCopy(int value)
 {
diff --git a/sources/shiboken2/tests/libsample/onlycopy.h b/sources/shiboken2/tests/libsample/onlycopy.h
index 2c1b255fd..84a32a951 100644
--- a/sources/shiboken2/tests/libsample/onlycopy.h
+++ b/sources/shiboken2/tests/libsample/onlycopy.h
@@ -32,20 +32,24 @@
 #include "libsamplemacros.h"
 #include <list>
 
-// These classes simulate a situation found in
-// QtWebKit's QWebDatabase and QWebSecurityOrigin.
+// These classes simulate a situation found in QWebEngineHistoryItem.
+
+class OnlyCopyPrivate;
 
 class LIBSAMPLE_API OnlyCopy
 {
 public:
     OnlyCopy(const OnlyCopy& other);
     OnlyCopy& operator=(const OnlyCopy& other);
-    int value() const { return m_value; }
-    static int getValue(OnlyCopy onlyCopy) { return onlyCopy.m_value; }
-    static int getValueFromReference(const OnlyCopy& onlyCopy) { return onlyCopy.m_value; }
+    ~OnlyCopy();
+
+    int value() const;
+    static int getValue(OnlyCopy onlyCopy) { return onlyCopy.value(); }
+    static int getValueFromReference(const OnlyCopy& onlyCopy) { return onlyCopy.value(); }
 private:
-    int m_value;
-    OnlyCopy(int value) : m_value(value) {};
+    OnlyCopyPrivate *d;
+    explicit OnlyCopy(int value);
+    explicit OnlyCopy(OnlyCopyPrivate *d); // rejected due to unknown OnlyCopyPrivate
     friend class FriendOfOnlyCopy;
 };