diff --git a/CHANGES.current b/CHANGES.current index 64a2e65fd..04ea95c3d 100644 --- a/CHANGES.current +++ b/CHANGES.current @@ -7,6 +7,11 @@ the issue number to the end of the URL: https://github.com/swig/swig/issues/ Version 4.0.0 (in progress) =========================== +2018-08-10: wsfulton + [Python] #1293 Improve TypeError message inconsistencies between default and fastdispatch + mode when handling overloaded C++ functions. Previously the error message did not always + display the possible C/C++ prototypes in fastdispatch mode. + 2018-07-31: wsfulton [Python] #1293 Overloaded C++ function wrappers now raise a TypeError instead of NotImplementedError when the types passed are incorrect. This change means diff --git a/Examples/test-suite/python/director_exception_runme.py b/Examples/test-suite/python/director_exception_runme.py index 892c7e653..aa9d490a3 100644 --- a/Examples/test-suite/python/director_exception_runme.py +++ b/Examples/test-suite/python/director_exception_runme.py @@ -25,6 +25,13 @@ class MyFoo3(Foo): def ping(self): raise MyException("foo", "bar") +class MyFoo4(Foo): + + def ping(self, *args): + print(type("bad", "call")) # throws TypeError message: type() takes 1 or 3 arguments + return "Foo4.ping" + + # Check that the NotImplementedError raised by MyFoo.ping() is returned by # MyFoo.pong(). ok = 0 @@ -51,7 +58,8 @@ b = launder(a) try: b.pong() except TypeError, e: - if str(e) == "SWIG director type mismatch in output value of type 'std::string'": + # fastdispatch mode adds on Additional Information to the exception message - just check the main exception message exists + if str(e).startswith("SWIG director type mismatch in output value of type 'std::string'"): ok = 1 else: print "Unexpected error message: %s" % str(e) @@ -74,6 +82,22 @@ except MyException, e: if not ok: raise RuntimeError + +# Check that the director returns the appropriate TypeError thrown in a director method +ok = 0 +a = MyFoo4() +b = launder(a) +try: + b.pong() +except TypeError as e: + if str(e).startswith("type() takes 1 or 3 arguments"): + ok = 1 + else: + print "Unexpected error message: %s" % str(e) +if not ok: + raise RuntimeError + + # This is expected to fail with -builtin option # Throwing builtin classes as exceptions not supported if not is_python_builtin(): diff --git a/Lib/python/pyerrors.swg b/Lib/python/pyerrors.swg index 463afae15..85c7d86db 100644 --- a/Lib/python/pyerrors.swg +++ b/Lib/python/pyerrors.swg @@ -71,3 +71,33 @@ SWIG_Python_AddErrorMsg(const char* mesg) PyErr_SetString(PyExc_RuntimeError, mesg); } } + +SWIGRUNTIME int +SWIG_Python_TypeErrorOccurred(PyObject *obj) +{ + PyObject *error; + if (obj) + return 0; + error = PyErr_Occurred(); + return error && PyErr_GivenExceptionMatches(error, PyExc_TypeError); +} + +SWIGRUNTIME void +SWIG_Python_RaiseOrModifyTypeError(const char *message) +{ + if (SWIG_Python_TypeErrorOccurred(NULL)) { + /* Use existing TypeError to preserve stacktrace and enhance with given message */ + PyObject *type = NULL, *value = NULL, *traceback = NULL; + PyErr_Fetch(&type, &value, &traceback); +#if PY_VERSION_HEX >= 0x03000000 + PyObject *newvalue = PyUnicode_FromFormat("%S\nAdditional Information:\n%s", value, message); +#else + PyObject *newvalue = PyString_FromFormat("%s\nAdditional Information:\n%s", PyString_AsString(value), message); +#endif + Py_XDECREF(value); + PyErr_Restore(type, newvalue, traceback); + } else { + /* Raise TypeError using given message */ + PyErr_SetString(PyExc_TypeError, message); + } +} diff --git a/Source/Modules/overload.cxx b/Source/Modules/overload.cxx index 3945d598f..5d278107c 100644 --- a/Source/Modules/overload.cxx +++ b/Source/Modules/overload.cxx @@ -442,7 +442,7 @@ String *Swig_overload_dispatch_cast(Node *n, const_String_or_char_ptr fmt, int * /* Loop over the functions */ - bool emitcheck = 1; + bool emitcheck = true; for (i = 0; i < nfunc; i++) { int fn = 0; Node *ni = Getitem(dispatch, i); @@ -518,7 +518,7 @@ String *Swig_overload_dispatch_cast(Node *n, const_String_or_char_ptr fmt, int * if (tml) Replaceid(tml, Getattr(pl, "lname"), "_v"); if (!tml || Cmp(tm, tml)) - emitcheck = 1; + emitcheck = true; //printf("tmap: %s[%d] (%d) => %s\n\n", // Char(Getattr(nk, "sym:name")), // l, emitcheck, tml?Char(tml):0); @@ -610,7 +610,7 @@ String *Swig_overload_dispatch_cast(Node *n, const_String_or_char_ptr fmt, int * /* Fast dispatch mechanism, provided by Salvador Fandi~no Garc'ia (#930586). */ -String *Swig_overload_dispatch_fast(Node *n, const_String_or_char_ptr fmt, int *maxargs) { +static String *overload_dispatch_fast(Node *n, const_String_or_char_ptr fmt, int *maxargs, const_String_or_char_ptr fmt_fastdispatch) { int i, j; *maxargs = 1; @@ -653,6 +653,7 @@ String *Swig_overload_dispatch_fast(Node *n, const_String_or_char_ptr fmt, int * // printf("overload: %s coll=%d\n", Char(Getattr(n, "sym:name")), Len(coll)); + bool emitcheck = false; int num_braces = 0; bool test = (Len(coll) > 0 && num_arguments); if (test) { @@ -673,7 +674,7 @@ String *Swig_overload_dispatch_fast(Node *n, const_String_or_char_ptr fmt, int * /* if all the wrappers have the same type check on this argument we can optimize it out */ - bool emitcheck = 0; + emitcheck = false; for (int k = 0; k < Len(coll) && !emitcheck; k++) { Node *nk = Getitem(coll, k); Parm *pk = Getattr(nk, "wrap:parms"); @@ -695,7 +696,7 @@ String *Swig_overload_dispatch_fast(Node *n, const_String_or_char_ptr fmt, int * if (tml) Replaceid(tml, Getattr(pl, "lname"), "_v"); if (!tml || Cmp(tm, tml)) - emitcheck = 1; + emitcheck = true; //printf("tmap: %s[%d] (%d) => %s\n\n", // Char(Getattr(nk, "sym:name")), // l, emitcheck, tml?Char(tml):0); @@ -752,8 +753,8 @@ String *Swig_overload_dispatch_fast(Node *n, const_String_or_char_ptr fmt, int * for ( /* empty */ ; num_braces > 0; num_braces--) Printf(f, "}\n"); - - String *lfmt = ReplaceFormat(fmt, num_arguments); + // The language module may want to generate different code for last overloaded function called (with same number of arguments) + String *lfmt = ReplaceFormat(!emitcheck && fmt_fastdispatch ? fmt_fastdispatch : fmt, num_arguments); Printf(f, Char(lfmt), Getattr(ni, "wrap:name")); Printf(f, "}\n"); /* braces closes "if" for this method */ @@ -770,10 +771,10 @@ String *Swig_overload_dispatch_fast(Node *n, const_String_or_char_ptr fmt, int * return f; } -String *Swig_overload_dispatch(Node *n, const_String_or_char_ptr fmt, int *maxargs) { +String *Swig_overload_dispatch(Node *n, const_String_or_char_ptr fmt, int *maxargs, const_String_or_char_ptr fmt_fastdispatch) { if (fast_dispatch_mode || GetFlag(n, "feature:fastdispatch")) { - return Swig_overload_dispatch_fast(n, fmt, maxargs); + return overload_dispatch_fast(n, fmt, maxargs, fmt_fastdispatch); } int i, j; diff --git a/Source/Modules/python.cxx b/Source/Modules/python.cxx index f7ec22a51..8e234d8ad 100755 --- a/Source/Modules/python.cxx +++ b/Source/Modules/python.cxx @@ -2578,13 +2578,16 @@ public: String *tmp = NewString(""); String *dispatch; - const char *dispatch_code = funpack ? "return %s(self, argc, argv);" : - (builtin_ctor ? "return %s(self, args, NULL);" : "return %s(self, args);"); + + const char *dispatch_call = funpack ? "%s(self, argc, argv);" : (builtin_ctor ? "%s(self, args, NULL);" : "%s(self, args);"); + String *dispatch_code = NewStringf("return %s", dispatch_call); if (castmode) { dispatch = Swig_overload_dispatch_cast(n, dispatch_code, &maxargs); } else { - dispatch = Swig_overload_dispatch(n, dispatch_code, &maxargs); + String *fastdispatch_code = NewStringf("PyObject *retobj = %s\nif (!SWIG_Python_TypeErrorOccurred(retobj)) return retobj;\nSWIG_fail;", dispatch_call); + dispatch = Swig_overload_dispatch(n, dispatch_code, &maxargs, fastdispatch_code); + Delete(fastdispatch_code); } /* Generate a dispatch wrapper for all overloaded functions */ @@ -2645,7 +2648,7 @@ public: Delete(fulldecl); } while ((sibl = Getattr(sibl, "sym:nextSibling"))); Append(f->code, "fail:\n"); - Printf(f->code, " SWIG_SetErrorMsg(PyExc_TypeError," + Printf(f->code, " SWIG_Python_RaiseOrModifyTypeError(" "\"Wrong number or type of arguments for overloaded function '%s'.\\n\"" "\n\" Possible C/C++ prototypes are:\\n\"%s);\n", symname, protoTypes); Printf(f->code, "return %s;\n", builtin_ctor ? "-1" : "0"); Delete(protoTypes); @@ -2662,6 +2665,7 @@ public: } DelWrapper(f); Delete(dispatch); + Delete(dispatch_code); Delete(tmp); Delete(wname); } diff --git a/Source/Modules/swigmod.h b/Source/Modules/swigmod.h index 1e49993bf..9626166a6 100644 --- a/Source/Modules/swigmod.h +++ b/Source/Modules/swigmod.h @@ -375,9 +375,8 @@ void emit_mark_varargs(ParmList *l); String *emit_action(Node *n); int emit_action_code(Node *n, String *wrappercode, String *action); void Swig_overload_check(Node *n); -String *Swig_overload_dispatch(Node *n, const_String_or_char_ptr fmt, int *); +String *Swig_overload_dispatch(Node *n, const_String_or_char_ptr fmt, int *, const_String_or_char_ptr fmt_fastdispatch = 0); String *Swig_overload_dispatch_cast(Node *n, const_String_or_char_ptr fmt, int *); -String *Swig_overload_dispatch_fast(Node *n, const_String_or_char_ptr fmt, int *); List *Swig_overload_rank(Node *n, bool script_lang_wrapping); SwigType *cplus_value_type(SwigType *t);