From 5abbce3485e91bdbac532d4c811b9696f8e88cfc Mon Sep 17 00:00:00 2001 From: Friedemann Kleint Date: Mon, 4 Feb 2019 15:11:19 +0100 Subject: shiboken: Handle modifications in template inheritance Array modifications did not work in template specializations (like typedef QGenericMatrix<2,2,int> QMatrix2x2> causing warnings like: There's no user provided way (conversion rule, argument removal, custom code, etc) to handle the primitive type 'const float *' of argument 1 in function 'QMatrix2x2::QMatrix2x2(const float * values)'. Rewrite the array modification code to operate on AbstractMetaType only instead of requiring code model data types and add the missing handling to AbstractMetaBuilderPrivate::inheritTemplate(). Add a test. Note that the warning was fixed by another change removing the array modification since it did not take effect due to the presence of a manually added PySequence constructor. Change-Id: Ie4a1092fbef7237f8858790a74e2f75070ef6586 Reviewed-by: Cristian Maureira-Fredes --- .../shiboken2/ApiExtractor/abstractmetabuilder.cpp | 64 +++++++++++----------- .../shiboken2/ApiExtractor/abstractmetabuilder_p.h | 2 - .../shiboken2/ApiExtractor/abstractmetalang.cpp | 29 ++++++++++ sources/shiboken2/ApiExtractor/abstractmetalang.h | 1 + .../shiboken2/tests/libsample/nontypetemplate.h | 1 + .../tests/samplebinding/nontypetemplate_test.py | 15 +++++ .../tests/samplebinding/typesystem_sample.xml | 13 ++++- 7 files changed, 89 insertions(+), 36 deletions(-) diff --git a/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp b/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp index 37ff3b72c..0b11b1666 100644 --- a/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp +++ b/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp @@ -1840,34 +1840,27 @@ static inline AbstractMetaFunction::FunctionType functionTypeFromCodeModel(CodeM return result; } -bool AbstractMetaBuilderPrivate::setArrayArgumentType(AbstractMetaFunction *func, - const FunctionModelItem &functionItem, - int i) -{ - if (i < 0 || i >= func->arguments().size()) { - qCWarning(lcShiboken).noquote() - << msgCannotSetArrayUsage(func->minimalSignature(), i, - QLatin1String("Index out of range.")); - return false; - } - AbstractMetaType *metaType = func->arguments().at(i)->type(); - if (metaType->indirections() == 0) { - qCWarning(lcShiboken).noquote() - << msgCannotSetArrayUsage(func->minimalSignature(), i, - QLatin1String("Type does not have indirections.")); - return false; - } - TypeInfo elementType = functionItem->arguments().at(i)->type(); - elementType.setIndirections(elementType.indirections() - 1); - AbstractMetaType *element = translateType(elementType); - if (element == nullptr) { - qCWarning(lcShiboken).noquote() - << msgCannotSetArrayUsage(func->minimalSignature(), i, - QLatin1String("Cannot translate element type ") + elementType.toString()); - return false; +// Apply the modifications of the arguments +static bool applyArrayArgumentModifications(const FunctionModificationList &functionMods, + AbstractMetaFunction *func, + QString *errorMessage) +{ + for (const FunctionModification &mod : functionMods) { + for (const ArgumentModification &argMod : mod.argument_mods) { + if (argMod.array) { + const int i = argMod.index - 1; + if (i < 0 || i >= func->arguments().size()) { + *errorMessage = msgCannotSetArrayUsage(func->minimalSignature(), i, + QLatin1String("Index out of range.")); + return false; + } + if (!func->arguments().at(i)->type()->applyArrayModification(errorMessage)) { + *errorMessage = msgCannotSetArrayUsage(func->minimalSignature(), i, *errorMessage); + return false; + } + } + } } - metaType->setArrayElementType(element); - metaType->setTypeUsagePattern(AbstractMetaType::NativePointerAsArrayPattern); return true; } @@ -2109,11 +2102,10 @@ AbstractMetaFunction *AbstractMetaBuilderPrivate::traverseFunction(const Functio if (!metaArguments.isEmpty()) { fixArgumentNames(metaFunction, functionMods); - for (const FunctionModification &mod : functionMods) { - for (const ArgumentModification &argMod : mod.argument_mods) { - if (argMod.array) - setArrayArgumentType(metaFunction, functionItem, argMod.index - 1); - } + QString errorMessage; + if (!applyArrayArgumentModifications(functionMods, metaFunction, &errorMessage)) { + qCWarning(lcShiboken, "While traversing %s: %s", + qPrintable(className), qPrintable(errorMessage)); } } @@ -2744,6 +2736,7 @@ bool AbstractMetaBuilderPrivate::inheritTemplate(AbstractMetaClass *subclass, { QVector targs = info.instantiations(); QVector templateTypes; + QString errorMessage; if (subclass->isTypeDef()) { subclass->setHasCloneOperator(templateClass->hasCloneOperator()); @@ -2876,6 +2869,13 @@ bool AbstractMetaBuilderPrivate::inheritTemplate(AbstractMetaClass *subclass, te->addFunctionModification(mod); } + + if (!applyArrayArgumentModifications(f->modifications(subclass), f.data(), + &errorMessage)) { + qCWarning(lcShiboken, "While specializing %s (%s): %s", + qPrintable(subclass->name()), qPrintable(templateClass->name()), + qPrintable(errorMessage)); + } subclass->addFunction(f.take()); } diff --git a/sources/shiboken2/ApiExtractor/abstractmetabuilder_p.h b/sources/shiboken2/ApiExtractor/abstractmetabuilder_p.h index 226d6defd..185dd0e30 100644 --- a/sources/shiboken2/ApiExtractor/abstractmetabuilder_p.h +++ b/sources/shiboken2/ApiExtractor/abstractmetabuilder_p.h @@ -156,8 +156,6 @@ public: AbstractMetaArgumentList reverseList(const AbstractMetaArgumentList &list); void setInclude(TypeEntry *te, const QString &fileName) const; void fixArgumentNames(AbstractMetaFunction *func, const FunctionModificationList &mods); - bool setArrayArgumentType(AbstractMetaFunction *func, - const FunctionModelItem &functionItem, int i); void fillAddedFunctions(AbstractMetaClass *metaClass); diff --git a/sources/shiboken2/ApiExtractor/abstractmetalang.cpp b/sources/shiboken2/ApiExtractor/abstractmetalang.cpp index a10a15b08..5940aa86a 100644 --- a/sources/shiboken2/ApiExtractor/abstractmetalang.cpp +++ b/sources/shiboken2/ApiExtractor/abstractmetalang.cpp @@ -207,6 +207,35 @@ AbstractMetaType *AbstractMetaType::copy() const return cpy; } +// For applying the function argument modification: change into a type +// where "int *" becomes "int[]". +bool AbstractMetaType::applyArrayModification(QString *errorMessage) +{ + if (m_pattern == AbstractMetaType::NativePointerAsArrayPattern) { + *errorMessage = QLatin1String(" modification already applied."); + return false; + } + if (m_arrayElementType != nullptr) { + QTextStream(errorMessage) << "The type \"" << cppSignature() + << "\" is an array of " << m_arrayElementType->name() << '.'; + return false; + } + if (m_indirections.isEmpty()) { + QTextStream(errorMessage) << "The type \"" << cppSignature() + << "\" does not have indirections."; + return false; + } + // Element type to be used for ArrayHandle<>, strip constness. + auto elementType = copy(); + elementType->m_indirections.pop_front(); + elementType->setConstant(false); + elementType->setVolatile(false); + elementType->decideUsagePattern(); + m_arrayElementType = elementType; + setTypeUsagePattern(AbstractMetaType::NativePointerAsArrayPattern); + return true; +} + AbstractMetaTypeCList AbstractMetaType::nestedArrayTypes() const { AbstractMetaTypeCList result; diff --git a/sources/shiboken2/ApiExtractor/abstractmetalang.h b/sources/shiboken2/ApiExtractor/abstractmetalang.h index 074adbe00..9d36706ac 100644 --- a/sources/shiboken2/ApiExtractor/abstractmetalang.h +++ b/sources/shiboken2/ApiExtractor/abstractmetalang.h @@ -488,6 +488,7 @@ public: QString cppSignature() const; AbstractMetaType *copy() const; + bool applyArrayModification(QString *errorMessage); const TypeEntry *typeEntry() const { diff --git a/sources/shiboken2/tests/libsample/nontypetemplate.h b/sources/shiboken2/tests/libsample/nontypetemplate.h index 4e2100626..5a9e670c6 100644 --- a/sources/shiboken2/tests/libsample/nontypetemplate.h +++ b/sources/shiboken2/tests/libsample/nontypetemplate.h @@ -37,6 +37,7 @@ template class IntArray { public: + explicit IntArray(const int *data) { std::copy(data, data + Size, m_array); } explicit IntArray(int v) { std::fill(m_array, m_array + Size, v); } int sum() const { return std::accumulate(m_array, m_array + Size, int(0)); } diff --git a/sources/shiboken2/tests/samplebinding/nontypetemplate_test.py b/sources/shiboken2/tests/samplebinding/nontypetemplate_test.py index 9adfa2441..a7a4da72b 100644 --- a/sources/shiboken2/tests/samplebinding/nontypetemplate_test.py +++ b/sources/shiboken2/tests/samplebinding/nontypetemplate_test.py @@ -28,6 +28,14 @@ ## ############################################################################# +hasNumPy = False + +try: + import numpy + hasNumPy = True +except ImportError: + pass + import unittest from sample import IntArray2, IntArray3 @@ -40,5 +48,12 @@ class NonTypeTemplateTest(unittest.TestCase): array3 = IntArray3(5) self.assertEqual(array3.sum(), 15) + def testArrayInitializer(self): + if not hasNumPy: + return + array3 = IntArray3(numpy.array([1, 2, 3], dtype = 'int32')) + self.assertEqual(array3.sum(), 6) + + if __name__ == '__main__': unittest.main() diff --git a/sources/shiboken2/tests/samplebinding/typesystem_sample.xml b/sources/shiboken2/tests/samplebinding/typesystem_sample.xml index 9b967fc53..78ceaab43 100644 --- a/sources/shiboken2/tests/samplebinding/typesystem_sample.xml +++ b/sources/shiboken2/tests/samplebinding/typesystem_sample.xml @@ -521,8 +521,17 @@ - - + + + + + + + + + + +