diff options
| author | Friedemann Kleint <Friedemann.Kleint@qt.io> | 2021-02-12 14:17:55 +0100 |
|---|---|---|
| committer | Friedemann Kleint <Friedemann.Kleint@qt.io> | 2021-02-17 15:16:15 +0100 |
| commit | 5fcbb17760d9ddf0a1e39c40baeaf28df9a4bb5e (patch) | |
| tree | 3a59f0fda60e5b0cd671a0fc163772e6ab78582a | |
| parent | 1a53685c10dc3dab972a42fe02166ff0349320f4 (diff) | |
shiboken6: Improve detection of default/copy constructability
Add support for deleted functions. Extend the checks for default
constructability by checks for fields and base classes. Refactor the
check for copy constructability to recursively check on base
classes. Remove functionality for adding private copy constructors
which was apparently unused.
Change-Id: I8105f277699d6121aa8aa193d9cb16bf8133e901
Reviewed-by: Cristian Maureira-Fredes <cristian.maureira-fredes@qt.io>
9 files changed, 158 insertions, 35 deletions
diff --git a/sources/shiboken6/ApiExtractor/abstractmetabuilder.cpp b/sources/shiboken6/ApiExtractor/abstractmetabuilder.cpp index 16fe8e01b..e8c82c247 100644 --- a/sources/shiboken6/ApiExtractor/abstractmetabuilder.cpp +++ b/sources/shiboken6/ApiExtractor/abstractmetabuilder.cpp @@ -499,16 +499,12 @@ void AbstractMetaBuilderPrivate::traverseDom(const FileModelItem &dom) for (AbstractMetaClass *cls : qAsConst(m_metaClasses)) { cls->fixFunctions(); - const bool couldAddDefaultCtors = cls->isConstructible() - && !cls->isNamespace() - && (cls->attributes() & AbstractMetaAttributes::HasRejectedConstructor) == 0; - if (couldAddDefaultCtors) { - if (!cls->hasConstructors()) - cls->addDefaultConstructor(); - if (cls->typeEntry()->isValue() && !cls->isAbstract() && !cls->hasCopyConstructor()) - cls->addDefaultCopyConstructor(ancestorHasPrivateCopyConstructor(cls)); - } + if (cls->canAddDefaultConstructor()) + cls->addDefaultConstructor(); + if (cls->canAddDefaultCopyConstructor()) + cls->addDefaultCopyConstructor(); } + const auto &allEntries = types->entries(); ReportHandler::startProgress("Detecting inconsistencies in typesystem (" @@ -1804,10 +1800,25 @@ static bool applyArrayArgumentModifications(const FunctionModificationList &func } AbstractMetaFunction *AbstractMetaBuilderPrivate::traverseFunction(const FunctionModelItem &functionItem, - const AbstractMetaClass *currentClass) + AbstractMetaClass *currentClass) { - if (functionItem->isDeleted() || !functionItem->templateParameters().isEmpty()) + if (!functionItem->templateParameters().isEmpty()) + return nullptr; + + if (functionItem->isDeleted()) { + switch (functionItem->functionType()) { + case CodeModel::Constructor: + if (functionItem->isDefaultConstructor()) + currentClass->setHasDeletedDefaultConstructor(true); + break; + case CodeModel::CopyConstructor: + currentClass->setHasDeletedCopyConstructor(true); + break; + default: + break; + } return nullptr; + } QString functionName = functionItem->name(); QString className; if (currentClass) { @@ -2622,18 +2633,6 @@ AbstractMetaClassList AbstractMetaBuilderPrivate::getBaseClasses(const AbstractM return baseClasses; } -bool AbstractMetaBuilderPrivate::ancestorHasPrivateCopyConstructor(const AbstractMetaClass *metaClass) const -{ - if (metaClass->hasPrivateCopyConstructor()) - return true; - const AbstractMetaClassList &baseClasses = getBaseClasses(metaClass); - for (const AbstractMetaClass *cls : baseClasses) { - if (ancestorHasPrivateCopyConstructor(cls)) - return true; - } - return false; -} - std::optional<AbstractMetaType> AbstractMetaBuilderPrivate::inheritTemplateType(const AbstractMetaTypeList &templateTypes, const AbstractMetaType &metaType) diff --git a/sources/shiboken6/ApiExtractor/abstractmetabuilder_p.h b/sources/shiboken6/ApiExtractor/abstractmetabuilder_p.h index 8191bc161..e7e6f8e10 100644 --- a/sources/shiboken6/ApiExtractor/abstractmetabuilder_p.h +++ b/sources/shiboken6/ApiExtractor/abstractmetabuilder_p.h @@ -118,7 +118,7 @@ public: AbstractMetaClass *metaClass, QString *errorMessage); AbstractMetaFunction *traverseFunction(const FunctionModelItem &function, - const AbstractMetaClass *currentClass); + AbstractMetaClass *currentClass); std::optional<AbstractMetaField> traverseField(const VariableModelItem &field, const AbstractMetaClass *cls); void checkFunctionModifications(); @@ -173,7 +173,6 @@ public: TypeInfo *info = Q_NULLPTR, ComplexTypeEntry **baseContainerType = Q_NULLPTR) const; AbstractMetaClassList getBaseClasses(const AbstractMetaClass *metaClass) const; - bool ancestorHasPrivateCopyConstructor(const AbstractMetaClass *metaClass) const; static bool inheritTemplate(AbstractMetaClass *subclass, const AbstractMetaClass *templateClass, diff --git a/sources/shiboken6/ApiExtractor/abstractmetafunction.cpp b/sources/shiboken6/ApiExtractor/abstractmetafunction.cpp index 5ccf36e00..93e73b6d9 100644 --- a/sources/shiboken6/ApiExtractor/abstractmetafunction.cpp +++ b/sources/shiboken6/ApiExtractor/abstractmetafunction.cpp @@ -552,6 +552,13 @@ bool AbstractMetaFunction::isConstructor() const || d->m_functionType == MoveConstructorFunction; } +bool AbstractMetaFunction::isDefaultConstructor() const +{ + return d->m_functionType == ConstructorFunction + && (d->m_arguments.isEmpty() + || d->m_arguments.constFirst().hasDefaultValueExpression()); +} + bool AbstractMetaFunction::needsReturnType() const { switch (d->m_functionType) { diff --git a/sources/shiboken6/ApiExtractor/abstractmetafunction.h b/sources/shiboken6/ApiExtractor/abstractmetafunction.h index 0c604f1e3..290f334f7 100644 --- a/sources/shiboken6/ApiExtractor/abstractmetafunction.h +++ b/sources/shiboken6/ApiExtractor/abstractmetafunction.h @@ -202,6 +202,8 @@ public: bool isDeprecated() const; bool isDestructor() const { return functionType() == DestructorFunction; } bool isConstructor() const; + bool isCopyConstructor() const { return functionType() == CopyConstructorFunction; } + bool isDefaultConstructor() const; bool needsReturnType() const; bool isInGlobalScope() const; bool isSignal() const { return functionType() == SignalFunction; } diff --git a/sources/shiboken6/ApiExtractor/abstractmetalang.cpp b/sources/shiboken6/ApiExtractor/abstractmetalang.cpp index e80bc4371..f3dc44dee 100644 --- a/sources/shiboken6/ApiExtractor/abstractmetalang.cpp +++ b/sources/shiboken6/ApiExtractor/abstractmetalang.cpp @@ -56,6 +56,8 @@ public: m_hasNonpublic(false), m_hasNonPrivateConstructor(false), m_hasPrivateConstructor(false), + m_hasDeletedDefaultConstructor(false), + m_hasDeletedCopyConstructor(false), m_functionsFixed(false), m_inheritanceDone(false), m_hasPrivateDestructor(false), @@ -75,6 +77,8 @@ public: uint m_hasNonpublic : 1; uint m_hasNonPrivateConstructor : 1; uint m_hasPrivateConstructor : 1; + uint m_hasDeletedDefaultConstructor : 1; + uint m_hasDeletedCopyConstructor : 1; uint m_functionsFixed : 1; uint m_inheritanceDone : 1; // m_baseClasses has been populated from m_baseClassNames uint m_hasPrivateDestructor : 1; @@ -759,8 +763,9 @@ void AbstractMetaClass::addDefaultConstructor() this->setHasNonPrivateConstructor(true); } -void AbstractMetaClass::addDefaultCopyConstructor(bool isPrivate) +void AbstractMetaClass::addDefaultCopyConstructor() { + d->m_hasNonPrivateConstructor = true; auto f = new AbstractMetaFunction; f->setType(AbstractMetaType::createVoid()); f->setOriginalName(name()); @@ -780,10 +785,7 @@ void AbstractMetaClass::addDefaultCopyConstructor(bool isPrivate) f->addArgument(arg); AbstractMetaAttributes::Attributes attr = FinalInTargetLang | AddedMethod; - if (isPrivate) - attr |= AbstractMetaAttributes::Private; - else - attr |= AbstractMetaAttributes::Public; + attr |= AbstractMetaAttributes::Public; f->setAttributes(attr); f->setImplementingClass(this); f->setOriginalAttributes(f->attributes()); @@ -811,6 +813,26 @@ void AbstractMetaClass::setHasPrivateConstructor(bool value) d->m_hasPrivateConstructor = value; } +bool AbstractMetaClass::hasDeletedDefaultConstructor() const +{ + return d->m_hasDeletedDefaultConstructor; +} + +void AbstractMetaClass::setHasDeletedDefaultConstructor(bool value) +{ + d->m_hasDeletedDefaultConstructor = value; +} + +bool AbstractMetaClass::hasDeletedCopyConstructor() const +{ + return d->m_hasDeletedCopyConstructor; +} + +void AbstractMetaClass::setHasDeletedCopyConstructor(bool value) +{ + d->m_hasDeletedCopyConstructor = value; +} + bool AbstractMetaClass::hasPrivateDestructor() const { return d->m_hasPrivateDestructor; @@ -843,9 +865,81 @@ void AbstractMetaClass::setHasVirtualDestructor(bool value) d->m_hasVirtuals = d->m_isPolymorphic = 1; } -bool AbstractMetaClass::isConstructible() const +bool AbstractMetaClass::isDefaultConstructible() const +{ + // Private constructors are skipped by the builder. + if (hasDeletedDefaultConstructor() || hasPrivateConstructor()) + return false; + const AbstractMetaFunctionCList ctors = + queryFunctions(FunctionQueryOption::Constructors); + for (const auto &ct : ctors) { + if (ct->isDefaultConstructor()) + return ct->visibility() == AbstractMetaAttributes::Public; + } + return ctors.isEmpty() && isImplicitlyDefaultConstructible(); +} + +// Non-comprehensive check for default constructible field +// (non-ref or not const value). +static bool defaultConstructibleField(const AbstractMetaField &f) +{ + const auto &type = f.type(); + return type.referenceType() == NoReference + && !(type.indirections() == 0 && type.isConstant()); // no const values +} + +bool AbstractMetaClass::isImplicitlyDefaultConstructible() const +{ + return std::all_of(d->m_fields.cbegin(), d->m_fields.cend(), + defaultConstructibleField) + && std::all_of(d->m_baseClasses.cbegin(), d->m_baseClasses.cend(), + [] (const AbstractMetaClass *c) { + return c->isDefaultConstructible(); + }); +} + +static bool canAddDefaultConstructorHelper(const AbstractMetaClass *cls) +{ + return !cls->isNamespace() + && !cls->attributes().testFlag(AbstractMetaAttributes::HasRejectedConstructor) + && !cls->hasPrivateDestructor(); +} + +bool AbstractMetaClass::canAddDefaultConstructor() const +{ + return canAddDefaultConstructorHelper(this) && !hasConstructors() + && !hasPrivateConstructor() && isImplicitlyDefaultConstructible(); +} + +bool AbstractMetaClass::isCopyConstructible() const { - return (hasNonPrivateConstructor() || !hasPrivateConstructor()) && !hasPrivateDestructor(); + // Private constructors are skipped by the builder. + if (hasDeletedCopyConstructor() || hasPrivateCopyConstructor()) + return false; + const AbstractMetaFunctionCList copyCtors = + queryFunctions(FunctionQueryOption::CopyConstructor); + return copyCtors.isEmpty() + ? isImplicitlyCopyConstructible() + : copyCtors.constFirst()->visibility() == AbstractMetaAttributes::Public; +} + +bool AbstractMetaClass::isImplicitlyCopyConstructible() const +{ + // Fields are currently not considered + return std::all_of(d->m_baseClasses.cbegin(), d->m_baseClasses.cend(), + [] (const AbstractMetaClass *c) { + return c->isCopyConstructible(); + }); +} + +bool AbstractMetaClass::canAddDefaultCopyConstructor() const +{ + if (!canAddDefaultConstructorHelper(this) + || !d->m_typeEntry->isValue() || isAbstract() + || hasPrivateCopyConstructor() || hasCopyConstructor()) { + return false; + } + return isImplicitlyCopyConstructible(); } bool AbstractMetaClass::generateExceptionHandling() const @@ -947,8 +1041,10 @@ bool AbstractMetaClass::queryFunction(const AbstractMetaFunction *f, FunctionQue return false; } - if (!query.testFlag(FunctionQueryOption::Constructors) && f->isConstructor()) + if (query.testFlag(FunctionQueryOption::CopyConstructor) + && (!f->isCopyConstructor() || f->ownerClass() != f->implementingClass())) { return false; + } // Destructors are never included in the functions of a class currently /* diff --git a/sources/shiboken6/ApiExtractor/abstractmetalang.h b/sources/shiboken6/ApiExtractor/abstractmetalang.h index e1e90863f..31a470667 100644 --- a/sources/shiboken6/ApiExtractor/abstractmetalang.h +++ b/sources/shiboken6/ApiExtractor/abstractmetalang.h @@ -78,7 +78,7 @@ public: bool hasPrivateCopyConstructor() const; void addDefaultConstructor(); - void addDefaultCopyConstructor(bool isPrivate = false); + void addDefaultCopyConstructor(); bool hasNonPrivateConstructor() const; void setHasNonPrivateConstructor(bool value); @@ -86,6 +86,12 @@ public: bool hasPrivateConstructor() const; void setHasPrivateConstructor(bool value); + bool hasDeletedDefaultConstructor() const; + void setHasDeletedDefaultConstructor(bool value); + + bool hasDeletedCopyConstructor() const; + void setHasDeletedCopyConstructor(bool value); + bool hasPrivateDestructor() const; void setHasPrivateDestructor(bool value); @@ -95,7 +101,13 @@ public: bool hasVirtualDestructor() const; void setHasVirtualDestructor(bool value); - bool isConstructible() const; + bool isDefaultConstructible() const; + bool isImplicitlyDefaultConstructible() const; + bool canAddDefaultConstructor() const; + + bool isCopyConstructible() const; + bool isImplicitlyCopyConstructible() const; + bool canAddDefaultCopyConstructor() const; bool generateExceptionHandling() const; diff --git a/sources/shiboken6/ApiExtractor/abstractmetalang_enums.h b/sources/shiboken6/ApiExtractor/abstractmetalang_enums.h index fd8f158d3..d2e50acba 100644 --- a/sources/shiboken6/ApiExtractor/abstractmetalang_enums.h +++ b/sources/shiboken6/ApiExtractor/abstractmetalang_enums.h @@ -33,6 +33,7 @@ enum class FunctionQueryOption { Constructors = 0x0000001, // Only constructors + CopyConstructor = 0x0000002, // Only copy constructors //Destructors = 0x0000002, // Only destructors. Not included in class. FinalInTargetLangFunctions = 0x0000008, // Only functions that are non-virtual in TargetLang ClassImplements = 0x0000020, // Only functions implemented by the current class diff --git a/sources/shiboken6/ApiExtractor/parser/codemodel.cpp b/sources/shiboken6/ApiExtractor/parser/codemodel.cpp index 2621838fe..8a4ab153b 100644 --- a/sources/shiboken6/ApiExtractor/parser/codemodel.cpp +++ b/sources/shiboken6/ApiExtractor/parser/codemodel.cpp @@ -784,6 +784,12 @@ void _FunctionModelItem::setVariadics(bool isVariadics) m_isVariadics = isVariadics; } +bool _FunctionModelItem::isDefaultConstructor() const +{ + return m_functionType == CodeModel::Constructor + && (m_arguments.isEmpty() || m_arguments.constFirst()->defaultValue()); +} + bool _FunctionModelItem::isNoExcept() const { return m_exceptionSpecification == ExceptionSpecification::NoExcept; diff --git a/sources/shiboken6/ApiExtractor/parser/codemodel.h b/sources/shiboken6/ApiExtractor/parser/codemodel.h index db61a65e7..e4c2ab55d 100644 --- a/sources/shiboken6/ApiExtractor/parser/codemodel.h +++ b/sources/shiboken6/ApiExtractor/parser/codemodel.h @@ -502,6 +502,7 @@ public: bool isVariadics() const; void setVariadics(bool isVariadics); + bool isDefaultConstructor() const; bool isSimilar(const FunctionModelItem &other) const; |
