Merge branch 'python-type-error'

* python-type-error:
  Python fastdispatch error message improvements
  Correct PyErr_Fetch/PyErr_Restore variable names
  Overloaded C++ function Python wrappers now raise a TypeError instead of NotImplementedError

CHANGES.current

@@ -7,9 +7,35 @@ 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-08-02: furylynx,jacobwgillespie,p2k
             [Javascript] #1290, #968. Add support for NodeJS versions 2-10.
 
+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
+            there is now consistency with non-overloaded function wrappers which have always
+            raised TypeError when the incorrect types are passed. The error message remains
+            the same and is for example now:
+
+              TypeError: Wrong number or type of arguments for overloaded function 'f'.
+                Possible C/C++ prototypes are:
+                  f(int)
+                  f(char const *)
+
+            instead of:
+
+              NotImplementedError: Wrong number or type of arguments for overloaded function 'f'.
+                Possible C/C++ prototypes are:
+                  f(int)
+                  f(char const *)
+
+            *** POTENTIAL INCOMPATIBILITY ***
+
 2018-06-23: wsfulton
             [Python] #718 Fix pythonnondynamic feature for modern classes
 
@@ -686,6 +712,7 @@ Version 4.0.0 (in progress)
 2017-05-26: Volker Diels-Grabsch, vadz
             [Java] #842 Extend from java.util.AbstractList<> and implement java.util.RandomAccess for
             std::vector wrappers. This notably allows to iterate over wrapped vectors in a natural way.
+
 2017-05-30: davidcl
             [Scilab] #994 Undefined symbol error when loading in Scilab 6
 

Doc/Manual/Python.html

@@ -4894,7 +4894,7 @@ If you don't you'll get an error message along the lines of:
 Traceback (most recent call last):
   File "runme.py", line 3, in >module<
     example.foo(["foo", "bar", "spam", "1"])
-NotImplementedError: Wrong number or type of arguments for overloaded function 'foo'.
+TypeError: Wrong number or type of arguments for overloaded function 'foo'.
   Possible C/C++ prototypes are:
     foo(int, char **)
     foo()

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():

Examples/test-suite/python/li_std_string_extra_runme.py

@@ -95,14 +95,18 @@ if li_std_string_extra.test_value_basic_overload(123) != "int":
 
 try:
     li_std_string_extra.test_value_basic_overload([x])
-    raise RuntimeError, "should throw NotImplementedError"
-except NotImplementedError:
+    raise RuntimeError, "should throw TypeError"
+except TypeError as e:
+    if str(e).find("Possible C/C++ prototypes are:") == -1:
+        raise RuntimeError("Incorrect error message text:\n{}".format(e))
     pass
 
 try:
     li_std_string_extra.test_value_basic_overload([123])
-    raise RuntimeError, "should throw NotImplementedError"
-except NotImplementedError:
+    raise RuntimeError, "should throw TypeError"
+except TypeError as e:
+    if str(e).find("Possible C/C++ prototypes are:") == -1:
+        raise RuntimeError("Incorrect error message text:\n{}".format(e))
     pass
 
 # Global variables

Examples/test-suite/python/primitive_types_runme.py

@@ -549,14 +549,9 @@ def checkOverload(t, name, val, delta, prevval, limit):
             if t.ovr_str(val + delta) == name:
                 raise RuntimeError, "bad " + name + " typemap"
             if val == limit:
-                # Should raise NotImplementedError here since this is the largest integral type
-                raise RuntimeError, "bad " + name + " typemap"
-        except NotImplementedError:
-            # NotImplementedError is expected only if this is the most extreme type
-            if val != limit:
+                # Should raise TypeError here since this is the largest integral type
                 raise RuntimeError, "bad " + name + " typemap"
         except TypeError:
-            # TypeError is raised instead if swig is run with -O or -fastdispatch
             if val != limit:
                 raise RuntimeError, "bad " + name + " typemap"
 

Examples/test-suite/python/varargs_runme.py

@@ -30,7 +30,5 @@ if varargs.test_plenty("Hello", 1, 2) != "Hello":
 try:
     varargs.test_plenty("Hello", 1, 2, 3)
     raise RuntimeError
-except NotImplementedError:
-    pass
 except TypeError:
     pass

Lib/python/builtin.swg

@@ -436,15 +436,15 @@ SwigPyBuiltin_destructor_closure(SwigPyWrapperFunction wrapper, const char *wrap
   Py_XDECREF(sobj->dict);
   if (sobj->own) {
     PyObject *o;
-    PyObject *val = 0, *type = 0, *tb = 0;
-    PyErr_Fetch(&val, &type, &tb);
+    PyObject *type = 0, *value = 0, *traceback = 0;
+    PyErr_Fetch(&type, &value, &traceback);
     o = wrapper(a, NULL);
     if (!o) {
       PyObject *deallocname = PyString_FromString(wrappername);
       PyErr_WriteUnraisable(deallocname);
       Py_DECREF(deallocname);
     }
-    PyErr_Restore(val, type, tb);
+    PyErr_Restore(type, value, traceback);
     Py_XDECREF(o);
   }
   if (PyType_IS_GC(a->ob_type)) {

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);
+  }
+}

Lib/python/pyrun.swg

@@ -530,8 +530,8 @@ SwigPyObject_dealloc(PyObject *v)
          remain true upon return from SwigPyObject_dealloc.  So save
          and restore. */
       
-      PyObject *val = NULL, *type = NULL, *tb = NULL;
-      PyErr_Fetch(&val, &type, &tb);
+      PyObject *type = NULL, *value = NULL, *traceback = NULL;
+      PyErr_Fetch(&type, &value, &traceback);
 
       if (data->delargs) {
         /* we need to create a temporary object to carry the destroy operation */
@@ -546,7 +546,7 @@ SwigPyObject_dealloc(PyObject *v)
       if (!res)
         PyErr_WriteUnraisable(destroy);
 
-      PyErr_Restore(val, type, tb);
+      PyErr_Restore(type, value, traceback);
 
       Py_XDECREF(res);
     } 

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;

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_NotImplementedError,"
+      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);
   }

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);