diff options
author | Christian Tismer <tismer@stackless.com> | 2023-03-15 18:55:50 +0100 |
---|---|---|
committer | Christian Tismer <tismer@stackless.com> | 2023-03-20 16:38:23 +0100 |
commit | 18812159a8cd5295ac8d51e37f9021ad21434b90 (patch) | |
tree | 6939bf411c9bdf316231a081ed1d44d28ca7af8d /sources/shiboken6 | |
parent | 5468617478c93951de7744b6bad8608911a78880 (diff) |
cppgenerator: get rid of all goto constructs
[ChangeLog][shiboken6] The generated code no longer contains `goto`.
Change-Id: I3b90abafd8dbe2c19b4fffb7880322451d1ed068
Fixes: PYSIDE-2256
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@qt.io>
Diffstat (limited to 'sources/shiboken6')
-rw-r--r-- | sources/shiboken6/generator/shiboken/cppgenerator.cpp | 99 | ||||
-rw-r--r-- | sources/shiboken6/generator/shiboken/cppgenerator.h | 13 | ||||
-rw-r--r-- | sources/shiboken6/libshiboken/basewrapper.cpp | 27 | ||||
-rw-r--r-- | sources/shiboken6/libshiboken/basewrapper.h | 21 |
4 files changed, 100 insertions, 60 deletions
diff --git a/sources/shiboken6/generator/shiboken/cppgenerator.cpp b/sources/shiboken6/generator/shiboken/cppgenerator.cpp index 9f2d74093..63fc65f19 100644 --- a/sources/shiboken6/generator/shiboken/cppgenerator.cpp +++ b/sources/shiboken6/generator/shiboken/cppgenerator.cpp @@ -2387,7 +2387,7 @@ void CppGenerator::writeConstructorWrapper(TextStream &s, const OverloadData &ov s << '\n'; if (overloadData.maxArgs() > 0) - writeOverloadedFunctionDecisor(s, overloadData); + writeOverloadedFunctionDecisor(s, overloadData, errorReturn); writeFunctionCalls(s, overloadData, classContext, errorReturn); s << '\n'; @@ -2399,7 +2399,9 @@ void CppGenerator::writeConstructorWrapper(TextStream &s, const OverloadData &ov << indent << "delete cptr;\n" << errorReturn << outdent << "}\n"; if (overloadData.maxArgs() > 0) - s << "if (!cptr) goto " << cpythonFunctionName(rfunc) << "_TypeError;\n\n"; + s << "if (!cptr)\n" << indent + << "return " << returnErrorWrongArguments(overloadData, errorReturn) << ";\n\n" + << outdent; s << "Shiboken::Object::setValidCpp(sbkSelf, true);\n"; // If the created C++ object has a C++ wrapper the ownership is assigned to Python @@ -2422,7 +2424,8 @@ void CppGenerator::writeConstructorWrapper(TextStream &s, const OverloadData &ov << "metaObject = cptr->metaObject(); // <- init python qt properties\n" << "if (!errInfo.isNull() && PyDict_Check(errInfo.object())) {\n" << indent << "if (!PySide::fillQtProperties(self, metaObject, errInfo))\n" << indent - << "goto " << cpythonFunctionName(rfunc) << "_TypeError;\n" << outdent << outdent + << "return " << returnErrorWrongArguments(overloadData, errorReturn) << ";\n" + << outdent << outdent << "};\n"; } @@ -2460,8 +2463,6 @@ void CppGenerator::writeConstructorWrapper(TextStream &s, const OverloadData &ov } s << "\n\nreturn 1;\n"; - if (needsArgumentErrorHandling(overloadData)) - writeErrorSection(s, overloadData, errorReturn); s<< outdent << "}\n\n"; } @@ -2525,12 +2526,12 @@ void CppGenerator::writeMethodWrapper(TextStream &s, const OverloadData &overloa << "// Do not enter here if other object has implemented a reverse operator.\n" << "if (!" << PYTHON_RETURN_VAR << ") {\n" << indent; if (maxArgs > 0) - writeOverloadedFunctionDecisor(s, overloadData); + writeOverloadedFunctionDecisor(s, overloadData, ErrorReturn::Default); writeFunctionCalls(s, overloadData, classContext, ErrorReturn::Default); s << outdent << '\n' << "} // End of \"if (!" << PYTHON_RETURN_VAR << ")\"\n"; } else { // binary shift operator if (maxArgs > 0) - writeOverloadedFunctionDecisor(s, overloadData); + writeOverloadedFunctionDecisor(s, overloadData, ErrorReturn::Default); writeFunctionCalls(s, overloadData, classContext, ErrorReturn::Default); } @@ -2549,9 +2550,6 @@ void CppGenerator::writeMethodWrapper(TextStream &s, const OverloadData &overloa s << "Py_RETURN_NONE;\n"; } - if (needsArgumentErrorHandling(overloadData)) - writeErrorSection(s, overloadData, ErrorReturn::Default); - s<< outdent << "}\n\n"; } @@ -2596,7 +2594,8 @@ void CppGenerator::writeArgumentsInitializer(TextStream &s, const OverloadData & s << "errInfo.reset(Shiboken::checkInvalidArgumentCount(numArgs, " << minArgs << ", " << maxArgs << "));\n" << "if (!errInfo.isNull())\n" << indent - << "goto " << cpythonFunctionName(rfunc) << "_TypeError;\n" << outdent; + << "return " << returnErrorWrongArguments(overloadData, errorReturn) << ";\n" + << outdent; } const QList<int> invalidArgsLength = overloadData.invalidArgumentLengths(); @@ -2608,7 +2607,8 @@ void CppGenerator::writeArgumentsInitializer(TextStream &s, const OverloadData & s << "numArgs == " << invalidArgsLength.at(i); } s << ")\n" << indent - << "goto " << cpythonFunctionName(rfunc) << "_TypeError;\n" << outdent; + << "return " << returnErrorWrongArguments(overloadData, errorReturn) << ";\n" + << outdent; } s << '\n'; @@ -2747,15 +2747,21 @@ void CppGenerator::writeCppSelfDefinition(TextStream &s, writeCppSelfDefinition(s, context, errorReturn, flags); } -void CppGenerator::writeErrorSection(TextStream &s, const OverloadData &overloadData, - ErrorReturn errorReturn) +QString CppGenerator::returnErrorWrongArguments(const OverloadData &overloadData, + ErrorReturn errorReturn) { const auto rfunc = overloadData.referenceFunction(); QString argsVar = overloadData.pythonFunctionWrapperUsesListOfArguments() ? u"args"_s : PYTHON_ARG; - s << '\n' << cpythonFunctionName(rfunc) << "_TypeError:\n" << indent - << "Shiboken::setErrorAboutWrongArguments(" << argsVar << ", fullName, errInfo);\n" - << errorReturn << outdent; + switch (errorReturn) { + case ErrorReturn::Default: + return u"Shiboken::returnWrongArguments("_s + argsVar + u", fullName, errInfo)"_s; + case ErrorReturn::Zero: + return u"Shiboken::returnWrongArguments_Zero("_s + argsVar + u", fullName, errInfo)"_s; + case ErrorReturn::MinusOne: + return u"Shiboken::returnWrongArguments_MinusOne("_s + argsVar + u", fullName, errInfo)"_s; + } + return QString(); } void CppGenerator::writeFunctionReturnErrorCheckSection(TextStream &s, @@ -3143,7 +3149,9 @@ void CppGenerator::writeNoneReturn(TextStream &s, const AbstractMetaFunctionCPtr } } -void CppGenerator::writeOverloadedFunctionDecisor(TextStream &s, const OverloadData &overloadData) const +void CppGenerator::writeOverloadedFunctionDecisor(TextStream &s, + const OverloadData &overloadData, + ErrorReturn errorReturn) const { s << "// Overloaded function decisor\n"; const auto rfunc = overloadData.referenceFunction(); @@ -3170,8 +3178,9 @@ void CppGenerator::writeOverloadedFunctionDecisor(TextStream &s, const OverloadD } s << "// Function signature not found.\n" - << "if (overloadId == -1) goto " - << cpythonFunctionName(overloadData.referenceFunction()) << "_TypeError;\n\n"; + << "if (overloadId == -1)\n" << indent + << "return " << returnErrorWrongArguments(overloadData, errorReturn) << ";\n\n" + << outdent; } void CppGenerator::writeOverloadedFunctionDecisorEngine(TextStream &s, @@ -3386,7 +3395,7 @@ void CppGenerator::writeSingleFunctionCall(TextStream &s, const bool usePyArgs = overloadData.pythonFunctionWrapperUsesListOfArguments(); // Handle named arguments. - writeNamedArgumentResolution(s, func, usePyArgs, overloadData); + writeNamedArgumentResolution(s, func, usePyArgs, overloadData, errorReturn); bool injectCodeCallsFunc = injectedCodeCallsCppFunction(context, func); bool mayHaveUnunsedArguments = !func->isUserAdded() && func->hasInjectedCode() && injectCodeCallsFunc; @@ -3781,7 +3790,8 @@ static bool forceQObjectNamedArguments(const AbstractMetaFunctionCPtr &func) } void CppGenerator::writeNamedArgumentResolution(TextStream &s, const AbstractMetaFunctionCPtr &func, - bool usePyArgs, const OverloadData &overloadData) const + bool usePyArgs, const OverloadData &overloadData, + ErrorReturn errorReturn) const { const AbstractMetaArgumentList &args = OverloadData::getArgumentsWithDefaultValues(func); const bool hasDefaultArguments = !args.isEmpty(); @@ -3793,7 +3803,7 @@ void CppGenerator::writeNamedArgumentResolution(TextStream &s, const AbstractMet s << "if (kwds && PyDict_Size(kwds) > 0) {\n" << indent << "errInfo.reset(kwds);\n" << "Py_INCREF(errInfo.object());\n" - << "goto " << cpythonFunctionName(func) << "_TypeError;\n" + << "return " << returnErrorWrongArguments(overloadData, errorReturn) << ";\n" << outdent << "}\n"; } return; @@ -3817,14 +3827,14 @@ void CppGenerator::writeNamedArgumentResolution(TextStream &s, const AbstractMet << "if (value && " << pyArgName << ") {\n" << indent << "errInfo.reset(" << pyKeyName << ");\n" << "Py_INCREF(errInfo.object());\n" - << "goto " << cpythonFunctionName(func) << "_TypeError;\n" + << "return " << returnErrorWrongArguments(overloadData, errorReturn) << ";\n" << outdent << "}\nif (value) {\n" << indent << pyArgName << " = value;\nif (!"; const auto &type = arg.modifiedType(); writeTypeCheck(s, type, pyArgName, isNumber(type.typeEntry()), {}); s << ")\n" << indent - << "goto " << cpythonFunctionName(func) << "_TypeError;\n" << outdent - << outdent + << "return " << returnErrorWrongArguments(overloadData, errorReturn) << ";\n" + << outdent << outdent << "}\nPyDict_DelItem(kwds_dup, " << pyKeyName << ");\n" << outdent << "}\n"; } @@ -3835,7 +3845,7 @@ void CppGenerator::writeNamedArgumentResolution(TextStream &s, const AbstractMet s << "if (PyDict_Size(kwds_dup) > 0) {\n" << indent << "errInfo.reset(kwds_dup.release());\n"; if (!(func->isConstructor() && isQObject(func->ownerClass()))) - s << "goto " << cpythonFunctionName(func) << "_TypeError;\n"; + s << "return " << returnErrorWrongArguments(overloadData, errorReturn) << ";\n"; else s << "// fall through to handle extra keyword signals and properties\n"; s << outdent << "}\n" @@ -5307,7 +5317,6 @@ void CppGenerator::writeRichCompareFunction(TextStream &s, s << "switch (op) {\n" << indent; const QList<AbstractMetaFunctionCList> &groupedFuncs = filterGroupedOperatorFunctions(metaClass, OperatorQueryOption::ComparisonOp); - bool needErrorLabel = false; for (const AbstractMetaFunctionCList &overloads : groupedFuncs) { const auto rfunc = overloads[0]; @@ -5353,7 +5362,6 @@ void CppGenerator::writeRichCompareFunction(TextStream &s, TypeSystem::TargetLangCode, func, false /* uses PyArgs */, &func->arguments().constLast()); generateOperatorCode = false; - needErrorLabel |= std::any_of(snips.cbegin(), snips.cend(), containsGoto); } } if (generateOperatorCode) { @@ -5387,8 +5395,8 @@ void CppGenerator::writeRichCompareFunction(TextStream &s, << (op == AbstractMetaFunction::OperatorEqual ? "Py_False" : "Py_True") << ";\n" << "Py_INCREF(" << PYTHON_RETURN_VAR << ");\n" << outdent; } else { - s << indent << "goto " << baseName << "_RichComparison_TypeError;\n" << outdent; - needErrorLabel = true; + s << indent << "return Shiboken::returnFromRichCompare(" + << PYTHON_RETURN_VAR << ");\n" << outdent; } s << "}\n\n"; @@ -5397,21 +5405,9 @@ void CppGenerator::writeRichCompareFunction(TextStream &s, s << "default:\n" << indent << richCompareComment << "return FallbackRichCompare(self, " << PYTHON_ARG << ", op);\n" - << outdent << outdent << "}\n\n"; - - writeRichCompareFunctionFooter(s, baseName, needErrorLabel); -} - -void CppGenerator::writeRichCompareFunctionFooter(TextStream &s, - const QString &baseName, - bool writeErrorLabel) -{ - s << "if (" << PYTHON_RETURN_VAR << " && !PyErr_Occurred())\n" << indent - << "return " << PYTHON_RETURN_VAR << ";\n" << outdent; - if (writeErrorLabel) - s << baseName << "_RichComparison_TypeError:\n"; - s << "Shiboken::Errors::setOperatorNotImplemented();\n" - << ErrorReturn::Default << '\n' << outdent << "}\n\n"; + << outdent << outdent << "}\n\n" + << "return Shiboken::returnFromRichCompare(" << PYTHON_RETURN_VAR << ");\n" << outdent + << "}\n\n"; } using ComparisonOperatorList = QList<AbstractMetaFunction::ComparisonOperatorType>; @@ -5515,13 +5511,10 @@ void CppGenerator::writeSmartPointerRichCompareFunction(TextStream &s, } s << "}\n" << PYTHON_RETURN_VAR << " = " << CPP_RETURN_VAR << " ? Py_True : Py_False;\n" - << "Py_INCREF(" << PYTHON_RETURN_VAR << ");\n"; - - s << outdent << "} else {\n" << indent - << "goto " << baseName << "_RichComparison_TypeError;\n" - << outdent << "}\n"; - - writeRichCompareFunctionFooter(s, baseName, true); + << "Py_INCREF(" << PYTHON_RETURN_VAR << ");\n" + << outdent << "}\n" + << "return Shiboken::returnFromRichCompare(" << PYTHON_RETURN_VAR << ");\n" + << outdent << "}\n\n"; } // Return a flag combination for PyMethodDef diff --git a/sources/shiboken6/generator/shiboken/cppgenerator.h b/sources/shiboken6/generator/shiboken/cppgenerator.h index e2eace487..9f07529d9 100644 --- a/sources/shiboken6/generator/shiboken/cppgenerator.h +++ b/sources/shiboken6/generator/shiboken/cppgenerator.h @@ -168,6 +168,10 @@ private: static void writeErrorSection(TextStream &s, const OverloadData &overloadData, ErrorReturn errorReturn); + + static QString returnErrorWrongArguments(const OverloadData &overloadData, + ErrorReturn errorReturn); + static void writeFunctionReturnErrorCheckSection(TextStream &s, ErrorReturn errorReturn, bool hasReturnValue = true); @@ -272,7 +276,8 @@ private: * \param s text stream to write * \param overloadData the overload data describing all the possible overloads for the function/method */ - void writeOverloadedFunctionDecisor(TextStream &s, const OverloadData &overloadData) const; + void writeOverloadedFunctionDecisor(TextStream &s, const OverloadData &overloadData, + ErrorReturn errorReturn) const; /// Recursive auxiliar method to the other writeOverloadedFunctionDecisor. void writeOverloadedFunctionDecisorEngine(TextStream &s, const OverloadData &overloadData, @@ -355,7 +360,8 @@ private: const QString &isConvertibleFunc); void writeNamedArgumentResolution(TextStream &s, const AbstractMetaFunctionCPtr &func, - bool usePyArgs, const OverloadData &overloadData) const; + bool usePyArgs, const OverloadData &overloadData, + ErrorReturn errorReturn) const; /// Returns a string containing the name of an argument for the given function and argument index. static QString argumentNameFromIndex(const ApiExtractorResult &api, @@ -439,9 +445,6 @@ private: void writeRichCompareFunctionHeader(TextStream &s, const QString &baseName, const GeneratorContext &context) const; - static void writeRichCompareFunctionFooter(TextStream &s, - const QString &baseName, - bool writeLabel); void writeRichCompareFunction(TextStream &s, const GeneratorContext &context) const; void writeSmartPointerRichCompareFunction(TextStream &s, const GeneratorContext &context) const; diff --git a/sources/shiboken6/libshiboken/basewrapper.cpp b/sources/shiboken6/libshiboken/basewrapper.cpp index 3dd5d3539..4a5b54d23 100644 --- a/sources/shiboken6/libshiboken/basewrapper.cpp +++ b/sources/shiboken6/libshiboken/basewrapper.cpp @@ -7,6 +7,7 @@ #include "helper.h" #include "sbkconverter.h" #include "sbkenum.h" +#include "sbkerrors.h" #include "sbkfeature_base.h" #include "sbkstring.h" #include "sbkstaticstrings.h" @@ -754,6 +755,32 @@ void setErrorAboutWrongArguments(PyObject *args, const char *funcName, PyObject SetError_Argument(args, funcName, info); } +PyObject *returnWrongArguments(PyObject *args, const char *funcName, PyObject *info) +{ + setErrorAboutWrongArguments(args, funcName, info); + return {}; +} + +int returnWrongArguments_Zero(PyObject *args, const char *funcName, PyObject *info) +{ + setErrorAboutWrongArguments(args, funcName, info); + return 0; +} + +int returnWrongArguments_MinusOne(PyObject *args, const char *funcName, PyObject *info) +{ + setErrorAboutWrongArguments(args, funcName, info); + return -1; +} + +PyObject *returnFromRichCompare(PyObject *result) +{ + if (result && !PyErr_Occurred()) + return result; + Shiboken::Errors::setOperatorNotImplemented(); + return {}; +} + PyObject *checkInvalidArgumentCount(Py_ssize_t numArgs, Py_ssize_t minArgs, Py_ssize_t maxArgs) { PyObject *result = nullptr; diff --git a/sources/shiboken6/libshiboken/basewrapper.h b/sources/shiboken6/libshiboken/basewrapper.h index 86806c917..1d1127b8e 100644 --- a/sources/shiboken6/libshiboken/basewrapper.h +++ b/sources/shiboken6/libshiboken/basewrapper.h @@ -130,11 +130,28 @@ void callCppDestructor(void *cptr) delete reinterpret_cast<T *>(cptr); } -// setErrorAboutWrongArguments now gets overload information from the signature module. -// The extra info argument can contain additional data about the error. +/// setErrorAboutWrongArguments now gets overload information from the signature module. +/// The extra info argument can contain additional data about the error. LIBSHIBOKEN_API void setErrorAboutWrongArguments(PyObject *args, const char *funcName, PyObject *info); +/// Return values for the different retun variants. +/// This is used instead of goto. +LIBSHIBOKEN_API PyObject *returnWrongArguments(PyObject *args, const char *funcName, + PyObject *info); + +LIBSHIBOKEN_API int returnWrongArguments_Zero(PyObject *args, const char *funcName, + PyObject *info); + +LIBSHIBOKEN_API int returnWrongArguments_MinusOne(PyObject *args, const char *funcName, + PyObject *info); + +LIBSHIBOKEN_API void returnWrongArguments_Void(PyObject *args, const char *funcName, + PyObject *info); + +/// A simple special version for the end of rich comparison. +LIBSHIBOKEN_API PyObject *returnFromRichCompare(PyObject *result); + // Return error information object if the argument count is wrong LIBSHIBOKEN_API PyObject *checkInvalidArgumentCount(Py_ssize_t numArgs, Py_ssize_t minArgs, |