Skip to content

Commit 83fd880

Browse files
Raise TypeError instead of recursing on unsupported operator directions
PyType_Ready adds a slot wrapper for the forward and the reflected method of each number slot FixupSlots sets, when the type dict lacks them. A type declaring an operator in one direction only, like x + x, got a __radd__ wrapper that calls the very slot that dispatched to it, so 1.0 + x, 1 + x or sum([x]) recursed until RecursionError instead of raising TypeError. The same happened with [Flags] enums, whose bitwise operators are forward only (1 & flags). The wrappers without a CLR operator behind them are now removed after PyType_Ready, so the slot returns NotImplemented and Python raises its usual TypeError. A Python subclass can still define the missing method. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 56b3b6e commit 83fd880

4 files changed

Lines changed: 133 additions & 0 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ Instead, `PyIterable` does that.
9292

9393
### Fixed
9494

95+
- An operation a type supports in one direction only, like `1.0 + x` when `x` only declares `x + x`, raises `TypeError` instead of recursing until `RecursionError`
9596
- Fix incorrect dereference of wrapper object in `tp_repr`, which may result in a program crash
9697
- Fixed parameterless .NET constructor being silently called when a matching constructor overload is not found ([#238][i238])
9798
- Fix incorrect dereference in params array handling

‎src/embed_tests/TestOperator.cs‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -550,5 +550,105 @@ public void ShiftOperatorOverloads()
550550
assert c.Num == a.Num >> b.Num
551551
");
552552
}
553+
554+
public class ForwardOnlyOperable
555+
{
556+
public static ForwardOnlyOperable operator +(ForwardOnlyOperable a, ForwardOnlyOperable b) => a;
557+
}
558+
559+
public class ReverseOnlyOperable
560+
{
561+
public static ReverseOnlyOperable operator +(double a, ReverseOnlyOperable b) => b;
562+
}
563+
564+
[Flags]
565+
public enum OperableFlags
566+
{
567+
A = 1,
568+
B = 2
569+
}
570+
571+
private const string OperationErrors = @"
572+
def error(operation):
573+
try:
574+
operation()
575+
except Exception as e:
576+
return type(e).__name__
577+
";
578+
579+
[Test]
580+
public void UnsupportedReflectedOperationRaisesTypeError()
581+
{
582+
// the reflected operation has no CLR overload, it used to recurse until RecursionError
583+
using var _ = Py.GIL();
584+
using var scope = Py.CreateScope();
585+
scope.Exec(OperationErrors + @"
586+
from Python.EmbeddingTest import TestOperator
587+
x = TestOperator.ForwardOnlyOperable()
588+
supported = error(lambda: x + x), error(lambda: x.__add__(x))
589+
unsupported = error(lambda: 1.0 + x), error(lambda: 1 + x), error(lambda: sum([x])), error(lambda: x + 1.0)
590+
has_reflected = hasattr(x, '__radd__')
591+
");
592+
Assert.AreEqual(new string[] { null, null }, scope.Get<string[]>("supported"));
593+
CollectionAssert.AreEqual(new[] { "TypeError", "TypeError", "TypeError", "TypeError" }, scope.Get<string[]>("unsupported"));
594+
Assert.IsFalse(scope.Get<bool>("has_reflected"));
595+
}
596+
597+
[Test]
598+
public void UnsupportedForwardOperationRaisesTypeError()
599+
{
600+
// the forward operation has no CLR overload, it used to recurse until RecursionError
601+
using var _ = Py.GIL();
602+
using var scope = Py.CreateScope();
603+
scope.Exec(OperationErrors + @"
604+
from Python.EmbeddingTest import TestOperator
605+
y = TestOperator.ReverseOnlyOperable()
606+
supported = error(lambda: 1.0 + y)
607+
unsupported = error(lambda: y + 1.0), error(lambda: y + y)
608+
has_forward = hasattr(y, '__add__')
609+
");
610+
Assert.IsNull(scope.Get<string>("supported"));
611+
CollectionAssert.AreEqual(new[] { "TypeError", "TypeError" }, scope.Get<string[]>("unsupported"));
612+
Assert.IsFalse(scope.Get<bool>("has_forward"));
613+
}
614+
615+
[Test]
616+
public void UnsupportedReflectedFlagsOperationRaisesTypeError()
617+
{
618+
// the [Flags] bitwise operators are forward only
619+
using var _ = Py.GIL();
620+
using var scope = Py.CreateScope();
621+
scope.Exec(OperationErrors + @"
622+
from Python.EmbeddingTest import TestOperator
623+
flags = TestOperator.OperableFlags.A | TestOperator.OperableFlags.B
624+
supported = error(lambda: flags & TestOperator.OperableFlags.A)
625+
unsupported = error(lambda: 1 & flags), error(lambda: 1 | flags)
626+
");
627+
Assert.IsNull(scope.Get<string>("supported"));
628+
CollectionAssert.AreEqual(new[] { "TypeError", "TypeError" }, scope.Get<string[]>("unsupported"));
629+
}
630+
631+
[Test]
632+
public void PythonSubclassCanStillDefineTheReflectedOperation()
633+
{
634+
using var _ = Py.GIL();
635+
using var scope = Py.CreateScope();
636+
scope.Exec(@"
637+
from Python.EmbeddingTest import TestOperator
638+
class Custom(TestOperator.ForwardOnlyOperable):
639+
def __radd__(self, other):
640+
return 'custom'
641+
class Plain(TestOperator.ForwardOnlyOperable):
642+
pass
643+
custom = 1.0 + Custom()
644+
try:
645+
1.0 + Plain()
646+
plain = None
647+
except TypeError:
648+
plain = 'TypeError'
649+
");
650+
Assert.AreEqual("custom", scope.Get<string>("custom"));
651+
Assert.AreEqual("TypeError", scope.Get<string>("plain"));
652+
}
553653
}
554654
}

‎src/runtime/TypeManager.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,7 @@ internal static void InitializeClass(PyType type, ClassBase impl, Type clrType)
295295
{
296296
throw PythonException.ThrowLastAsClrException();
297297
}
298+
OperatorMethod.RemoveUnboundSlotWrappers(type, clrType);
298299

299300
var dict = Util.ReadRef(type, TypeOffset.tp_dict);
300301
string mn = clrType.Namespace ?? "";

‎src/runtime/Types/OperatorMethod.cs‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,37 @@ public static void FixupSlots(BorrowedReference pyType, Type clrType)
127127
}
128128
}
129129

130+
/// <summary>
131+
/// PyType_Ready adds a slot wrapper for the forward and the reflected method of every number slot
132+
/// <see cref="FixupSlots"/> set (__add__ and __radd__ for nb_add) the type does not define. With no CLR
133+
/// overload behind them they call the very slot that dispatched to them, so an operation the type does
134+
/// not support, like 1.0 + x when x only declares x + x, recursed until RecursionError. Removing them
135+
/// lets the slot return NotImplemented, so Python raises its usual TypeError.
136+
/// </summary>
137+
public static void RemoveUnboundSlotWrappers(BorrowedReference pyType, Type clrType)
138+
{
139+
var dict = Util.ReadRef(pyType, TypeOffset.tp_dict);
140+
var fixedSlots = GetOperatorCandidates(clrType)
141+
.Where(method => OpMethodMap.ContainsKey(method.Name))
142+
.Select(method => OpMethodMap[method.Name].MethodName)
143+
.Distinct();
144+
foreach (var pyName in fixedSlots)
145+
{
146+
RemoveSlotWrapper(dict, pyName);
147+
RemoveSlotWrapper(dict, ReversePyMethodName(pyName));
148+
}
149+
}
150+
151+
private static void RemoveSlotWrapper(BorrowedReference dict, string name)
152+
{
153+
var member = Runtime.PyDict_GetItemString(dict, name);
154+
// the CLR operators are managed method objects, anything else was added by PyType_Ready
155+
if (!member.IsNull && ManagedType.GetManagedObject(member) == null && Runtime.PyDict_DelItemString(dict, name) != 0)
156+
{
157+
throw PythonException.ThrowLastAsClrException();
158+
}
159+
}
160+
130161
static IEnumerable<MethodInfo> GetOperatorCandidates(Type clrType)
131162
{
132163
const BindingFlags flags = BindingFlags.Public | BindingFlags.Static;

0 commit comments

Comments
 (0)