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 diff --git a/src/embed_tests/TestOperator.cs b/src/embed_tests/TestOperator.cs index a5713274a..6bfb81bdb 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,125 @@ public void Dispose() PythonEngine.Shutdown(); } + // Mock Integer class to test math ops on non-native dotnet types + public readonly struct OwnInt + { + private readonly int _value; + + public int Num => _value; + + public OwnInt() + { + _value = 0; + } + + 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); + } + + 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) + { + value = (T)(object)new OwnInt(pyObj.As()); + return true; + } + } + public class OperableObject { public int Num { get; set; } @@ -524,6 +644,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) "); } } diff --git a/src/runtime/ClassManager.cs b/src/runtime/ClassManager.cs index 79ab20e82..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).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 07ed4fe22..9a5515c8e 100644 --- a/src/runtime/MethodBinder.cs +++ b/src/runtime/MethodBinder.cs @@ -28,9 +28,12 @@ internal class MethodBinder [NonSerialized] public bool init = false; + public const bool DefaultAllowThreads = true; public bool allow_threads = DefaultAllowThreads; + public bool argsReversed = false; + internal MethodBinder() { list = new List(); @@ -363,10 +366,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, argsReversed); } - 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 argsReversed = false) { var pynargs = (int)Runtime.PyTuple_Size(args); var isGeneric = false; @@ -386,7 +389,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 && 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) @@ -394,12 +397,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(); diff --git a/src/runtime/Types/MethodBinding.cs b/src/runtime/Types/MethodBinding.cs index 334d705a6..bfe22b0f3 100644 --- a/src/runtime/Types/MethodBinding.cs +++ b/src/runtime/Types/MethodBinding.cs @@ -237,6 +237,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); } finally diff --git a/src/runtime/Types/MethodObject.cs b/src/runtime/Types/MethodObject.cs index 05198da76..12484d301 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 argsReversed = false) { this.type = type; this.name = name; this.infoList = new List(); - binder = new MethodBinder(); + binder = new MethodBinder() { argsReversed = 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) - : this(type, name, info, allow_threads: AllowThreads(info)) + public MethodObject(MaybeType type, string name, MethodBase[] info, bool argsReversed = false) + : this(type, name, info, allow_threads: AllowThreads(info), argsReversed) { } 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();