Python fastdispatch error message improvements

When overloaded C++ methods are called, the fastdispatch mode sometimes
optimises out the generation of the typecheck typemap code, resulting in
the 'Possible C/C++ prototypes' information not being in the error message.
Now when a TypeError is thrown, the exception message is modified to contain the
'Possible C/C++ prototypes'.

See issue #1293

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

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

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

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");
 
-
+    // The language module may want to generate different code for last overloaded function called (with same number of arguments)
-    String *lfmt = ReplaceFormat(fmt, num_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_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);
   }

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