Skip to content

Commit 01d6772

Browse files
gertdreyerlostmsu
authored andcommitted
Bugfix: RecursionError when reverse/righthand operations invoked. e.g. __rsub__, __rmul__
1 parent 87fc365 commit 01d6772

File tree

5 files changed

+42
-39
lines changed

5 files changed

+42
-39
lines changed

src/runtime/ClassManager.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -546,7 +546,7 @@ private static ClassInfo GetClassInfo(Type type, ClassBase impl)
546546
ci.members[pyName] = new MethodObject(type, name, forwardMethods).AllocObject();
547547
// Only methods where only the right operand is the declaring type.
548548
if (reverseMethods.Length > 0)
549-
ci.members[pyNameReverse] = new MethodObject(type, name, reverseMethods).AllocObject();
549+
ci.members[pyNameReverse] = new MethodObject(type, name, reverseMethods, reverse_args: true).AllocObject();
550550
}
551551
}
552552

src/runtime/MethodBinder.cs

+24-16
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,22 @@ internal class MethodBinder
2828

2929
[NonSerialized]
3030
public bool init = false;
31+
3132
public const bool DefaultAllowThreads = true;
3233
public bool allow_threads = DefaultAllowThreads;
3334

34-
internal MethodBinder()
35+
public bool args_reversed = false;
36+
37+
internal MethodBinder(bool reverse_args = false)
3538
{
3639
list = new List<MaybeMethodBase>();
40+
args_reversed = reverse_args;
3741
}
3842

39-
internal MethodBinder(MethodInfo mi)
43+
internal MethodBinder(MethodInfo mi, bool reverse_args = false)
4044
{
4145
list = new List<MaybeMethodBase> { new MaybeMethodBase(mi) };
46+
args_reversed = reverse_args;
4247
}
4348

4449
public int Count
@@ -271,10 +276,11 @@ internal static int ArgPrecedence(Type t)
271276
/// <param name="inst">The Python target of the method invocation.</param>
272277
/// <param name="args">The Python arguments.</param>
273278
/// <param name="kw">The Python keyword arguments.</param>
279+
/// <param name="reverse_args">Reverse arguments of methods. Used for methods such as __radd__, __rsub__, __rmod__ etc</param>
274280
/// <returns>A Binding if successful. Otherwise null.</returns>
275-
internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw)
281+
internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, bool reverse_args = false)
276282
{
277-
return Bind(inst, args, kw, null, null);
283+
return Bind(inst, args, kw, null, null, reverse_args);
278284
}
279285

280286
/// <summary>
@@ -287,10 +293,11 @@ internal static int ArgPrecedence(Type t)
287293
/// <param name="args">The Python arguments.</param>
288294
/// <param name="kw">The Python keyword arguments.</param>
289295
/// <param name="info">If not null, only bind to that method.</param>
296+
/// <param name="reverse_args">Reverse arguments of methods. Used for methods such as __radd__, __rsub__, __rmod__ etc</param>
290297
/// <returns>A Binding if successful. Otherwise null.</returns>
291-
internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info)
298+
internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, bool reverse_args = false)
292299
{
293-
return Bind(inst, args, kw, info, null);
300+
return Bind(inst, args, kw, info, null, reverse_args);
294301
}
295302

296303
private readonly struct MatchedMethod
@@ -334,8 +341,9 @@ public MismatchedMethod(Exception exception, MethodBase mb)
334341
/// <param name="kw">The Python keyword arguments.</param>
335342
/// <param name="info">If not null, only bind to that method.</param>
336343
/// <param name="methodinfo">If not null, additionally attempt to bind to the generic methods in this array by inferring generic type parameters.</param>
344+
/// <param name="reverse_args">Reverse arguments of methods. Used for methods such as __radd__, __rsub__, __rmod__ etc</param>
337345
/// <returns>A Binding if successful. Otherwise null.</returns>
338-
internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, MethodBase[]? methodinfo)
346+
internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, MethodBase[]? methodinfo, bool reverse_args = false)
339347
{
340348
// loop to find match, return invoker w/ or w/o error
341349
var kwargDict = new Dictionary<string, PyObject>();
@@ -363,10 +371,10 @@ public MismatchedMethod(Exception exception, MethodBase mb)
363371
_methods = GetMethods();
364372
}
365373

366-
return Bind(inst, args, kwargDict, _methods, matchGenerics: true);
374+
return Bind(inst, args, kwargDict, _methods, matchGenerics: true, reverse_args);
367375
}
368376

369-
static Binding? Bind(BorrowedReference inst, BorrowedReference args, Dictionary<string, PyObject> kwargDict, MethodBase[] methods, bool matchGenerics)
377+
private static Binding? Bind(BorrowedReference inst, BorrowedReference args, Dictionary<string, PyObject> kwargDict, MethodBase[] methods, bool matchGenerics, bool reversed = false)
370378
{
371379
var pynargs = (int)Runtime.PyTuple_Size(args);
372380
var isGeneric = false;
@@ -386,7 +394,7 @@ public MismatchedMethod(Exception exception, MethodBase mb)
386394
// Binary operator methods will have 2 CLR args but only one Python arg
387395
// (unary operators will have 1 less each), since Python operator methods are bound.
388396
isOperator = isOperator && pynargs == pi.Length - 1;
389-
bool isReverse = isOperator && OperatorMethod.IsReverse((MethodInfo)mi); // Only cast if isOperator.
397+
bool isReverse = isOperator && reversed; // Only cast if isOperator.
390398
if (isReverse && OperatorMethod.IsComparisonOp((MethodInfo)mi))
391399
continue; // Comparison operators in Python have no reverse mode.
392400
if (!MatchesArgumentCount(pynargs, pi, kwargDict, out bool paramsArray, out ArrayList? defaultArgList, out int kwargsMatched, out int defaultsNeeded) && !isOperator)
@@ -809,14 +817,14 @@ static bool MatchesArgumentCount(int positionalArgumentCount, ParameterInfo[] pa
809817
return match;
810818
}
811819

812-
internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw)
820+
internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, bool reverse_args = false)
813821
{
814-
return Invoke(inst, args, kw, null, null);
822+
return Invoke(inst, args, kw, null, null, reverse_args);
815823
}
816824

817-
internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info)
825+
internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, bool reverse_args = false)
818826
{
819-
return Invoke(inst, args, kw, info, null);
827+
return Invoke(inst, args, kw, info, null, reverse_args = false);
820828
}
821829

822830
protected static void AppendArgumentTypes(StringBuilder to, BorrowedReference args)
@@ -852,7 +860,7 @@ protected static void AppendArgumentTypes(StringBuilder to, BorrowedReference ar
852860
to.Append(')');
853861
}
854862

855-
internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, MethodBase[]? methodinfo)
863+
internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, MethodBase[]? methodinfo, bool reverse_args = false)
856864
{
857865
// No valid methods, nothing to bind.
858866
if (GetMethods().Length == 0)
@@ -865,7 +873,7 @@ internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference a
865873
return Exceptions.RaiseTypeError(msg.ToString());
866874
}
867875

868-
Binding? binding = Bind(inst, args, kw, info, methodinfo);
876+
Binding? binding = Bind(inst, args, kw, info, methodinfo, reverse_args);
869877
object result;
870878
IntPtr ts = IntPtr.Zero;
871879

src/runtime/Types/MethodBinding.cs

+2-1
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,8 @@ public static NewReference tp_call(BorrowedReference ob, BorrowedReference args,
237237
}
238238
}
239239
}
240-
return self.m.Invoke(target is null ? BorrowedReference.Null : target, args, kw, self.info.UnsafeValue);
240+
241+
return self.m.Invoke(target is null ? BorrowedReference.Null : target, args, kw, self.info.UnsafeValue, self.m.binder.args_reversed);
241242
}
242243
finally
243244
{

src/runtime/Types/MethodObject.cs

+9-9
Original file line numberDiff line numberDiff line change
@@ -19,20 +19,20 @@ internal class MethodObject : ExtensionType
1919
{
2020
[NonSerialized]
2121
private MethodBase[]? _info = null;
22+
2223
private readonly List<MaybeMethodInfo> infoList;
2324
internal string name;
2425
internal readonly MethodBinder binder;
2526
internal bool is_static = false;
26-
2727
internal PyString? doc;
2828
internal MaybeType type;
2929

30-
public MethodObject(MaybeType type, string name, MethodBase[] info, bool allow_threads)
30+
public MethodObject(MaybeType type, string name, MethodBase[] info, bool allow_threads, bool reverse_args = false)
3131
{
3232
this.type = type;
3333
this.name = name;
3434
this.infoList = new List<MaybeMethodInfo>();
35-
binder = new MethodBinder();
35+
binder = new MethodBinder(reverse_args);
3636
foreach (MethodBase item in info)
3737
{
3838
this.infoList.Add(item);
@@ -45,8 +45,8 @@ public MethodObject(MaybeType type, string name, MethodBase[] info, bool allow_t
4545
binder.allow_threads = allow_threads;
4646
}
4747

48-
public MethodObject(MaybeType type, string name, MethodBase[] info)
49-
: this(type, name, info, allow_threads: AllowThreads(info))
48+
public MethodObject(MaybeType type, string name, MethodBase[] info, bool reverse_args = false)
49+
: this(type, name, info, allow_threads: AllowThreads(info), reverse_args)
5050
{
5151
}
5252

@@ -67,14 +67,14 @@ internal MethodBase[] info
6767
}
6868
}
6969

70-
public virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw)
70+
public virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, bool reverse_args = false)
7171
{
72-
return Invoke(inst, args, kw, null);
72+
return Invoke(inst, args, kw, null, reverse_args);
7373
}
7474

75-
public virtual NewReference Invoke(BorrowedReference target, BorrowedReference args, BorrowedReference kw, MethodBase? info)
75+
public virtual NewReference Invoke(BorrowedReference target, BorrowedReference args, BorrowedReference kw, MethodBase? info, bool reverse_args = false)
7676
{
77-
return binder.Invoke(target, args, kw, info, this.info);
77+
return binder.Invoke(target, args, kw, info, this.info, reverse_args);
7878
}
7979

8080
/// <summary>

src/runtime/Types/OperatorMethod.cs

+6-12
Original file line numberDiff line numberDiff line change
@@ -177,17 +177,14 @@ public static string ReversePyMethodName(string pyName)
177177
}
178178

179179
/// <summary>
180-
/// Check if the method is performing a reverse operation.
180+
/// Check if the method should have a reversed operation.
181181
/// </summary>
182182
/// <param name="method">The operator method.</param>
183183
/// <returns></returns>
184-
public static bool IsReverse(MethodBase method)
184+
public static bool HaveReverse(MethodBase method)
185185
{
186-
Type primaryType = method.IsOpsHelper()
187-
? method.DeclaringType.GetGenericArguments()[0]
188-
: method.DeclaringType;
189-
Type leftOperandType = method.GetParameters()[0].ParameterType;
190-
return leftOperandType != primaryType;
186+
var pi = method.GetParameters();
187+
return OpMethodMap.ContainsKey(method.Name) && pi.Length == 2;
191188
}
192189

193190
public static void FilterMethods(MethodBase[] methods, out MethodBase[] forwardMethods, out MethodBase[] reverseMethods)
@@ -196,14 +193,11 @@ public static void FilterMethods(MethodBase[] methods, out MethodBase[] forwardM
196193
var reverseMethodsList = new List<MethodBase>();
197194
foreach (var method in methods)
198195
{
199-
if (IsReverse(method))
196+
forwardMethodsList.Add(method);
197+
if (HaveReverse(method))
200198
{
201199
reverseMethodsList.Add(method);
202-
} else
203-
{
204-
forwardMethodsList.Add(method);
205200
}
206-
207201
}
208202
forwardMethods = forwardMethodsList.ToArray();
209203
reverseMethods = reverseMethodsList.ToArray();

0 commit comments

Comments
 (0)