Raise TypeError instead of recursing on unsupported operator directions

src/embed_tests/TestOperator.cs

@@ -550,5 +550,125 @@ public void ShiftOperatorOverloads()
 assert c.Num == a.Num >> b.Num
 ");
         }
+
+        public class ForwardOnlyOperable
+        {
+            public static ForwardOnlyOperable operator +(ForwardOnlyOperable a, ForwardOnlyOperable b) => a;
+        }
+
+        public class ReverseOnlyOperable
+        {
+            public static ReverseOnlyOperable operator +(double a, ReverseOnlyOperable b) => b;
+        }
+
+        [Flags]
+        public enum OperableFlags
+        {
+            A = 1,
+            B = 2
+        }
+
+        private const string OperationErrors = @"
+def error(operation):
+    try:
+        operation()
+    except Exception as e:
+        return type(e).__name__
+";
+
+        [Test]
+        public void UnsupportedReflectedOperationRaisesTypeError()
+        {
+            // the reflected operation has no CLR overload, it used to recurse until RecursionError
+            using var _ = Py.GIL();
+            using var scope = Py.CreateScope();
+            scope.Exec(OperationErrors + @"
+from Python.EmbeddingTest import TestOperator
+x = TestOperator.ForwardOnlyOperable()
+supported = error(lambda: x + x), error(lambda: x.__add__(x))
+unsupported = error(lambda: 1.0 + x), error(lambda: 1 + x), error(lambda: sum([x])), error(lambda: x + 1.0)
+has_reflected = hasattr(x, '__radd__')
+");
+            Assert.AreEqual(new string[] { null, null }, scope.Get<string[]>("supported"));
+            CollectionAssert.AreEqual(new[] { "TypeError", "TypeError", "TypeError", "TypeError" }, scope.Get<string[]>("unsupported"));
+            Assert.IsFalse(scope.Get<bool>("has_reflected"));
+        }
+
+        [Test]
+        public void UnsupportedForwardOperationRaisesTypeError()
+        {
+            // the forward operation has no CLR overload, it used to recurse until RecursionError
+            using var _ = Py.GIL();
+            using var scope = Py.CreateScope();
+            scope.Exec(OperationErrors + @"
+from Python.EmbeddingTest import TestOperator
+y = TestOperator.ReverseOnlyOperable()
+supported = error(lambda: 1.0 + y)
+unsupported = error(lambda: y + 1.0), error(lambda: y + y)
+has_forward = hasattr(y, '__add__')
+");
+            Assert.IsNull(scope.Get<string>("supported"));
+            CollectionAssert.AreEqual(new[] { "TypeError", "TypeError" }, scope.Get<string[]>("unsupported"));
+            Assert.IsFalse(scope.Get<bool>("has_forward"));
+        }
+
+        [Test]
+        public void UnsupportedReflectedFlagsOperationRaisesTypeError()
+        {
+            // the [Flags] bitwise operators are forward only
+            using var _ = Py.GIL();
+            using var scope = Py.CreateScope();
+            scope.Exec(OperationErrors + @"
+from Python.EmbeddingTest import TestOperator
+flags = TestOperator.OperableFlags.A | TestOperator.OperableFlags.B
+supported = error(lambda: flags & TestOperator.OperableFlags.A)
+unsupported = error(lambda: 1 & flags), error(lambda: 1 | flags)
+");
+            Assert.IsNull(scope.Get<string>("supported"));
+            CollectionAssert.AreEqual(new[] { "TypeError", "TypeError" }, scope.Get<string[]>("unsupported"));
+        }
+
+        [Test]
+        public void PythonSubclassCanStillDefineTheReflectedOperation()
+        {
+            using var _ = Py.GIL();
+            using var scope = Py.CreateScope();
+            scope.Exec(@"
+from Python.EmbeddingTest import TestOperator
+class Custom(TestOperator.ForwardOnlyOperable):
+    def __radd__(self, other):
+        return 'custom'
+class Plain(TestOperator.ForwardOnlyOperable):
+    pass
+custom = 1.0 + Custom()
+try:
+    1.0 + Plain()
+    plain = None
+except TypeError:
+    plain = 'TypeError'
+");
+            Assert.AreEqual("custom", scope.Get<string>("custom"));
+            Assert.AreEqual("TypeError", scope.Get<string>("plain"));
+        }
+
+        public class ForwardOnlyOperableWithReflectedMethod
+        {
+            public static ForwardOnlyOperableWithReflectedMethod operator +(ForwardOnlyOperableWithReflectedMethod a, ForwardOnlyOperableWithReflectedMethod b) => a;
+
+            // a CLR member named like the reflected method, not a slot wrapper
+            public string __radd__(double other) => "clr";
+        }
+
+        [Test]
+        public void ClrMemberNamedLikeTheReflectedMethodIsKept()
+        {
+            using var _ = Py.GIL();
+            using var scope = Py.CreateScope();
+            scope.Exec(@"
+from Python.EmbeddingTest import TestOperator
+result = 1.0 + TestOperator.ForwardOnlyOperableWithReflectedMethod()
+");
+            Assert.AreEqual("clr", scope.Get<string>("result"));
+        }
     }
 }

src/runtime/TypeManager.cs

@@ -295,6 +295,7 @@ internal static void InitializeClass(PyType type, ClassBase impl, Type clrType)
             {
                 throw PythonException.ThrowLastAsClrException();
             }
+            OperatorMethod.RemoveUnboundSlotWrappers(type, clrType);
 
             var dict = Util.ReadRef(type, TypeOffset.tp_dict);
             string mn = clrType.Namespace ?? "";

src/runtime/Types/OperatorMethod.cs

@@ -127,6 +127,47 @@ public static void FixupSlots(BorrowedReference pyType, Type clrType)
             }
         }
 
+        /// <summary>
+        /// PyType_Ready adds a slot wrapper for the forward and the reflected method of every number slot
+        /// <see cref="FixupSlots"/> set (__add__ and __radd__ for nb_add) the type does not define. With no CLR
+        /// overload behind them they call the very slot that dispatched to them, so an operation the type does
+        /// not support, like 1.0 + x when x only declares x + x, recursed until RecursionError. Removing them
+        /// lets the slot return NotImplemented, so Python raises its usual TypeError.
+        /// </summary>
+        public static void RemoveUnboundSlotWrappers(BorrowedReference pyType, Type clrType)
+        {
+            // best effort: a wrapper left behind only keeps the previous behavior, so nothing here fails the type creation
+            try
+            {
+                var dict = Util.ReadRef(pyType, TypeOffset.tp_dict);
+                var fixedSlots = GetOperatorCandidates(clrType)
+                    .Where(method => OpMethodMap.ContainsKey(method.Name))
+                    .Select(method => OpMethodMap[method.Name].MethodName)
+                    .Distinct();
+                foreach (var pyName in fixedSlots)
+                {
+                    RemoveSlotWrapper(dict, pyName);
+                    RemoveSlotWrapper(dict, ReversePyMethodName(pyName));
+                }
+            }
+            catch
+            {
+                // pass
+            }
+        }
+
+        private static void RemoveSlotWrapper(BorrowedReference dict, string name)
+        {
+            var member = Runtime.PyDict_GetItemString(dict, name);
+            // only the slot wrappers PyType_Ready added, the CLR operators are managed method objects
+            if (!member.IsNull && Runtime.PyObject_TYPE(member) == Runtime.PyWrapperDescriptorType
+                && Runtime.PyDict_DelItemString(dict, name) != 0)
+            {
+                // don't let a failed delete leak, the wrapper just stays
+                Exceptions.Clear();
+            }
+        }
+
         static IEnumerable<MethodInfo> GetOperatorCandidates(Type clrType)
         {
             const BindingFlags flags = BindingFlags.Public | BindingFlags.Static;