Correct the implementation of Python PEP 207.

Don't convert all exceptions into a NotImplemented return
when wrapping operators which are marked with %pythonmaybecall.

Closes #1783

CHANGES.current

@@ -7,6 +7,11 @@ the issue number to the end of the URL: https://github.com/swig/swig/issues/
 Version 4.2.0 (in progress)
 ===========================
 
+2023-10-21: wsfulton
+            [Python] #1783 Don't swallow all exceptions into a NotImplemented return
+            when wrapping operators which are marked with %pythonmaybecall. Corrects the
+            implementation of PEP 207.
+
 2023-10-18: wsfulton
             [C#, D] #902 Use the C++11 enum base, that is, the underlying enum
             type.

Doc/Manual/Python.html

@@ -1983,8 +1983,8 @@ overloaded assignment operators don't map to Python semantics and will be ignore
 <p>
 Operator overloading is implemented in the <tt>pyopers.swg</tt> library file.
 In particular overloaded operators are marked with the <tt>python:maybecall</tt> feature, also known as <tt>%pythonmaybecall</tt>.
-This feature forces SWIG to generate code that return an instance of Python's <tt>NotImplemented</tt>
-instead of raising an exception when the comparison fails, that is, on any kind of error.
+This feature forces SWIG to generate code that returns an instance of Python's <tt>NotImplemented</tt>
+instead of raising the usual <tt>TypeError</tt> exception when an incorrect type is passed to a SWIG wrapped method.
 This follows the guidelines in <a href="https://www.python.org/dev/peps/pep-0207/">PEP 207 - Rich Comparisons</a> and <a href="https://docs.python.org/3/library/constants.html#NotImplemented">NotImplemented Python constant</a>.
 </p>
 

Examples/test-suite/python/python_richcompare_runme.py

@@ -156,3 +156,83 @@ if not (x[1] is base2):
 
 if not (x[2] is a3):
     raise RuntimeError("Ordering failed")
+
+
+# Test custom exceptions can still be thrown in operators which use %pythonmaybecall
+et0 = python_richcompare.ExceptionThrower(0)
+et1 = python_richcompare.ExceptionThrower(1)
+et2 = python_richcompare.ExceptionThrower(2)
+
+if not(et1 < et2):
+    raise RuntimeError("ExceptionThrower (a) failed")
+
+if et2 < et1:
+    raise RuntimeError("ExceptionThrower (b) failed")
+
+try:
+    x = et2 < et0
+    raise RuntimeError("ExceptionThrower failed to throw ValueError (A)")
+except ValueError:
+    pass
+
+try:
+    x = et0 < et2
+    raise RuntimeError("ExceptionThrower failed to throw ValueError (B)")
+except ValueError:
+    pass
+
+if sys.version_info[0:2] >= (3, 0):
+    try:
+        x = et2 < 99
+        raise RuntimeError("ExceptionThrower (d) failed")
+    except TypeError:
+        pass
+
+    try:
+        x = 99 < et2
+        raise RuntimeError("ExceptionThrower (e) failed")
+    except TypeError:
+        pass
+
+    try:
+        x = et0 < 99
+        raise RuntimeError("ExceptionThrower (f) failed")
+    except TypeError:
+        pass
+
+    try:
+        x = 99 < et0
+        raise RuntimeError("ExceptionThrower (g) failed")
+    except TypeError:
+        pass
+
+
+# Overloaded operators and custom exceptions
+c0 = python_richcompare.SubClassCThrower(0)
+c1 = python_richcompare.SubClassCThrower(1)
+c1b = python_richcompare.SubClassCThrower(1)
+c2 = python_richcompare.SubClassCThrower(2)
+
+if c1 == c2:
+    raise RuntimeError("SubClassCThrower failed (a)")
+
+if not(c1 == c1b):
+    raise RuntimeError("SubClassCThrower failed (b)")
+
+if c0 == 99:
+    raise RuntimeError("SubClassCThrower failed (c)")
+
+if 99 == c0:
+    raise RuntimeError("SubClassCThrower failed (d)")
+
+try:
+    x = c0 == c1
+    raise RuntimeError("SubClassCThrower failed to throw (A)")
+except ValueError:
+    pass
+
+try:
+    x = c1 == c0
+    raise RuntimeError("SubClassCThrower failed to throw (B)")
+except ValueError:
+    pass

Examples/test-suite/python_richcompare.i

@@ -56,5 +56,67 @@ public:
     bool operator== (const SubClassA& x) const
     { return false; }
 };
-
+}
+
+// Test custom exceptions can still be thrown in operators which use %pythonmaybecall
+%{
+struct ZeroValueProblem {
+    ZeroValueProblem() {}
+};
+%}
+
+%exception ExceptionThrower::operator< {
+    try
+    {
+        $action
+    }
+    catch(const ZeroValueProblem&)
+    {
+        PyErr_SetString(PyExc_ValueError, "Zero not liked in operator<");
+        SWIG_fail;
+    }
+}
+
+%inline {
+class ExceptionThrower {
+    int i;
+public:
+    ExceptionThrower (int i_) : i(i_) {}
+    bool operator< (const ExceptionThrower& rhs) const {
+        if (rhs.i == 0 || i == 0)
+            throw ZeroValueProblem();
+        return this->i < rhs.i;
+    }
+};
+}
+
+%exception SubClassCThrower::operator== {
+    try
+    {
+        $action
+    }
+    catch(const ZeroValueProblem&)
+    {
+        PyErr_SetString(PyExc_ValueError, "Zero not liked in operator==");
+        SWIG_fail;
+    }
+}
+
+// Overloaded operators and custom exceptions
+%inline {
+class SubClassCThrower : public BaseClass {
+public:
+    SubClassCThrower (int i_) : BaseClass(i_) {}
+    ~SubClassCThrower () {}
+
+    bool operator== (const SubClassCThrower& rhs) const
+    {
+        if (rhs.i == 0 || i == 0)
+            throw ZeroValueProblem();
+        return rhs.i == i;
+    }
+
+    bool operator== (const SubClassA& rhs) const
+    { return rhs.i == i; }
+};
 }

Lib/python/pyrun.swg

@@ -683,12 +683,14 @@ SwigPyObject_compare(SwigPyObject *v, SwigPyObject *w)
 SWIGRUNTIME PyObject*
 SwigPyObject_richcompare(SwigPyObject *v, SwigPyObject *w, int op)
 {
-  PyObject* res;
-  if( op != Py_EQ && op != Py_NE ) {
+  PyObject* res = NULL;
+  if (!PyErr_Occurred()) {
+    if (op != Py_EQ && op != Py_NE) {
       Py_INCREF(Py_NotImplemented);
       return Py_NotImplemented;
     }
     res = PyBool_FromLong( (SwigPyObject_compare(v, w)==0) == (op == Py_EQ) ? 1 : 0);
+  }
   return res;  
 }
 

Source/Modules/python.cxx

@@ -3222,6 +3222,9 @@ public:
       Printv(f->code, "  return -1;\n", NIL);
     } else {
       if (GetFlag(n, "feature:python:maybecall")) {
+	Append(f->code, "  if (PyErr_Occurred() && !PyErr_ExceptionMatches(PyExc_TypeError)) {\n");
+	Append(f->code, "    return NULL;\n");
+	Append(f->code, "  }\n");
 	Append(f->code, "  PyErr_Clear();\n");
 	Append(f->code, "  Py_INCREF(Py_NotImplemented);\n");
 	Append(f->code, "  return Py_NotImplemented;\n");
@@ -4068,7 +4071,7 @@ public:
       Printf(f, "  }\n");
     }
     Delete(richcompare_list);
-    Printv(f, "  if (!result) {\n", NIL);
+    Printv(f, "  if (!result && !PyErr_Occurred()) {\n", NIL);
     Printv(f, "    if (SwigPyObject_Check(self) && SwigPyObject_Check(other)) {\n", NIL);
     Printv(f, "      result = SwigPyObject_richcompare((SwigPyObject *)self, (SwigPyObject *)other, op);\n", NIL);
     Printv(f, "    } else {\n", NIL);