diff --git a/src/embed_tests/TestOperator.cs b/src/embed_tests/TestOperator.cs index 078215077..8254899f0 100644 --- a/src/embed_tests/TestOperator.cs +++ b/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("supported")); + CollectionAssert.AreEqual(new[] { "TypeError", "TypeError", "TypeError", "TypeError" }, scope.Get("unsupported")); + Assert.IsFalse(scope.Get("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("supported")); + CollectionAssert.AreEqual(new[] { "TypeError", "TypeError" }, scope.Get("unsupported")); + Assert.IsFalse(scope.Get("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("supported")); + CollectionAssert.AreEqual(new[] { "TypeError", "TypeError" }, scope.Get("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("custom")); + Assert.AreEqual("TypeError", scope.Get("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("result")); + } } } diff --git a/src/runtime/TypeManager.cs b/src/runtime/TypeManager.cs index cc197d32b..f526d87a6 100644 --- a/src/runtime/TypeManager.cs +++ b/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 ?? ""; diff --git a/src/runtime/Types/OperatorMethod.cs b/src/runtime/Types/OperatorMethod.cs index e905375e0..12f79ab10 100644 --- a/src/runtime/Types/OperatorMethod.cs +++ b/src/runtime/Types/OperatorMethod.cs @@ -127,6 +127,47 @@ public static void FixupSlots(BorrowedReference pyType, Type clrType) } } + /// + /// PyType_Ready adds a slot wrapper for the forward and the reflected method of every number slot + /// 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. + /// + 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 GetOperatorCandidates(Type clrType) { const BindingFlags flags = BindingFlags.Public | BindingFlags.Static;