From 5d551fbfe1897f4ede46d89a0dbe27553d255d1b Mon Sep 17 00:00:00 2001 From: Gert Dreyer Date: Fri, 23 Feb 2024 09:27:01 +0200 Subject: [PATCH 1/5] Tests for operators on type CS type with codec --- src/embed_tests/TestOperator.cs | 230 ++++++++++++++++++++++++++++++++ 1 file changed, 230 insertions(+) diff --git a/src/embed_tests/TestOperator.cs b/src/embed_tests/TestOperator.cs index a5713274a..1d66903ca 100644 --- a/src/embed_tests/TestOperator.cs +++ b/src/embed_tests/TestOperator.cs @@ -15,6 +15,7 @@ public class TestOperator public void SetUp() { PythonEngine.Initialize(); + OwnIntCodec.Setup(); } [OneTimeTearDown] @@ -23,6 +24,120 @@ public void Dispose() PythonEngine.Shutdown(); } + // Mock Integer class to test math ops on non-native dotnet types + public struct OwnInt + { + private int _value; + + public int Num => _value; + + public OwnInt() + { + _value = 0; + } + + public OwnInt(int value) + { + _value = value; + } + + public static OwnInt operator -(OwnInt p1, OwnInt p2) + { + return new OwnInt(p1._value - p2._value); + } + + public static OwnInt operator +(OwnInt p1, OwnInt p2) + { + return new OwnInt(p1._value + p2._value); + } + + public static OwnInt operator *(OwnInt p1, OwnInt p2) + { + return new OwnInt(p1._value * p2._value); + } + + public static OwnInt operator /(OwnInt p1, OwnInt p2) + { + return new OwnInt(p1._value / p2._value); + } + + public static OwnInt operator %(OwnInt p1, OwnInt p2) + { + return new OwnInt(p1._value % p2._value); + } + + public static OwnInt operator ^(OwnInt p1, OwnInt p2) + { + return new OwnInt(p1._value ^ p2._value); + } + + public static bool operator <(OwnInt p1, OwnInt p2) + { + return p1._value < p2._value; + } + + public static bool operator >(OwnInt p1, OwnInt p2) + { + return p1._value > p2._value; + } + + public static bool operator ==(OwnInt p1, OwnInt p2) + { + return p1._value == p2._value; + } + + public static bool operator !=(OwnInt p1, OwnInt p2) + { + return p1._value != p2._value; + } + + public static OwnInt operator |(OwnInt p1, OwnInt p2) + { + return new OwnInt(p1._value | p2._value); + } + + public static OwnInt operator &(OwnInt p1, OwnInt p2) + { + return new OwnInt(p1._value & p2._value); + } + + public static bool operator <=(OwnInt p1, OwnInt p2) + { + return p1._value <= p2._value; + } + + public static bool operator >=(OwnInt p1, OwnInt p2) + { + return p1._value >= p2._value; + } + } + + // Codec for mock class above. + public class OwnIntCodec : IPyObjectDecoder + { + public static void Setup() + { + PyObjectConversions.RegisterDecoder(new OwnIntCodec()); + } + + public bool CanDecode(PyType objectType, Type targetType) + { + return objectType.Name == "int" && targetType == typeof(OwnInt); + } + + public bool TryDecode(PyObject pyObj, out T? value) + { + if (pyObj.PyType.Name != "int" || typeof(T) != typeof(OwnInt)) + { + value = default(T); + return false; + } + + value = (T)(object)new OwnInt(pyObj.As()); + return true; + } + } + public class OperableObject { public int Num { get; set; } @@ -524,6 +639,121 @@ public void ShiftOperatorOverloads() c = a >> b.Num assert c.Num == a.Num >> b.Num +"); + } + + [Test] + public void ReverseOperatorWithCodec() + { + string name = string.Format("{0}.{1}", + typeof(OwnInt).DeclaringType.Name, + typeof(OwnInt).Name); + string module = MethodBase.GetCurrentMethod().DeclaringType.Namespace; + + PythonEngine.Exec($@" +from {module} import * +cls = {name} +a = 2 +b = cls(10) + +c = a + b +assert c.Num == a + b.Num + +c = a - b +assert c.Num == a - b.Num + +c = a * b +assert c.Num == a * b.Num + +c = a / b +assert c.Num == a // b.Num + +c = a % b +assert c.Num == a % b.Num + +c = a & b +assert c.Num == a & b.Num + +c = a | b +assert c.Num == a | b.Num + +c = a ^ b +assert c.Num == a ^ b.Num + +c = a == b +assert c == (a == b.Num) + +c = a != b +assert c == (a != b.Num) + +c = a <= b +assert c == (a <= b.Num) + +c = a >= b +assert c == (a >= b.Num) + +c = a < b +assert c == (a < b.Num) + +c = a > b +assert c == (a > b.Num) +"); + } + + [Test] + public void ForwardOperatorWithCodec() + { + string name = string.Format("{0}.{1}", + typeof(OwnInt).DeclaringType.Name, + typeof(OwnInt).Name); + string module = MethodBase.GetCurrentMethod().DeclaringType.Namespace; + + PythonEngine.Exec($@" +from {module} import * +cls = {name} +a = cls(2) +b = 10 +c = a + b +assert c.Num == a.Num + b + +c = a - b +assert c.Num == a.Num - b + +c = a * b +assert c.Num == a.Num * b + +c = a / b +assert c.Num == a.Num // b + +c = a % b +assert c.Num == a.Num % b + +c = a & b +assert c.Num == a.Num & b + +c = a | b +assert c.Num == a.Num | b + +c = a ^ b +assert c.Num == a.Num ^ b + +c = a == b +assert c == (a.Num == b) + +c = a != b +assert c == (a.Num != b) + +c = a <= b +assert c == (a.Num <= b) + +c = a >= b +assert c == (a.Num >= b) + +c = a < b +assert c == (a.Num < b) + +c = a > b +assert c == (a.Num > b) "); } } From e7a3aba0733ab3b76ac7c87815df7b06ad057ab8 Mon Sep 17 00:00:00 2001 From: Gert Dreyer Date: Wed, 21 Feb 2024 22:07:18 +0200 Subject: [PATCH 2/5] Bugfix: RecursionError when reverse/righthand operations invoked. e.g. __rsub__, __rmul__ --- src/runtime/ClassManager.cs | 2 +- src/runtime/MethodBinder.cs | 40 +++++++++++++++++------------ src/runtime/Types/MethodBinding.cs | 3 ++- src/runtime/Types/MethodObject.cs | 18 ++++++------- src/runtime/Types/OperatorMethod.cs | 18 +++++-------- 5 files changed, 42 insertions(+), 39 deletions(-) diff --git a/src/runtime/ClassManager.cs b/src/runtime/ClassManager.cs index 79ab20e82..25f8639ab 100644 --- a/src/runtime/ClassManager.cs +++ b/src/runtime/ClassManager.cs @@ -546,7 +546,7 @@ private static ClassInfo GetClassInfo(Type type, ClassBase impl) ci.members[pyName] = new MethodObject(type, name, forwardMethods).AllocObject(); // Only methods where only the right operand is the declaring type. if (reverseMethods.Length > 0) - ci.members[pyNameReverse] = new MethodObject(type, name, reverseMethods).AllocObject(); + ci.members[pyNameReverse] = new MethodObject(type, name, reverseMethods, reverse_args: true).AllocObject(); } } diff --git a/src/runtime/MethodBinder.cs b/src/runtime/MethodBinder.cs index 07ed4fe22..18ef573d0 100644 --- a/src/runtime/MethodBinder.cs +++ b/src/runtime/MethodBinder.cs @@ -28,17 +28,22 @@ internal class MethodBinder [NonSerialized] public bool init = false; + public const bool DefaultAllowThreads = true; public bool allow_threads = DefaultAllowThreads; - internal MethodBinder() + public bool args_reversed = false; + + internal MethodBinder(bool reverse_args = false) { list = new List(); + args_reversed = reverse_args; } - internal MethodBinder(MethodInfo mi) + internal MethodBinder(MethodInfo mi, bool reverse_args = false) { list = new List { new MaybeMethodBase(mi) }; + args_reversed = reverse_args; } public int Count @@ -271,10 +276,11 @@ internal static int ArgPrecedence(Type t) /// The Python target of the method invocation. /// The Python arguments. /// The Python keyword arguments. + /// Reverse arguments of methods. Used for methods such as __radd__, __rsub__, __rmod__ etc /// A Binding if successful. Otherwise null. - internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw) + internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, bool reverse_args = false) { - return Bind(inst, args, kw, null, null); + return Bind(inst, args, kw, null, null, reverse_args); } /// @@ -287,10 +293,11 @@ internal static int ArgPrecedence(Type t) /// The Python arguments. /// The Python keyword arguments. /// If not null, only bind to that method. + /// Reverse arguments of methods. Used for methods such as __radd__, __rsub__, __rmod__ etc /// A Binding if successful. Otherwise null. - internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info) + internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, bool reverse_args = false) { - return Bind(inst, args, kw, info, null); + return Bind(inst, args, kw, info, null, reverse_args); } private readonly struct MatchedMethod @@ -334,8 +341,9 @@ public MismatchedMethod(Exception exception, MethodBase mb) /// The Python keyword arguments. /// If not null, only bind to that method. /// If not null, additionally attempt to bind to the generic methods in this array by inferring generic type parameters. + /// Reverse arguments of methods. Used for methods such as __radd__, __rsub__, __rmod__ etc /// A Binding if successful. Otherwise null. - internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, MethodBase[]? methodinfo) + internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, MethodBase[]? methodinfo, bool reverse_args = false) { // loop to find match, return invoker w/ or w/o error var kwargDict = new Dictionary(); @@ -363,10 +371,10 @@ public MismatchedMethod(Exception exception, MethodBase mb) _methods = GetMethods(); } - return Bind(inst, args, kwargDict, _methods, matchGenerics: true); + return Bind(inst, args, kwargDict, _methods, matchGenerics: true, reverse_args); } - static Binding? Bind(BorrowedReference inst, BorrowedReference args, Dictionary kwargDict, MethodBase[] methods, bool matchGenerics) + private static Binding? Bind(BorrowedReference inst, BorrowedReference args, Dictionary kwargDict, MethodBase[] methods, bool matchGenerics, bool reversed = false) { var pynargs = (int)Runtime.PyTuple_Size(args); var isGeneric = false; @@ -386,7 +394,7 @@ public MismatchedMethod(Exception exception, MethodBase mb) // Binary operator methods will have 2 CLR args but only one Python arg // (unary operators will have 1 less each), since Python operator methods are bound. isOperator = isOperator && pynargs == pi.Length - 1; - bool isReverse = isOperator && OperatorMethod.IsReverse((MethodInfo)mi); // Only cast if isOperator. + bool isReverse = isOperator && reversed; // Only cast if isOperator. if (isReverse && OperatorMethod.IsComparisonOp((MethodInfo)mi)) continue; // Comparison operators in Python have no reverse mode. 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 return match; } - internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw) + internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, bool reverse_args = false) { - return Invoke(inst, args, kw, null, null); + return Invoke(inst, args, kw, null, null, reverse_args); } - internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info) + internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, bool reverse_args = false) { - return Invoke(inst, args, kw, info, null); + return Invoke(inst, args, kw, info, null, reverse_args = false); } protected static void AppendArgumentTypes(StringBuilder to, BorrowedReference args) @@ -852,7 +860,7 @@ protected static void AppendArgumentTypes(StringBuilder to, BorrowedReference ar to.Append(')'); } - internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, MethodBase[]? methodinfo) + internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, MethodBase[]? methodinfo, bool reverse_args = false) { // No valid methods, nothing to bind. if (GetMethods().Length == 0) @@ -865,7 +873,7 @@ internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference a return Exceptions.RaiseTypeError(msg.ToString()); } - Binding? binding = Bind(inst, args, kw, info, methodinfo); + Binding? binding = Bind(inst, args, kw, info, methodinfo, reverse_args); object result; IntPtr ts = IntPtr.Zero; diff --git a/src/runtime/Types/MethodBinding.cs b/src/runtime/Types/MethodBinding.cs index 334d705a6..2f943f3fb 100644 --- a/src/runtime/Types/MethodBinding.cs +++ b/src/runtime/Types/MethodBinding.cs @@ -237,7 +237,8 @@ public static NewReference tp_call(BorrowedReference ob, BorrowedReference args, } } } - return self.m.Invoke(target is null ? BorrowedReference.Null : target, args, kw, self.info.UnsafeValue); + + return self.m.Invoke(target is null ? BorrowedReference.Null : target, args, kw, self.info.UnsafeValue, self.m.binder.args_reversed); } finally { diff --git a/src/runtime/Types/MethodObject.cs b/src/runtime/Types/MethodObject.cs index 05198da76..1943ed884 100644 --- a/src/runtime/Types/MethodObject.cs +++ b/src/runtime/Types/MethodObject.cs @@ -19,20 +19,20 @@ internal class MethodObject : ExtensionType { [NonSerialized] private MethodBase[]? _info = null; + private readonly List infoList; internal string name; internal readonly MethodBinder binder; internal bool is_static = false; - internal PyString? doc; internal MaybeType type; - public MethodObject(MaybeType type, string name, MethodBase[] info, bool allow_threads) + public MethodObject(MaybeType type, string name, MethodBase[] info, bool allow_threads, bool reverse_args = false) { this.type = type; this.name = name; this.infoList = new List(); - binder = new MethodBinder(); + binder = new MethodBinder(reverse_args); foreach (MethodBase item in info) { this.infoList.Add(item); @@ -45,8 +45,8 @@ public MethodObject(MaybeType type, string name, MethodBase[] info, bool allow_t binder.allow_threads = allow_threads; } - public MethodObject(MaybeType type, string name, MethodBase[] info) - : this(type, name, info, allow_threads: AllowThreads(info)) + public MethodObject(MaybeType type, string name, MethodBase[] info, bool reverse_args = false) + : this(type, name, info, allow_threads: AllowThreads(info), reverse_args) { } @@ -67,14 +67,14 @@ internal MethodBase[] info } } - public virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw) + public virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, bool reverse_args = false) { - return Invoke(inst, args, kw, null); + return Invoke(inst, args, kw, null, reverse_args); } - public virtual NewReference Invoke(BorrowedReference target, BorrowedReference args, BorrowedReference kw, MethodBase? info) + public virtual NewReference Invoke(BorrowedReference target, BorrowedReference args, BorrowedReference kw, MethodBase? info, bool reverse_args = false) { - return binder.Invoke(target, args, kw, info, this.info); + return binder.Invoke(target, args, kw, info, this.info, reverse_args); } /// diff --git a/src/runtime/Types/OperatorMethod.cs b/src/runtime/Types/OperatorMethod.cs index e3cc23370..a2ca73982 100644 --- a/src/runtime/Types/OperatorMethod.cs +++ b/src/runtime/Types/OperatorMethod.cs @@ -177,17 +177,14 @@ public static string ReversePyMethodName(string pyName) } /// - /// Check if the method is performing a reverse operation. + /// Check if the method should have a reversed operation. /// /// The operator method. /// - public static bool IsReverse(MethodBase method) + public static bool HaveReverse(MethodBase method) { - Type primaryType = method.IsOpsHelper() - ? method.DeclaringType.GetGenericArguments()[0] - : method.DeclaringType; - Type leftOperandType = method.GetParameters()[0].ParameterType; - return leftOperandType != primaryType; + var pi = method.GetParameters(); + return OpMethodMap.ContainsKey(method.Name) && pi.Length == 2; } 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 var reverseMethodsList = new List(); foreach (var method in methods) { - if (IsReverse(method)) + forwardMethodsList.Add(method); + if (HaveReverse(method)) { reverseMethodsList.Add(method); - } else - { - forwardMethodsList.Add(method); } - } forwardMethods = forwardMethodsList.ToArray(); reverseMethods = reverseMethodsList.ToArray(); From c2567464345e24fd7e0de9ea255742481d4ac2e2 Mon Sep 17 00:00:00 2001 From: Gert Dreyer Date: Fri, 23 Feb 2024 15:33:44 +0200 Subject: [PATCH 3/5] Update Authors and Changelog --- AUTHORS.md | 1 + CHANGELOG.md | 1 + 2 files changed, 2 insertions(+) diff --git a/AUTHORS.md b/AUTHORS.md index 577e898aa..18435671c 100644 --- a/AUTHORS.md +++ b/AUTHORS.md @@ -86,3 +86,4 @@ - ([@gpetrou](https://github.com/gpetrou)) - Ehsan Iran-Nejad ([@eirannejad](https://github.com/eirannejad)) - ([@legomanww](https://github.com/legomanww)) +- ([@gertdreyer](https://github.com/gertdreyer)) diff --git a/CHANGELOG.md b/CHANGELOG.md index fdab9bf64..5b545045f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ This document follows the conventions laid out in [Keep a CHANGELOG][]. ### Fixed +- Fixed RecursionError for reverse operators on C# operable types from python. See #2240 ## [3.0.3](https://github.com/pythonnet/pythonnet/releases/tag/v3.0.3) - 2023-10-11 From 5277ed97f83f8d5907ecb2376d36a7e7205fea85 Mon Sep 17 00:00:00 2001 From: Gert Dreyer Date: Mon, 26 Feb 2024 13:11:41 +0200 Subject: [PATCH 4/5] Normalized names. Added HashCode and Equals to testing objects --- src/embed_tests/TestOperator.cs | 19 +++++++----- src/runtime/ClassManager.cs | 2 +- src/runtime/MethodBinder.cs | 50 ++++++++++++++++-------------- src/runtime/Types/MethodBinding.cs | 2 +- src/runtime/Types/MethodObject.cs | 16 +++++----- 5 files changed, 48 insertions(+), 41 deletions(-) diff --git a/src/embed_tests/TestOperator.cs b/src/embed_tests/TestOperator.cs index 1d66903ca..1ec3268ac 100644 --- a/src/embed_tests/TestOperator.cs +++ b/src/embed_tests/TestOperator.cs @@ -41,6 +41,17 @@ public OwnInt(int value) _value = value; } + public override int GetHashCode() + { + return unchecked(65535 + _value.GetHashCode()); + } + + public override bool Equals(object obj) + { + return obj is OwnInt @object && + Num == @object.Num; + } + public static OwnInt operator -(OwnInt p1, OwnInt p2) { return new OwnInt(p1._value - p2._value); @@ -125,14 +136,8 @@ public bool CanDecode(PyType objectType, Type targetType) return objectType.Name == "int" && targetType == typeof(OwnInt); } - public bool TryDecode(PyObject pyObj, out T? value) + public bool TryDecode(PyObject pyObj, out T value) { - if (pyObj.PyType.Name != "int" || typeof(T) != typeof(OwnInt)) - { - value = default(T); - return false; - } - value = (T)(object)new OwnInt(pyObj.As()); return true; } diff --git a/src/runtime/ClassManager.cs b/src/runtime/ClassManager.cs index 25f8639ab..ecb6055a8 100644 --- a/src/runtime/ClassManager.cs +++ b/src/runtime/ClassManager.cs @@ -546,7 +546,7 @@ private static ClassInfo GetClassInfo(Type type, ClassBase impl) ci.members[pyName] = new MethodObject(type, name, forwardMethods).AllocObject(); // Only methods where only the right operand is the declaring type. if (reverseMethods.Length > 0) - ci.members[pyNameReverse] = new MethodObject(type, name, reverseMethods, reverse_args: true).AllocObject(); + ci.members[pyNameReverse] = new MethodObject(type, name, reverseMethods, argsReversed: true).AllocObject(); } } diff --git a/src/runtime/MethodBinder.cs b/src/runtime/MethodBinder.cs index 18ef573d0..836e1da3e 100644 --- a/src/runtime/MethodBinder.cs +++ b/src/runtime/MethodBinder.cs @@ -32,18 +32,18 @@ internal class MethodBinder public const bool DefaultAllowThreads = true; public bool allow_threads = DefaultAllowThreads; - public bool args_reversed = false; + public bool argsReversed = false; - internal MethodBinder(bool reverse_args = false) + internal MethodBinder(bool argsReversed = false) { list = new List(); - args_reversed = reverse_args; + this.argsReversed = argsReversed; } - internal MethodBinder(MethodInfo mi, bool reverse_args = false) + internal MethodBinder(MethodInfo mi, bool argsReversed = false) { list = new List { new MaybeMethodBase(mi) }; - args_reversed = reverse_args; + this.argsReversed = argsReversed; } public int Count @@ -276,11 +276,11 @@ internal static int ArgPrecedence(Type t) /// The Python target of the method invocation. /// The Python arguments. /// The Python keyword arguments. - /// Reverse arguments of methods. Used for methods such as __radd__, __rsub__, __rmod__ etc + /// Reverse arguments of methods. Used for methods such as __radd__, __rsub__, __rmod__ etc /// A Binding if successful. Otherwise null. - internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, bool reverse_args = false) + internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, bool argsReversed = false) { - return Bind(inst, args, kw, null, null, reverse_args); + return Bind(inst, args, kw, null, null, argsReversed); } /// @@ -293,11 +293,11 @@ internal static int ArgPrecedence(Type t) /// The Python arguments. /// The Python keyword arguments. /// If not null, only bind to that method. - /// Reverse arguments of methods. Used for methods such as __radd__, __rsub__, __rmod__ etc + /// Reverse arguments of methods. Used for methods such as __radd__, __rsub__, __rmod__ etc /// A Binding if successful. Otherwise null. - internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, bool reverse_args = false) + internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, bool argsReversed = false) { - return Bind(inst, args, kw, info, null, reverse_args); + return Bind(inst, args, kw, info, null, argsReversed); } private readonly struct MatchedMethod @@ -341,9 +341,9 @@ public MismatchedMethod(Exception exception, MethodBase mb) /// The Python keyword arguments. /// If not null, only bind to that method. /// If not null, additionally attempt to bind to the generic methods in this array by inferring generic type parameters. - /// Reverse arguments of methods. Used for methods such as __radd__, __rsub__, __rmod__ etc + /// Reverse arguments of methods. Used for methods such as __radd__, __rsub__, __rmod__ etc /// A Binding if successful. Otherwise null. - internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, MethodBase[]? methodinfo, bool reverse_args = false) + internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, MethodBase[]? methodinfo, bool argsReversed = false) { // loop to find match, return invoker w/ or w/o error var kwargDict = new Dictionary(); @@ -371,10 +371,10 @@ public MismatchedMethod(Exception exception, MethodBase mb) _methods = GetMethods(); } - return Bind(inst, args, kwargDict, _methods, matchGenerics: true, reverse_args); + return Bind(inst, args, kwargDict, _methods, matchGenerics: true, argsReversed); } - private static Binding? Bind(BorrowedReference inst, BorrowedReference args, Dictionary kwargDict, MethodBase[] methods, bool matchGenerics, bool reversed = false) + private static Binding? Bind(BorrowedReference inst, BorrowedReference args, Dictionary kwargDict, MethodBase[] methods, bool matchGenerics, bool argsReversed = false) { var pynargs = (int)Runtime.PyTuple_Size(args); var isGeneric = false; @@ -394,7 +394,7 @@ public MismatchedMethod(Exception exception, MethodBase mb) // Binary operator methods will have 2 CLR args but only one Python arg // (unary operators will have 1 less each), since Python operator methods are bound. isOperator = isOperator && pynargs == pi.Length - 1; - bool isReverse = isOperator && reversed; // Only cast if isOperator. + bool isReverse = isOperator && argsReversed; // Only cast if isOperator. if (isReverse && OperatorMethod.IsComparisonOp((MethodInfo)mi)) continue; // Comparison operators in Python have no reverse mode. if (!MatchesArgumentCount(pynargs, pi, kwargDict, out bool paramsArray, out ArrayList? defaultArgList, out int kwargsMatched, out int defaultsNeeded) && !isOperator) @@ -402,12 +402,14 @@ public MismatchedMethod(Exception exception, MethodBase mb) continue; } // Preprocessing pi to remove either the first or second argument. - if (isOperator && !isReverse) { + if (isOperator && !isReverse) + { // The first Python arg is the right operand, while the bound instance is the left. // We need to skip the first (left operand) CLR argument. pi = pi.Skip(1).ToArray(); } - else if (isOperator && isReverse) { + else if (isOperator && isReverse) + { // The first Python arg is the left operand. // We need to take the first CLR argument. pi = pi.Take(1).ToArray(); @@ -817,14 +819,14 @@ static bool MatchesArgumentCount(int positionalArgumentCount, ParameterInfo[] pa return match; } - internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, bool reverse_args = false) + internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, bool argsReversed = false) { - return Invoke(inst, args, kw, null, null, reverse_args); + return Invoke(inst, args, kw, null, null, argsReversed); } - internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, bool reverse_args = false) + internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, bool argsReversed = false) { - return Invoke(inst, args, kw, info, null, reverse_args = false); + return Invoke(inst, args, kw, info, null, argsReversed = false); } protected static void AppendArgumentTypes(StringBuilder to, BorrowedReference args) @@ -860,7 +862,7 @@ protected static void AppendArgumentTypes(StringBuilder to, BorrowedReference ar to.Append(')'); } - internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, MethodBase[]? methodinfo, bool reverse_args = false) + internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, MethodBase[]? methodinfo, bool argsReversed = false) { // No valid methods, nothing to bind. if (GetMethods().Length == 0) @@ -873,7 +875,7 @@ internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference a return Exceptions.RaiseTypeError(msg.ToString()); } - Binding? binding = Bind(inst, args, kw, info, methodinfo, reverse_args); + Binding? binding = Bind(inst, args, kw, info, methodinfo, argsReversed); object result; IntPtr ts = IntPtr.Zero; diff --git a/src/runtime/Types/MethodBinding.cs b/src/runtime/Types/MethodBinding.cs index 2f943f3fb..79607d1ae 100644 --- a/src/runtime/Types/MethodBinding.cs +++ b/src/runtime/Types/MethodBinding.cs @@ -238,7 +238,7 @@ public static NewReference tp_call(BorrowedReference ob, BorrowedReference args, } } - return self.m.Invoke(target is null ? BorrowedReference.Null : target, args, kw, self.info.UnsafeValue, self.m.binder.args_reversed); + return self.m.Invoke(target is null ? BorrowedReference.Null : target, args, kw, self.info.UnsafeValue, self.m.binder.argsReversed); } finally { diff --git a/src/runtime/Types/MethodObject.cs b/src/runtime/Types/MethodObject.cs index 1943ed884..4bc21458b 100644 --- a/src/runtime/Types/MethodObject.cs +++ b/src/runtime/Types/MethodObject.cs @@ -27,12 +27,12 @@ internal class MethodObject : ExtensionType internal PyString? doc; internal MaybeType type; - public MethodObject(MaybeType type, string name, MethodBase[] info, bool allow_threads, bool reverse_args = false) + public MethodObject(MaybeType type, string name, MethodBase[] info, bool allow_threads, bool argsReversed = false) { this.type = type; this.name = name; this.infoList = new List(); - binder = new MethodBinder(reverse_args); + binder = new MethodBinder(argsReversed); foreach (MethodBase item in info) { this.infoList.Add(item); @@ -45,8 +45,8 @@ public MethodObject(MaybeType type, string name, MethodBase[] info, bool allow_t binder.allow_threads = allow_threads; } - public MethodObject(MaybeType type, string name, MethodBase[] info, bool reverse_args = false) - : this(type, name, info, allow_threads: AllowThreads(info), reverse_args) + public MethodObject(MaybeType type, string name, MethodBase[] info, bool argsReversed = false) + : this(type, name, info, allow_threads: AllowThreads(info), argsReversed) { } @@ -67,14 +67,14 @@ internal MethodBase[] info } } - public virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, bool reverse_args = false) + public virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, bool argsReversed = false) { - return Invoke(inst, args, kw, null, reverse_args); + return Invoke(inst, args, kw, null, argsReversed); } - public virtual NewReference Invoke(BorrowedReference target, BorrowedReference args, BorrowedReference kw, MethodBase? info, bool reverse_args = false) + public virtual NewReference Invoke(BorrowedReference target, BorrowedReference args, BorrowedReference kw, MethodBase? info, bool argsReversed = false) { - return binder.Invoke(target, args, kw, info, this.info, reverse_args); + return binder.Invoke(target, args, kw, info, this.info, argsReversed); } /// From ed2505c412b0640151d2e61807cd2369d71e5f73 Mon Sep 17 00:00:00 2001 From: Gert Dreyer Date: Mon, 26 Feb 2024 20:49:55 +0200 Subject: [PATCH 5/5] Cleanup Codec/Argument Reversing Passing. --- src/embed_tests/TestOperator.cs | 4 ++-- src/runtime/MethodBinder.cs | 31 +++++++++++++----------------- src/runtime/Types/MethodBinding.cs | 2 +- src/runtime/Types/MethodObject.cs | 10 +++++----- 4 files changed, 21 insertions(+), 26 deletions(-) diff --git a/src/embed_tests/TestOperator.cs b/src/embed_tests/TestOperator.cs index 1ec3268ac..6bfb81bdb 100644 --- a/src/embed_tests/TestOperator.cs +++ b/src/embed_tests/TestOperator.cs @@ -25,9 +25,9 @@ public void Dispose() } // Mock Integer class to test math ops on non-native dotnet types - public struct OwnInt + public readonly struct OwnInt { - private int _value; + private readonly int _value; public int Num => _value; diff --git a/src/runtime/MethodBinder.cs b/src/runtime/MethodBinder.cs index 836e1da3e..9a5515c8e 100644 --- a/src/runtime/MethodBinder.cs +++ b/src/runtime/MethodBinder.cs @@ -34,16 +34,14 @@ internal class MethodBinder public bool argsReversed = false; - internal MethodBinder(bool argsReversed = false) + internal MethodBinder() { list = new List(); - this.argsReversed = argsReversed; } - internal MethodBinder(MethodInfo mi, bool argsReversed = false) + internal MethodBinder(MethodInfo mi) { list = new List { new MaybeMethodBase(mi) }; - this.argsReversed = argsReversed; } public int Count @@ -276,11 +274,10 @@ internal static int ArgPrecedence(Type t) /// The Python target of the method invocation. /// The Python arguments. /// The Python keyword arguments. - /// Reverse arguments of methods. Used for methods such as __radd__, __rsub__, __rmod__ etc /// A Binding if successful. Otherwise null. - internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, bool argsReversed = false) + internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw) { - return Bind(inst, args, kw, null, null, argsReversed); + return Bind(inst, args, kw, null, null); } /// @@ -293,11 +290,10 @@ internal static int ArgPrecedence(Type t) /// The Python arguments. /// The Python keyword arguments. /// If not null, only bind to that method. - /// Reverse arguments of methods. Used for methods such as __radd__, __rsub__, __rmod__ etc /// A Binding if successful. Otherwise null. - internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, bool argsReversed = false) + internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info) { - return Bind(inst, args, kw, info, null, argsReversed); + return Bind(inst, args, kw, info, null); } private readonly struct MatchedMethod @@ -341,9 +337,8 @@ public MismatchedMethod(Exception exception, MethodBase mb) /// The Python keyword arguments. /// If not null, only bind to that method. /// If not null, additionally attempt to bind to the generic methods in this array by inferring generic type parameters. - /// Reverse arguments of methods. Used for methods such as __radd__, __rsub__, __rmod__ etc /// A Binding if successful. Otherwise null. - internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, MethodBase[]? methodinfo, bool argsReversed = false) + internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, MethodBase[]? methodinfo) { // loop to find match, return invoker w/ or w/o error var kwargDict = new Dictionary(); @@ -819,14 +814,14 @@ static bool MatchesArgumentCount(int positionalArgumentCount, ParameterInfo[] pa return match; } - internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, bool argsReversed = false) + internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw) { - return Invoke(inst, args, kw, null, null, argsReversed); + return Invoke(inst, args, kw, null, null); } - internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, bool argsReversed = false) + internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info) { - return Invoke(inst, args, kw, info, null, argsReversed = false); + return Invoke(inst, args, kw, info, null); } protected static void AppendArgumentTypes(StringBuilder to, BorrowedReference args) @@ -862,7 +857,7 @@ protected static void AppendArgumentTypes(StringBuilder to, BorrowedReference ar to.Append(')'); } - internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, MethodBase[]? methodinfo, bool argsReversed = false) + internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, MethodBase[]? methodinfo) { // No valid methods, nothing to bind. if (GetMethods().Length == 0) @@ -875,7 +870,7 @@ internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference a return Exceptions.RaiseTypeError(msg.ToString()); } - Binding? binding = Bind(inst, args, kw, info, methodinfo, argsReversed); + Binding? binding = Bind(inst, args, kw, info, methodinfo); object result; IntPtr ts = IntPtr.Zero; diff --git a/src/runtime/Types/MethodBinding.cs b/src/runtime/Types/MethodBinding.cs index 79607d1ae..bfe22b0f3 100644 --- a/src/runtime/Types/MethodBinding.cs +++ b/src/runtime/Types/MethodBinding.cs @@ -238,7 +238,7 @@ public static NewReference tp_call(BorrowedReference ob, BorrowedReference args, } } - return self.m.Invoke(target is null ? BorrowedReference.Null : target, args, kw, self.info.UnsafeValue, self.m.binder.argsReversed); + return self.m.Invoke(target is null ? BorrowedReference.Null : target, args, kw, self.info.UnsafeValue); } finally { diff --git a/src/runtime/Types/MethodObject.cs b/src/runtime/Types/MethodObject.cs index 4bc21458b..12484d301 100644 --- a/src/runtime/Types/MethodObject.cs +++ b/src/runtime/Types/MethodObject.cs @@ -32,7 +32,7 @@ public MethodObject(MaybeType type, string name, MethodBase[] info, bool allow_t this.type = type; this.name = name; this.infoList = new List(); - binder = new MethodBinder(argsReversed); + binder = new MethodBinder() { argsReversed = argsReversed }; foreach (MethodBase item in info) { this.infoList.Add(item); @@ -67,14 +67,14 @@ internal MethodBase[] info } } - public virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, bool argsReversed = false) + public virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw) { - return Invoke(inst, args, kw, null, argsReversed); + return Invoke(inst, args, kw, null); } - public virtual NewReference Invoke(BorrowedReference target, BorrowedReference args, BorrowedReference kw, MethodBase? info, bool argsReversed = false) + public virtual NewReference Invoke(BorrowedReference target, BorrowedReference args, BorrowedReference kw, MethodBase? info) { - return binder.Invoke(target, args, kw, info, this.info, argsReversed); + return binder.Invoke(target, args, kw, info, this.info); } ///