From 8a465c1f6e156975d25938622e0299e64bf08295 Mon Sep 17 00:00:00 2001 From: Friedemann Kleint Date: Mon, 28 Jan 2019 10:36:39 +0100 Subject: 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 --- .../shiboken2/ApiExtractor/abstractmetabuilder.cpp | 22 ++++++++------- .../shiboken2/ApiExtractor/abstractmetabuilder_p.h | 2 +- sources/shiboken2/ApiExtractor/abstractmetalang.h | 11 ++++---- .../generator/shiboken2/shibokengenerator.cpp | 24 ++++++++++++---- sources/shiboken2/tests/libsample/onlycopy.cpp | 32 ++++++++++++++++++++-- sources/shiboken2/tests/libsample/onlycopy.h | 18 +++++++----- 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 -// 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; }; -- cgit v1.2.3