From a0543241df2273ad60a4c92e4ffe6e0cfb1042b9 Mon Sep 17 00:00:00 2001 From: Christian Tismer Date: Sat, 14 Jul 2018 15:10:56 +0200 Subject: Produce TypeError Messages Using the Signature Module The TypeError messages can now be produced, based upon the signature module. As a feature under test, we produce ValueErrors instead in certain cases. This will probably improve, later. We are currently investigating how much can be determined, automatically. Task-number: PYSIDE-795 Change-Id: Ie8a648beaf8a3bed388e3c01ba501bb36859722e Reviewed-by: Friedemann Kleint Reviewed-by: Cristian Maureira-Fredes --- .../pyside2/PySide2/support/signature/mapping.py | 1 + .../shiboken2/generator/shiboken2/cppgenerator.cpp | 89 +-------------- sources/shiboken2/libshiboken/basewrapper.cpp | 40 +------ sources/shiboken2/libshiboken/basewrapper.h | 4 +- sources/shiboken2/libshiboken/signature.cpp | 29 +++++ sources/shiboken2/libshiboken/signature.h | 1 + sources/shiboken2/shibokenmodule/CMakeLists.txt | 2 + .../support/signature/errorhandler.py | 124 +++++++++++++++++++++ .../shibokenmodule/support/signature/loader.py | 2 + .../shibokenmodule/support/signature/mapping.py | 1 + .../shibokenmodule/support/signature/parser.py | 3 +- .../shiboken2/tests/samplebinding/decisor_test.py | 6 +- 12 files changed, 174 insertions(+), 128 deletions(-) create mode 100644 sources/shiboken2/shibokenmodule/support/signature/errorhandler.py diff --git a/sources/pyside2/PySide2/support/signature/mapping.py b/sources/pyside2/PySide2/support/signature/mapping.py index 61fa2d41f..96afd3c7a 100644 --- a/sources/pyside2/PySide2/support/signature/mapping.py +++ b/sources/pyside2/PySide2/support/signature/mapping.py @@ -63,6 +63,7 @@ class Reloader(Sbk_Reloader): Sbk_Reloader.update(self, globals()) update_mapping = Reloader().update +namespace = globals() # our module's __dict__, updated def init_QtCore(): diff --git a/sources/shiboken2/generator/shiboken2/cppgenerator.cpp b/sources/shiboken2/generator/shiboken2/cppgenerator.cpp index 99bfae9f0..a62a0ac27 100644 --- a/sources/shiboken2/generator/shiboken2/cppgenerator.cpp +++ b/sources/shiboken2/generator/shiboken2/cppgenerator.cpp @@ -2083,93 +2083,8 @@ void CppGenerator::writeErrorSection(QTextStream& s, OverloadData& overloadData) QString argsVar = pythonFunctionWrapperUsesListOfArguments(overloadData) ? QLatin1String("args") : QLatin1String(PYTHON_ARG); - if (verboseErrorMessagesDisabled()) { - s << INDENT << "Shiboken::setErrorAboutWrongArguments(" << argsVar << ", \"" << funcName << "\", 0);" << endl; - } else { - QStringList overloadSignatures; - const OverloadData::MetaFunctionList &overloads = overloadData.overloads(); - for (const AbstractMetaFunction *f : overloads) { - QStringList args; - const AbstractMetaArgumentList &arguments = f->arguments(); - for (AbstractMetaArgument *arg : arguments) { - QString strArg; - AbstractMetaType* argType = arg->type(); - if (isCString(argType)) { - strArg = QLatin1String("\" SBK_BYTES_NAME \""); - } else if (argType->isPrimitive()) { - const PrimitiveTypeEntry* ptp = reinterpret_cast(argType->typeEntry()); - while (ptp->referencedTypeEntry()) - ptp = ptp->referencedTypeEntry(); - strArg = ptp->name(); - if (strArg == QLatin1String("QString")) { - strArg = QLatin1String("unicode"); - } else if (strArg == QLatin1String("QChar")) { - strArg = QLatin1String("1-unicode"); - } else { - strArg = ptp->name(); - static const QRegularExpression regex(QStringLiteral("^signed\\s+")); - Q_ASSERT(regex.isValid()); - strArg.remove(regex); - if (strArg == QLatin1String("double")) - strArg = QLatin1String("float"); - } - } else if (argType->typeEntry()->isContainer()) { - strArg = argType->fullName(); - if (strArg == QLatin1String("QList") || strArg == QLatin1String("QVector") - || strArg == QLatin1String("QLinkedList") || strArg == QLatin1String("QStack") - || strArg == QLatin1String("QQueue")) { - strArg = QLatin1String("list"); - } else if (strArg == QLatin1String("QMap") || strArg == QLatin1String("QHash") - || strArg == QLatin1String("QMultiMap") || strArg == QLatin1String("QMultiHash")) { - strArg = QLatin1String("dict"); - } else if (strArg == QLatin1String("QPair")) { - strArg = QLatin1String("2-tuple"); - } - } else { - strArg = argType->fullName(); - if (strArg == QLatin1String("PyUnicode")) - strArg = QLatin1String("unicode"); - else if (strArg == QLatin1String("PyString")) - strArg = QLatin1String("str"); - else if (strArg == QLatin1String("PyBytes")) - strArg = QLatin1String("\" SBK_BYTES_NAME \""); - else if (strArg == QLatin1String("PyByteArray")) - strArg = QLatin1String("bytearray"); - else if (strArg == QLatin1String("PySequence")) - strArg = QLatin1String("list"); - else if (strArg == QLatin1String("PyTuple")) - strArg = QLatin1String("tuple"); - else if (strArg == QLatin1String("PyDict")) - strArg = QLatin1String("dict"); - else if (strArg == QLatin1String("PyObject")) - strArg = QLatin1String("object"); - else if (strArg == QLatin1String("PyCallable")) - strArg = QLatin1String("callable"); - else if (strArg == QLatin1String("uchar")) - strArg = QLatin1String("buffer"); // This depends on an inject code to be true, but if it's not true - // the function wont work at all, so it must be true. - } - if (!arg->defaultValueExpression().isEmpty()) { - strArg += QLatin1String(" = "); - if ((isCString(argType) || isPointerToWrapperType(argType)) - && arg->defaultValueExpression() == QLatin1String("0")) { - strArg += QLatin1String("None"); - } else { - QString e = arg->defaultValueExpression(); - e.replace(QLatin1String("::"), QLatin1String(".")); - e.replace(QLatin1String("\""), QLatin1String("\\\"")); - strArg += e; - } - } - args << strArg; - } - overloadSignatures << QLatin1Char('"') + args.join(QLatin1String(", ")) + QLatin1Char('"'); - } - s << INDENT << "const char* overloads[] = {" << overloadSignatures.join(QLatin1String(", ")) - << ", 0};" << endl; - s << INDENT << "Shiboken::setErrorAboutWrongArguments(" << argsVar << ", \"" << funcName << "\", overloads);" << endl; - } - s << INDENT << returnStatement(m_currentErrorCode) << endl; + s << INDENT << "Shiboken::setErrorAboutWrongArguments(" << argsVar << ", \"" << funcName << "\");" << endl; + s << INDENT << "return " << m_currentErrorCode << ';' << endl; } void CppGenerator::writeFunctionReturnErrorCheckSection(QTextStream& s, bool hasReturnValue) diff --git a/sources/shiboken2/libshiboken/basewrapper.cpp b/sources/shiboken2/libshiboken/basewrapper.cpp index e820e749b..3e1a7e8e5 100644 --- a/sources/shiboken2/libshiboken/basewrapper.cpp +++ b/sources/shiboken2/libshiboken/basewrapper.cpp @@ -596,42 +596,10 @@ void init() shibokenAlreadInitialised = true; } -void setErrorAboutWrongArguments(PyObject* args, const char* funcName, const char** cppOverloads) -{ - std::string msg; - std::string params; - if (args) { - if (PyTuple_Check(args)) { - for (Py_ssize_t i = 0, max = PyTuple_GET_SIZE(args); i < max; ++i) { - if (i) - params += ", "; - PyObject* arg = PyTuple_GET_ITEM(args, i); - params += Py_TYPE(arg)->tp_name; - } - } else { - params = Py_TYPE(args)->tp_name; - } - } - - if (!cppOverloads) { - msg = "'" + std::string(funcName) + "' called with wrong argument types: " + params; - } else { - msg = "'" + std::string(funcName) + "' called with wrong argument types:\n "; - msg += funcName; - msg += '('; - msg += params; - msg += ")\n"; - msg += "Supported signatures:"; - for (int i = 0; cppOverloads[i]; ++i) { - msg += "\n "; - msg += funcName; - msg += '('; - msg += cppOverloads[i]; - msg += ')'; - } - } - PyErr_SetString(PyExc_TypeError, msg.c_str()); - +// setErrorAboutWrongArguments now gets overload info from the signature module. +void setErrorAboutWrongArguments(PyObject *args, const char *funcName) +{ + SetError_Argument(args, funcName); } class FindBaseTypeVisitor : public HierarchyVisitor diff --git a/sources/shiboken2/libshiboken/basewrapper.h b/sources/shiboken2/libshiboken/basewrapper.h index 15682c600..e1cc64ba9 100644 --- a/sources/shiboken2/libshiboken/basewrapper.h +++ b/sources/shiboken2/libshiboken/basewrapper.h @@ -141,7 +141,9 @@ void callCppDestructor(void* cptr) * Shiboken::importModule is DEPRECATED. Use Shiboken::Module::import() instead. */ SBK_DEPRECATED(LIBSHIBOKEN_API bool importModule(const char* moduleName, PyTypeObject*** cppApiPtr)); -LIBSHIBOKEN_API void setErrorAboutWrongArguments(PyObject* args, const char* funcName, const char** cppOverloads); + +// setErrorAboutWrongArguments now gets overload info from the signature module. +LIBSHIBOKEN_API void setErrorAboutWrongArguments(PyObject* args, const char* funcName); namespace ObjectType { diff --git a/sources/shiboken2/libshiboken/signature.cpp b/sources/shiboken2/libshiboken/signature.cpp index 564e5fcef..a8874e2e0 100644 --- a/sources/shiboken2/libshiboken/signature.cpp +++ b/sources/shiboken2/libshiboken/signature.cpp @@ -74,6 +74,7 @@ typedef struct safe_globals_struc { // init part 2: run module PyObject *sigparse_func; PyObject *createsig_func; + PyObject *seterror_argument_func; } safe_globals_struc, *safe_globals; static safe_globals pyside_globals = 0; @@ -510,6 +511,9 @@ init_phase_2(safe_globals_struc *p, PyMethodDef *methods) p->createsig_func = PyObject_GetAttrString(p->helper_module, "create_signature"); if (p->createsig_func == NULL) goto error; + p->seterror_argument_func = PyObject_GetAttrString(p->helper_module, "seterror_argument"); + if (p->seterror_argument_func == NULL) + goto error; return 0; error: @@ -950,4 +954,29 @@ FinishSignatureInitialization(PyObject *module, const char *signatures) } } +void +SetError_Argument(PyObject *args, const char *func_name) +{ + /* + * This function replaces the type error construction with extra + * overloads parameter in favor of using the signature module. + * Error messages are rare, so we do it completely in Python. + */ + init_module_1(); + init_module_2(); + Shiboken::AutoDecRef res(PyObject_CallFunction( + pyside_globals->seterror_argument_func, + const_cast("(Os)"), args, func_name)); + if (res.isNull()) { + PyErr_Print(); + Py_FatalError("seterror_argument did not receive a result"); + } + PyObject *err, *msg; + if (!PyArg_UnpackTuple(res, func_name, 2, 2, &err, &msg)) { + PyErr_Print(); + Py_FatalError("unexpected failure in seterror_argument"); + } + PyErr_SetObject(err, msg); +} + } //extern "C" diff --git a/sources/shiboken2/libshiboken/signature.h b/sources/shiboken2/libshiboken/signature.h index b65317662..c850698a6 100644 --- a/sources/shiboken2/libshiboken/signature.h +++ b/sources/shiboken2/libshiboken/signature.h @@ -47,6 +47,7 @@ extern "C" LIBSHIBOKEN_API int SbkSpecial_Type_Ready(PyObject *, PyTypeObject *, const char *); //WS LIBSHIBOKEN_API void FinishSignatureInitialization(PyObject *, const char *); +LIBSHIBOKEN_API void SetError_Argument(PyObject *, const char *); } // extern "C" diff --git a/sources/shiboken2/shibokenmodule/CMakeLists.txt b/sources/shiboken2/shibokenmodule/CMakeLists.txt index 0eba3eaff..373ce102f 100644 --- a/sources/shiboken2/shibokenmodule/CMakeLists.txt +++ b/sources/shiboken2/shibokenmodule/CMakeLists.txt @@ -57,6 +57,8 @@ configure_file("${CMAKE_CURRENT_SOURCE_DIR}/support/__init__.py" "${CMAKE_CURRENT_BINARY_DIR}/support/__init__.py" COPYONLY) configure_file("${CMAKE_CURRENT_SOURCE_DIR}/support/signature/__init__.py" "${CMAKE_CURRENT_BINARY_DIR}/support/signature/__init__.py" COPYONLY) +configure_file("${CMAKE_CURRENT_SOURCE_DIR}/support/signature/errorhandler.py" + "${CMAKE_CURRENT_BINARY_DIR}/support/signature/errorhandler.py" COPYONLY) configure_file("${CMAKE_CURRENT_SOURCE_DIR}/support/signature/layout.py" "${CMAKE_CURRENT_BINARY_DIR}/support/signature/layout.py" COPYONLY) configure_file("${CMAKE_CURRENT_SOURCE_DIR}/support/signature/loader.py" diff --git a/sources/shiboken2/shibokenmodule/support/signature/errorhandler.py b/sources/shiboken2/shibokenmodule/support/signature/errorhandler.py new file mode 100644 index 000000000..902bb05af --- /dev/null +++ b/sources/shiboken2/shibokenmodule/support/signature/errorhandler.py @@ -0,0 +1,124 @@ +############################################################################# +## +## Copyright (C) 2019 The Qt Company Ltd. +## Contact: https://www.qt.io/licensing/ +## +## This file is part of Qt for Python. +## +## $QT_BEGIN_LICENSE:LGPL$ +## Commercial License Usage +## Licensees holding valid commercial Qt licenses may use this file in +## accordance with the commercial license agreement provided with the +## Software or, alternatively, in accordance with the terms contained in +## a written agreement between you and The Qt Company. For licensing terms +## and conditions see https://www.qt.io/terms-conditions. For further +## information use the contact form at https://www.qt.io/contact-us. +## +## GNU Lesser General Public License Usage +## Alternatively, this file may be used under the terms of the GNU Lesser +## General Public License version 3 as published by the Free Software +## Foundation and appearing in the file LICENSE.LGPL3 included in the +## packaging of this file. Please review the following information to +## ensure the GNU Lesser General Public License version 3 requirements +## will be met: https://www.gnu.org/licenses/lgpl-3.0.html. +## +## GNU General Public License Usage +## Alternatively, this file may be used under the terms of the GNU +## General Public License version 2.0 or (at your option) the GNU General +## Public license version 3 or any later version approved by the KDE Free +## Qt Foundation. The licenses are as published by the Free Software +## Foundation and appearing in the file LICENSE.GPL2 and LICENSE.GPL3 +## included in the packaging of this file. Please review the following +## information to ensure the GNU General Public License requirements will +## be met: https://www.gnu.org/licenses/gpl-2.0.html and +## https://www.gnu.org/licenses/gpl-3.0.html. +## +## $QT_END_LICENSE$ +## +############################################################################# + +from __future__ import print_function, absolute_import + +""" +errorhandler.py + +This module handles the TypeError messages which were previously +produced by the generated C code. + +This version is at least consistent with the signatures, which +are created by the same module. + +Experimentally, we are trying to guess those errors which are +just the wrong number of elements in an iterator. +At the moment, it is unclear whether the information given is +enough to produce a useful ValueError. + +This matter will be improved in a later version. +""" + +from signature_loader import get_signature, inspect +from signature_loader.mapping import update_mapping, namespace +from textwrap import dedent + + +def qt_isinstance(inst, the_type): + if the_type == float: + return isinstance(inst, int) or isinstance(int, float) + try: + return isinstance(inst, the_type) + except TypeError as e: + print("FIXME", e) + return False + + +def matched_type(args, sigs): + for sig in sigs: + params = list(sig.parameters.values()) + if len(args) > len(params): + continue + if len(args) < len(params): + k = len(args) + if params[k].default is params[k].empty: + # this is a necessary parameter, so it fails. + continue + ok = True + for arg, param in zip(args, params): + ann = param.annotation + if qt_isinstance(arg, ann): + continue + ok = False + if ok: + return sig + return None + + +def seterror_argument(args, func_name): + update_mapping() + func = eval(func_name, namespace) + sigs = get_signature(func, "typeerror") + if type(sigs) != list: + sigs = [sigs] + if type(args) != tuple: + args = (args,) + # temp! + found = matched_type(args, sigs) + if found: + msg = dedent(""" + '{func_name}' called with wrong argument values: + {func_name}{args} + Found signature: + {func_name}{found} + """.format(**locals())).strip() + return ValueError, msg + type_str = ", ".join(type(arg).__name__ for arg in args) + msg = dedent(""" + '{func_name}' called with wrong argument types: + {func_name}({type_str}) + Supported signatures: + """.format(**locals())).strip() + for sig in sigs: + msg += "\n {func_name}{sig}".format(**locals()) + # We don't raise the error here, to avoid the loader in the traceback. + return TypeError, msg + +# end of file diff --git a/sources/shiboken2/shibokenmodule/support/signature/loader.py b/sources/shiboken2/shibokenmodule/support/signature/loader.py index de27d441c..be30483fe 100644 --- a/sources/shiboken2/shibokenmodule/support/signature/loader.py +++ b/sources/shiboken2/shibokenmodule/support/signature/loader.py @@ -181,6 +181,8 @@ with ensure_import_support(): mapping = sbk_mapping mapping.__name__ = "mapping" put_into_loader_package(mapping) + from support.signature import errorhandler + put_into_loader_package(errorhandler) from support.signature import layout put_into_loader_package(layout) from support.signature.lib import enum_sig diff --git a/sources/shiboken2/shibokenmodule/support/signature/mapping.py b/sources/shiboken2/shibokenmodule/support/signature/mapping.py index 0195f0280..40db43729 100644 --- a/sources/shiboken2/shibokenmodule/support/signature/mapping.py +++ b/sources/shiboken2/shibokenmodule/support/signature/mapping.py @@ -203,6 +203,7 @@ def check_module(mod): update_mapping = Reloader().update type_map = {} +namespace = globals() # our module's __dict__ def init_Shiboken(): diff --git a/sources/shiboken2/shibokenmodule/support/signature/parser.py b/sources/shiboken2/shibokenmodule/support/signature/parser.py index 5178d9ef9..2f6e6e3f5 100644 --- a/sources/shiboken2/shibokenmodule/support/signature/parser.py +++ b/sources/shiboken2/shibokenmodule/support/signature/parser.py @@ -45,8 +45,7 @@ import warnings import types import keyword import functools -from signature_loader.mapping import ( - type_map, update_mapping, __dict__ as namespace) +from signature_loader.mapping import type_map, update_mapping, namespace _DEBUG = False LIST_KEYWORDS = False diff --git a/sources/shiboken2/tests/samplebinding/decisor_test.py b/sources/shiboken2/tests/samplebinding/decisor_test.py index 9102e2d4e..734c43760 100644 --- a/sources/shiboken2/tests/samplebinding/decisor_test.py +++ b/sources/shiboken2/tests/samplebinding/decisor_test.py @@ -43,13 +43,15 @@ class DecisorTest(unittest.TestCase): This can trigger the bug #262, which means using an argument not provided by the user.''' pt = Point() - self.assertRaises(TypeError, SampleNamespace.forceDecisorSideA, pt) + # This exception may move from a TypeError to a ValueError. + self.assertRaises((TypeError, ValueError), SampleNamespace.forceDecisorSideA, pt) def testCallWithInvalidParametersSideB(self): '''Same as the previous test, but with an integer as first argument, just to complicate things for the overload method decisor.''' pt = Point() - self.assertRaises(TypeError, SampleNamespace.forceDecisorSideB, 1, pt) + # This exception may move from a TypeError to a ValueError. + self.assertRaises((TypeError, ValueError), SampleNamespace.forceDecisorSideB, 1, pt) def testDecideCallWithInheritance(self): '''Call methods overloads that receive parent and inheritor classes' instances.''' -- cgit v1.2.3