[mypyc] Fix RecursionError from binary dunders without a reverse method

mypyc/codegen/emitwrapper.py

@@ -15,7 +15,7 @@
 from collections.abc import Sequence
 
 from mypy.nodes import ARG_NAMED, ARG_NAMED_OPT, ARG_OPT, ARG_POS, ARG_STAR, ARG_STAR2, ArgKind
-from mypy.operators import op_methods_to_symbols, reverse_op_method_names, reverse_op_methods
+from mypy.operators import reverse_op_method_names, reverse_op_methods
 from mypyc.codegen.emit import AssignHandler, Emitter, ErrorHandler, GotoHandler, ReturnHandler
 from mypyc.common import (
     BITMAP_BITS,
@@ -364,11 +364,16 @@ def generate_bin_op_forward_only_wrapper(
 ) -> None:
     gen.emit_arg_processing(error=GotoHandler("typefail"), raise_exception=False)
     handle_third_pow_argument(fn, emitter, gen, if_unsupported=["goto typefail;"])
-    gen.emit_call(not_implemented_handler="goto typefail;")
+    gen.emit_call()
     gen.emit_error_handling()
     emitter.emit_label("typefail")
     # If some argument has an incompatible type, treat this the same as
-    # returning NotImplemented, and try to call the reverse operator method.
+    # returning NotImplemented, and let CPython try the reverse operator
+    # method of the right operand. Don't call the reverse method here:
+    # this class doesn't define one, so for an instance of this class
+    # (as in 'x + x' or '1 + x') the lookup would find the slot wrapper
+    # that CPython adds for this slot, which calls this wrapper again
+    # with the same arguments.
     #
     # Note that in normal Python you'd instead of an explicit
     # return of NotImplemented, but it doesn't generally work here
@@ -382,7 +387,7 @@ def generate_bin_op_forward_only_wrapper(
     #        if not isinstance(other, int):
     #            return NotImplemented
     #        ...
-    generate_bin_op_reverse_dunder_call(fn, emitter, reverse_op_methods[fn.name])
+    emitter.emit_line("Py_RETURN_NOTIMPLEMENTED;")
     gen.finish()
 
 
@@ -395,8 +400,7 @@ def generate_bin_op_reverse_only_wrapper(
     gen.emit_call()
     gen.emit_error_handling()
     emitter.emit_label("typefail")
-    emitter.emit_line("Py_INCREF(Py_NotImplemented);")
-    emitter.emit_line("return Py_NotImplemented;")
+    emitter.emit_line("Py_RETURN_NOTIMPLEMENTED;")
     gen.finish()
 
 
@@ -439,31 +443,14 @@ def generate_bin_op_both_wrappers(
     handle_third_pow_argument(fn_rev, emitter, gen, if_unsupported=["goto typefail2;"])
     gen.emit_call()
     gen.emit_error_handling()
-    emitter.emit_line("} else {")
-    generate_bin_op_reverse_dunder_call(fn, emitter, fn_rev.name)
     emitter.emit_line("}")
+    # If the right operand has a different type, CPython tries its reverse
+    # method after we return NotImplemented.
     emitter.emit_label("typefail2")
-    emitter.emit_line("Py_INCREF(Py_NotImplemented);")
-    emitter.emit_line("return Py_NotImplemented;")
+    emitter.emit_line("Py_RETURN_NOTIMPLEMENTED;")
     gen.finish()
 
 
-def generate_bin_op_reverse_dunder_call(fn: FuncIR, emitter: Emitter, rmethod: str) -> None:
-    if fn.name in ("__pow__", "__rpow__"):
-        # Ternary pow() will never call the reverse dunder.
-        emitter.emit_line("if (obj_mod == Py_None) {")
-    emitter.emit_line(
-        'return CPy_CallReverseOpMethod(obj_left, obj_right, "{}", mypyc_interned_str.{});'.format(
-            op_methods_to_symbols[fn.name], rmethod
-        )
-    )
-    if fn.name in ("__pow__", "__rpow__"):
-        emitter.emit_line("} else {")
-        emitter.emit_line("Py_INCREF(Py_NotImplemented);")
-        emitter.emit_line("return Py_NotImplemented;")
-        emitter.emit_line("}")
-
-
 def handle_third_pow_argument(
     fn: FuncIR, emitter: Emitter, gen: WrapperGenerator, *, if_unsupported: list[str]
 ) -> None:

mypyc/lib-rt/CPy.h

@@ -1078,8 +1078,6 @@ int CPyStatics_Initialize(PyObject **statics,
                           const int *tuples,
                           const int *frozensets);
 PyObject *CPy_Super(PyObject *builtins, PyObject *self);
-PyObject *CPy_CallReverseOpMethod(PyObject *left, PyObject *right, const char *op,
-                                  PyObject *method);
 
 bool CPyImport_ImportMany(PyObject *modules, CPyModuleCache *statics[], PyObject *globals,
                           PyObject *tb_path, PyObject *tb_function, Py_ssize_t *tb_lines);

mypyc/lib-rt/misc_ops.c

@@ -926,38 +926,6 @@ PyObject *CPyImport_GetNativeAttrs(PyObject *mod, PyObject *names,
     return Py_NewRef(mod);
 }
 
-// From CPython
-static PyObject *
-CPy_BinopTypeError(PyObject *left, PyObject *right, const char *op) {
-    PyErr_Format(PyExc_TypeError,
-                 "unsupported operand type(s) for %.100s: "
-                 "'%.100s' and '%.100s'",
-                 op,
-                 Py_TYPE(left)->tp_name,
-                 Py_TYPE(right)->tp_name);
-    return NULL;
-}
-
-PyObject *
-CPy_CallReverseOpMethod(PyObject *left,
-                        PyObject *right,
-                        const char *op,
-                        PyObject *method) {
-    // Look up reverse method
-    PyObject *m = PyObject_GetAttr(right, method);
-    if (m == NULL) {
-        // If reverse method not defined, generate TypeError instead AttributeError
-        if (PyErr_ExceptionMatches(PyExc_AttributeError)) {
-            CPy_BinopTypeError(left, right, op);
-        }
-        return NULL;
-    }
-    // Call reverse method
-    PyObject *result = PyObject_CallOneArg(m, left);
-    Py_DECREF(m);
-    return result;
-}
-
 PyObject *CPySingledispatch_RegisterFunction(PyObject *singledispatch_func,
                                              PyObject *cls,
                                              PyObject *func) {

mypyc/lib-rt/static_data.c

@@ -28,20 +28,6 @@ intern_strings(void) {
     INTERN_STRING(__orig_bases__, "__orig_bases__");
     INTERN_STRING(__qualname__, "__qualname__");
     INTERN_STRING(__slots__, "__slots__");
-    INTERN_STRING(__radd__, "__radd__");
-    INTERN_STRING(__rsub__, "__rsub__");
-    INTERN_STRING(__rmul__, "__rmul__");
-    INTERN_STRING(__rtruediv__, "__rtruediv__");
-    INTERN_STRING(__rmod__, "__rmod__");
-    INTERN_STRING(__rdivmod__, "__rdivmod__");
-    INTERN_STRING(__rfloordiv__, "__rfloordiv__");
-    INTERN_STRING(__rpow__, "__rpow__");
-    INTERN_STRING(__rmatmul__, "__rmatmul__");
-    INTERN_STRING(__rand__, "__rand__");
-    INTERN_STRING(__ror__, "__ror__");
-    INTERN_STRING(__rxor__, "__rxor__");
-    INTERN_STRING(__rlshift__, "__rlshift__");
-    INTERN_STRING(__rrshift__, "__rrshift__");
     INTERN_STRING(__eq__, "__eq__");
     INTERN_STRING(__ne__, "__ne__");
     INTERN_STRING(__gt__, "__gt__");

mypyc/lib-rt/static_data.h

@@ -20,20 +20,6 @@ typedef struct mypyc_interned_str_struct {
     PyObject *__qualname__;
     PyObject *__slots__;
     PyObject *__name__;
-    PyObject *__radd__;
-    PyObject *__rsub__;
-    PyObject *__rmul__;
-    PyObject *__rtruediv__;
-    PyObject *__rmod__;
-    PyObject *__rdivmod__;
-    PyObject *__rfloordiv__;
-    PyObject *__rpow__;
-    PyObject *__rmatmul__;
-    PyObject *__rand__;
-    PyObject *__ror__;
-    PyObject *__rxor__;
-    PyObject *__rlshift__;
-    PyObject *__rrshift__;
     PyObject *__eq__;
     PyObject *__ne__;
     PyObject *__gt__;

mypyc/test-data/run-dunders.test

@@ -444,7 +444,7 @@ def test_error() -> None:
     try:
         a ** 'x'
     except TypeError as e:
-        assert str(e) == "unsupported operand type(s) for **: 'C' and 'str'"
+        assert str(e) == "unsupported operand type(s) for ** or pow(): 'C' and 'str'"
     else:
         assert False
 
@@ -666,6 +666,94 @@ def test_unannotated_add_and_radd_2() -> None:
 
 # TODO: Inheritance
 
+[case testDundersBinaryForwardOnlyNotImplemented]
+from typing import Any
+from testutil import assertRaises
+from interp import CountRadd
+
+class C:
+    def __add__(self, x: int) -> int:
+        return x + 1
+
+    def __pow__(self, x: int) -> int:
+        return x + 2
+
+class Sub(C):
+    pass
+
+class D:
+    def __add__(self, x: int) -> int:
+        return x + 3
+
+    def __radd__(self, x: int) -> int:
+        return x + 4
+
+class E:
+    def __xor__(self, x: E) -> Any:
+        return NotImplemented
+
+def test_forward_only() -> None:
+    c: Any = C()
+    assert c + 1 == 2
+    assert c ** 1 == 3
+    with assertRaises(TypeError, "unsupported operand type(s) for +: 'C' and 'C'"):
+        c + c
+    with assertRaises(TypeError, "unsupported operand type(s) for +: 'int' and 'C'"):
+        1 + c
+    with assertRaises(TypeError, 'can only concatenate str (not "C") to str'):
+        'x' + c
+    with assertRaises(TypeError, "unsupported operand type(s) for ** or pow(): 'C' and 'C'"):
+        c ** c
+    with assertRaises(TypeError, "unsupported operand type(s) for ** or pow(): 'int' and 'C'"):
+        2 ** c
+    with assertRaises(TypeError, "unsupported operand type(s) for ** or pow(): 'float' and 'C'"):
+        2.5 ** c
+    with assertRaises(TypeError, "unsupported operand type(s) for ** or pow(): 'NoneType' and 'C'"):
+        None ** c
+    x: Any = 2
+    with assertRaises(TypeError, "unsupported operand type(s) for **=: 'int' and 'C'"):
+        x **= c
+
+def test_forward_returns_not_implemented() -> None:
+    e = E()
+    with assertRaises(TypeError, "unsupported operand type(s) for ^: 'E' and 'E'"):
+        e ^ e
+
+def test_inherited_slot() -> None:
+    s: Any = Sub()
+    assert s + 1 == 2
+    with assertRaises(TypeError, "unsupported operand type(s) for +: 'Sub' and 'Sub'"):
+        s + s
+    with assertRaises(TypeError, "unsupported operand type(s) for +: 'int' and 'Sub'"):
+        1 + s
+    with assertRaises(TypeError, "unsupported operand type(s) for +: 'C' and 'Sub'"):
+        C() + s
+
+def test_other_native_class() -> None:
+    c: Any = C()
+    d: Any = D()
+    with assertRaises(TypeError, "unsupported operand type(s) for +: 'C' and 'D'"):
+        c + d
+    with assertRaises(TypeError, "unsupported operand type(s) for +: 'D' and 'C'"):
+        d + c
+
+def test_reverse_method_called_once() -> None:
+    objs: list[Any] = [C(), D()]
+    for o in objs:
+        r = CountRadd()
+        with assertRaises(TypeError):
+            o + r
+        assert r.calls == 1
+
+[file interp.py]
+class CountRadd:
+    def __init__(self) -> None:
+        self.calls = 0
+
+    def __radd__(self, x: object) -> object:
+        self.calls += 1
+        return NotImplemented
+
 [case testDifferentReverseDunders]
 from typing import Any
 
@@ -945,7 +1033,7 @@ def test_errors() -> None:
     f = Forward()
     with assertRaises(TypeError, f"unsupported operand type(s) for {op}: 'Forward', 'int', 'str'"):
         pow(f, 3, "x")  # type: ignore
-    with assertRaises(TypeError, "unsupported operand type(s) for **: 'Forward' and 'str'"):
+    with assertRaises(TypeError, "unsupported operand type(s) for ** or pow(): 'Forward' and 'str'"):
         f**"x"  # type: ignore
     r = Reverse()
     with assertRaises(TypeError, "unsupported operand type(s) for ** or pow(): 'str' and 'Reverse'"):