From 9c6b541ceef3039e9e19a1d9a12223851a849cf6 Mon Sep 17 00:00:00 2001 From: Marcelo Lira Date: Wed, 10 Aug 2011 00:59:19 -0300 Subject: Improved the generation of argument conversion in modified functions. Added the SBK_UNUSED macro to libshiboken to prevent compilation warnings. An unit test was added. Reviewed by Hugo Parente Reviewed by Luciano Wolf --- generator/cppgenerator.cpp | 77 +++++++++++++------------------ generator/cppgenerator.h | 16 ++++--- generator/shibokengenerator.cpp | 46 +++++++++++++++--- generator/shibokengenerator.h | 15 ++++++ libshiboken/helper.h | 2 + tests/libsample/modifications.cpp | 20 ++++++++ tests/libsample/modifications.h | 14 ++++-- tests/samplebinding/modifications_test.py | 6 +++ tests/samplebinding/typesystem_sample.xml | 15 +++++- 9 files changed, 149 insertions(+), 62 deletions(-) diff --git a/generator/cppgenerator.cpp b/generator/cppgenerator.cpp index 3ae270eb4..30fe65f1c 100644 --- a/generator/cppgenerator.cpp +++ b/generator/cppgenerator.cpp @@ -945,7 +945,7 @@ void CppGenerator::writeMethodWrapperPreamble(QTextStream& s, OverloadData& over if (maxArgs > 0) s << INDENT << "int overloadId = -1;" << endl; - if (usesNamedArguments) + if (usesNamedArguments && !rfunc->isCallOperator()) s << INDENT << "int numNamedArgs = (kwds ? PyDict_Size(kwds) : 0);" << endl; if (initPythonArguments) { @@ -1361,9 +1361,6 @@ void CppGenerator::writeCppSelfDefinition(QTextStream& s, const AbstractMetaFunc } writeCppSelfDefinition(s, func->ownerClass(), hasStaticOverload); - - if (func->isUserAdded()) - s << INDENT << "(void)" CPP_SELF_VAR "; // avoid warnings about unused variables" << endl; } void CppGenerator::writeErrorSection(QTextStream& s, OverloadData& overloadData) @@ -1520,13 +1517,16 @@ void CppGenerator::writeArgumentConversion(QTextStream& s, const AbstractMetaType* argType, const QString& argName, const QString& pyArgName, const AbstractMetaClass* context, - const QString& defaultValue) + const QString& defaultValue, + bool castArgumentAsUnused) { if (argType->typeEntry()->isCustom() || argType->typeEntry()->isVarargs()) return; if (isWrapperType(argType)) writeInvalidPyObjectCheck(s, pyArgName); writePythonToCppTypeConversion(s, argType, pyArgName, argName, context, defaultValue); + if (castArgumentAsUnused) + writeUnusedVariableCast(s, argName); } const AbstractMetaType* CppGenerator::getArgumentType(const AbstractMetaFunction* func, int argPos) @@ -1845,28 +1845,38 @@ void CppGenerator::writeSingleFunctionCall(QTextStream& s, const OverloadData& o // Handle named arguments. writeNamedArgumentResolution(s, func, usePyArgs); + bool injectCodeCallsFunc = injectedCodeCallsCppFunction(func); + bool mayHaveUnunsedArguments = !func->isUserAdded() && func->hasInjectedCode() && injectCodeCallsFunc; int removedArgs = 0; for (int argIdx = 0; argIdx < func->arguments().count(); ++argIdx) { + bool hasConversionRule = !func->conversionRule(TypeSystem::NativeCode, argIdx + 1).isEmpty(); + const AbstractMetaArgument* arg = func->arguments().at(argIdx); if (func->argumentRemoved(argIdx + 1)) { + if (!arg->defaultValueExpression().isEmpty()) { + QString cppArgRemoved = QString(CPP_ARG_REMOVED"%1").arg(argIdx); + s << INDENT << getFullTypeName(arg->type()) << ' ' << cppArgRemoved; + s << " = " << guessScopeForDefaultValue(func, arg) << ';' << endl; + writeUnusedVariableCast(s, cppArgRemoved); + } else if (!injectCodeCallsFunc && !func->isUserAdded() && !hasConversionRule) { + // When an argument is removed from a method signature and no other means of calling + // the method are provided (as with code injection) the generator must abort. + qFatal(qPrintable(QString("No way to call '%1::%2' with the modifications described in the type system.") + .arg(func->ownerClass()->name()) + .arg(func->signature())), NULL); + } removedArgs++; continue; } - - if (!func->conversionRule(TypeSystem::NativeCode, argIdx + 1).isEmpty()) + if (hasConversionRule) continue; - const AbstractMetaType* argType = getArgumentType(func, argIdx + 1); - - if (!argType) + if (!argType || (mayHaveUnunsedArguments && !injectedCodeUsesArgument(func, argIdx))) continue; - int argPos = argIdx - removedArgs; QString argName = QString(CPP_ARG"%1").arg(argPos); QString pyArgName = usePyArgs ? QString(PYTHON_ARGS "[%1]").arg(argPos) : PYTHON_ARG; - const AbstractMetaArgument* arg = func->arguments().at(argIdx); QString defaultValue = guessScopeForDefaultValue(func, arg); - - writeArgumentConversion(s, argType, argName, pyArgName, implementingClass, defaultValue); + writeArgumentConversion(s, argType, argName, pyArgName, implementingClass, defaultValue, func->isUserAdded()); } s << endl; @@ -2017,9 +2027,7 @@ void CppGenerator::writeMethodCall(QTextStream& s, const AbstractMetaFunction* f writeConversionRule(s, func, TypeSystem::NativeCode); if (!func->isUserAdded()) { - bool badModifications = false; QStringList userArgs; - if (!func->isCopyConstructor()) { int removedArgs = 0; for (int i = 0; i < maxArgs + removedArgs; i++) { @@ -2032,14 +2040,10 @@ void CppGenerator::writeMethodCall(QTextStream& s, const AbstractMetaFunction* f removedArgs++; // If have conversion rules I will use this for removed args - if (hasConversionRule) { + if (hasConversionRule) userArgs << QString("%1"CONV_RULE_OUT_VAR_SUFFIX).arg(arg->name()); - } else { - if (arg->defaultValueExpression().isEmpty()) - badModifications = true; - else - userArgs << guessScopeForDefaultValue(func, arg); - } + else if (!arg->defaultValueExpression().isEmpty()) + userArgs << QString(CPP_ARG_REMOVED"%1").arg(i); } else { int idx = arg->argumentIndex() - removedArgs; QString argName = hasConversionRule @@ -2064,15 +2068,11 @@ void CppGenerator::writeMethodCall(QTextStream& s, const AbstractMetaFunction* f continue; else argsClear = false; - otherArgsModified |= defValModified || hasConversionRule || func->argumentRemoved(i + 1); - - if (!arg->defaultValueExpression().isEmpty()) - otherArgs.prepend(guessScopeForDefaultValue(func, arg)); - else if (hasConversionRule) + if (hasConversionRule) otherArgs.prepend(QString("%1"CONV_RULE_OUT_VAR_SUFFIX).arg(arg->name())); else - badModifications = true; + otherArgs.prepend(QString(CPP_ARG_REMOVED"%1").arg(i)); } if (otherArgsModified) userArgs << otherArgs; @@ -2082,16 +2082,7 @@ void CppGenerator::writeMethodCall(QTextStream& s, const AbstractMetaFunction* f QString methodCall; QTextStream mc(&methodCall); - if (badModifications) { - // When an argument is removed from a method signature and no other - // means of calling the method is provided (as with code injection) - // the generator must write a compiler error line stating the situation. - if (func->injectedCodeSnips(CodeSnip::Any, TypeSystem::TargetLangCode).isEmpty()) { - qFatal(qPrintable("No way to call \"" + func->ownerClass()->name() - + "::" + func->minimalSignature() - + "\" with the modifications described in the type system file"), NULL); - } - } else if (func->isOperatorOverload() && !func->isCallOperator()) { + if (func->isOperatorOverload() && !func->isCallOperator()) { QByteArray firstArg("(*" CPP_SELF_VAR ")"); if (func->isPointerOperator()) firstArg.remove(1, 1); // remove the de-reference operator @@ -2953,6 +2944,7 @@ void CppGenerator::writeRichCompareFunction(QTextStream& s, const AbstractMetaCl s << baseName << "_richcompare(PyObject* " PYTHON_SELF_VAR ", PyObject* " PYTHON_ARG ", int op)" << endl; s << '{' << endl; writeCppSelfDefinition(s, metaClass, false, true); + writeUnusedVariableCast(s, CPP_SELF_VAR); s << INDENT << "PyObject* " PYTHON_RETURN_VAR " = 0;" << endl; s << endl; @@ -2983,26 +2975,21 @@ void CppGenerator::writeRichCompareFunction(QTextStream& s, const AbstractMetaCl const AbstractMetaFunction* func = data->referenceFunction(); if (func->isStatic()) continue; - const AbstractMetaType* argType = getArgumentType(func, 1); - if (!argType) continue; - bool numberType = alternativeNumericTypes == 1 || ShibokenGenerator::isPyInt(argType); - if (!first) { s << " else "; } else { first = false; s << INDENT; } - s << "if (" << cpythonIsConvertibleFunction(argType, numberType) << "(" PYTHON_ARG ")) {" << endl; { Indentation indent(INDENT); s << INDENT << "// " << func->signature() << endl; - writeArgumentConversion(s, argType, CPP_ARG0, PYTHON_ARG, metaClass); + writeArgumentConversion(s, argType, CPP_ARG0, PYTHON_ARG, metaClass, QString(), func->isUserAdded()); // If the function is user added, use the inject code if (func->isUserAdded()) { diff --git a/generator/cppgenerator.h b/generator/cppgenerator.h index b6ce72ce7..18eb01be0 100644 --- a/generator/cppgenerator.h +++ b/generator/cppgenerator.h @@ -77,17 +77,19 @@ private: * Writes Python to C++ conversions for arguments on Python wrappers. * If implicit conversions, and thus new object allocation, are needed, * code to deallocate a possible new instance is also generated. - * \param s text stream to write - * \param argType a pointer to the argument type to be converted - * \param argName C++ argument name - * \param pyArgName Python argument name - * \param context the current meta class - * \param defaultValue an optional default value to be used instead of the conversion result + * \param s text stream to write + * \param argType a pointer to the argument type to be converted + * \param argName C++ argument name + * \param pyArgName Python argument name + * \param context the current meta class + * \param defaultValue an optional default value to be used instead of the conversion result + * \param castArgumentAsUnused if true the converted argument is cast as unused to avoid compiler warnings */ void writeArgumentConversion(QTextStream& s, const AbstractMetaType* argType, const QString& argName, const QString& pyArgName, const AbstractMetaClass* context = 0, - const QString& defaultValue = QString()); + const QString& defaultValue = QString(), + bool castArgumentAsUnused = false); /** * Returns the AbstractMetaType for a function argument. diff --git a/generator/shibokengenerator.cpp b/generator/shibokengenerator.cpp index 90adc58ac..2d317c650 100644 --- a/generator/shibokengenerator.cpp +++ b/generator/shibokengenerator.cpp @@ -1164,6 +1164,11 @@ void ShibokenGenerator::writeFunctionCall(QTextStream& s, s << ')'; } +void ShibokenGenerator::writeUnusedVariableCast(QTextStream& s, const QString& variableName) +{ + s << INDENT << "SBK_UNUSED(" << variableName<< ')' << endl; +} + AbstractMetaFunctionList ShibokenGenerator::filterFunctions(const AbstractMetaClass* metaClass) { AbstractMetaFunctionList result; @@ -1281,12 +1286,10 @@ ShibokenGenerator::ArgumentVarReplacementList ShibokenGenerator::getArgumentRepl bool hasConversionRule = !func->conversionRule(convLang, i+1).isEmpty(); bool argRemoved = func->argumentRemoved(i+1); removed = removed + (int) argRemoved; - if (argRemoved || (lastArg && arg->argumentIndex() > lastArg->argumentIndex())) - argValue = arg->defaultValueExpression(); - - if (argRemoved && hasConversionRule && argValue.isEmpty()) + if (argRemoved && hasConversionRule) argValue = QString("%1"CONV_RULE_OUT_VAR_SUFFIX).arg(arg->name()); - + else if (argRemoved || (lastArg && arg->argumentIndex() > lastArg->argumentIndex())) + argValue = QString(CPP_ARG_REMOVED"%1").arg(i); if (!argRemoved && argValue.isEmpty()) { int argPos = i - removed; if (arg->type()->typeEntry()->isCustom()) { @@ -1456,8 +1459,11 @@ void ShibokenGenerator::writeCodeSnips(QTextStream& s, ArgumentVarReplacementList argReplacements = getArgumentReplacement(func, usePyArgs, language, lastArg); QStringList args; - foreach (ArgumentVarReplacementPair pair, argReplacements) + foreach (ArgumentVarReplacementPair pair, argReplacements) { + if (pair.second.startsWith(CPP_ARG_REMOVED)) + continue; args << pair.second; + } code.replace("%ARGUMENT_NAMES", args.join(", ")); foreach (ArgumentVarReplacementPair pair, argReplacements) { @@ -1623,6 +1629,19 @@ bool ShibokenGenerator::injectedCodeHasReturnValueAttribution(const AbstractMeta return false; } +bool ShibokenGenerator::injectedCodeUsesArgument(const AbstractMetaFunction* func, int argumentIndex) +{ + CodeSnipList snips = func->injectedCodeSnips(CodeSnip::Any); + foreach (CodeSnip snip, snips) { + QString code = snip.code(); + if (code.contains("%ARGUMENT_NAMES")) + return true; + if (code.contains(QString("%%1").arg(argumentIndex + 1))) + return true; + } + return false; +} + bool ShibokenGenerator::hasMultipleInheritanceInAncestry(const AbstractMetaClass* metaClass) { if (!metaClass || metaClass->baseClassNames().isEmpty()) @@ -1948,6 +1967,21 @@ QString ShibokenGenerator::getTypeIndexVariableName(const AbstractMetaType* type return QString("SBK%1_IDX").arg(processInstantiationsVariableName(type)); } +QString ShibokenGenerator::getFullTypeName(const TypeEntry* type) +{ + return QString("%1%2").arg(type->isCppPrimitive() ? "" : "::").arg(type->qualifiedCppName()); +} +QString ShibokenGenerator::getFullTypeName(const AbstractMetaType* type) +{ + if (isCString(type)) + return QString("const char*"); + return getFullTypeName(type->typeEntry()) + QString("*").repeated(type->indirections()); +} +QString ShibokenGenerator::getFullTypeName(const AbstractMetaClass* metaClass) +{ + return getFullTypeName(metaClass->typeEntry()); +} + bool ShibokenGenerator::verboseErrorMessagesDisabled() const { return m_verboseErrorMessagesDisabled; diff --git a/generator/shibokengenerator.h b/generator/shibokengenerator.h index b6d336f05..226450231 100644 --- a/generator/shibokengenerator.h +++ b/generator/shibokengenerator.h @@ -27,6 +27,7 @@ #define CONV_RULE_OUT_VAR_SUFFIX "_out" #define CPP_ARG "cppArg" #define CPP_ARG0 CPP_ARG"0" +#define CPP_ARG_REMOVED "removed_"CPP_ARG #define CPP_RETURN_VAR "cppResult" #define CPP_SELF_VAR "cppSelf" #define PYTHON_ARG "pyArg" @@ -205,6 +206,12 @@ public: */ bool injectedCodeHasReturnValueAttribution(const AbstractMetaFunction* func, TypeSystem::Language language = TypeSystem::TargetLangCode); + /** + * Verifies if any of the function's code injections uses the type system variable + * for function arguments of a given index. + */ + bool injectedCodeUsesArgument(const AbstractMetaFunction* func, int argumentIndex); + /** * Function which parse the metafunction information * \param func the function witch will be parserd @@ -394,6 +401,12 @@ public: QString getTypeIndexVariableName(const AbstractMetaClass* metaClass, bool alternativeTemplateName = false); QString getTypeIndexVariableName(const TypeEntry* type); QString getTypeIndexVariableName(const AbstractMetaType* type); + + /// Returns the proper full name for \p type. + QString getFullTypeName(const TypeEntry* type); + QString getFullTypeName(const AbstractMetaType* type); + QString getFullTypeName(const AbstractMetaClass* metaClass); + /// Returns true if the user don't want verbose error messages on the generated bindings. bool verboseErrorMessagesDisabled() const; @@ -440,6 +453,8 @@ protected: const AbstractMetaFunction* metaFunc, Options options = NoOption) const; + void writeUnusedVariableCast(QTextStream& s, const QString& variableName); + AbstractMetaFunctionList filterFunctions(const AbstractMetaClass* metaClass); // All data about extended converters: the type entries of the target type, and a diff --git a/libshiboken/helper.h b/libshiboken/helper.h index f179b2d7c..13c119afc 100644 --- a/libshiboken/helper.h +++ b/libshiboken/helper.h @@ -28,6 +28,8 @@ #include "conversions.h" #include "autodecref.h" +#define SBK_UNUSED(x) (void)x; + namespace Shiboken { diff --git a/tests/libsample/modifications.cpp b/tests/libsample/modifications.cpp index 739ca5c74..d809e823e 100644 --- a/tests/libsample/modifications.cpp +++ b/tests/libsample/modifications.cpp @@ -22,9 +22,21 @@ #include #include "modifications.h" +#include "objecttype.h" using namespace std; +Modifications::Modifications() +{ + m_object = new ObjectType(); + m_object->setObjectName("MyObject"); +} + +Modifications::~Modifications() +{ + delete m_object; +} + std::pair Modifications::pointToPair(Point pt, bool* ok) { @@ -109,3 +121,11 @@ Modifications::sumPointCoordinates(const Point* point) { return point->x() + point->y(); } + +bool +Modifications::nonConversionRuleForArgumentWithDefaultValue(ObjectType** object) +{ + if (object) + *object = m_object; + return true; +} diff --git a/tests/libsample/modifications.h b/tests/libsample/modifications.h index c7a7748c6..2acffa406 100644 --- a/tests/libsample/modifications.h +++ b/tests/libsample/modifications.h @@ -27,11 +27,13 @@ #include #include "point.h" +class ObjectType; + class LIBSAMPLE_API Modifications { public: - Modifications() {} - virtual ~Modifications() {} + Modifications(); + virtual ~Modifications(); enum OverloadedModFunc { OverloadedNone, @@ -102,6 +104,13 @@ public: // Mark the argument with a tag; // the test implementation must expect point never to be null. int sumPointCoordinates(const Point* point); + + // Sets an ObjectType in the argument and returns true. + bool nonConversionRuleForArgumentWithDefaultValue(ObjectType** object = 0); + ObjectType* getObject() const { return m_object; } + +private: + ObjectType* m_object; }; class LIBSAMPLE_API AbstractModifications : public Modifications @@ -117,4 +126,3 @@ public: }; #endif // MODIFICATIONS_H - diff --git a/tests/samplebinding/modifications_test.py b/tests/samplebinding/modifications_test.py index 344801015..3cb502ce6 100644 --- a/tests/samplebinding/modifications_test.py +++ b/tests/samplebinding/modifications_test.py @@ -148,5 +148,11 @@ class ModificationsTest(unittest.TestCase): self.assertEqual(self.mods.sumPointCoordinates(point), 12 + 34) self.assertRaises(TypeError, self.mods.sumPointCoordinates, None) + def testNonConversionRuleForArgumentWithDefaultValue(self): + status, obj = self.mods.nonConversionRuleForArgumentWithDefaultValue() + self.assert_(status) + self.assertEqual(obj, self.mods.getObject()) + self.assertEqual(obj.objectName(), 'MyObject') + if __name__ == '__main__': unittest.main() diff --git a/tests/samplebinding/typesystem_sample.xml b/tests/samplebinding/typesystem_sample.xml index dc4faf352..4f1d5481b 100644 --- a/tests/samplebinding/typesystem_sample.xml +++ b/tests/samplebinding/typesystem_sample.xml @@ -728,6 +728,19 @@ + + + + + + + + + ObjectType* tmpObject = 0; + %RETURN_TYPE %0 = %CPPSELF.%FUNCTION_NAME(&tmpObject); + %PYARG_0 = Shiboken::makeTuple(%0, tmpObject); + + @@ -1761,4 +1774,4 @@ - \ No newline at end of file + -- cgit v1.2.3