From 9a8cb966faf8a7cdaa10dbf5db05d465b2760189 Mon Sep 17 00:00:00 2001 From: Christabella Irwanto Date: Tue, 29 Dec 2020 13:17:27 +0000 Subject: [PATCH 01/12] Add comparison operators --- src/runtime/operatormethod.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/runtime/operatormethod.cs b/src/runtime/operatormethod.cs index 1e0244510..6b0ee1926 100644 --- a/src/runtime/operatormethod.cs +++ b/src/runtime/operatormethod.cs @@ -49,6 +49,11 @@ static OperatorMethod() ["op_OnesComplement"] = new SlotDefinition("__invert__", TypeOffset.nb_invert), ["op_UnaryNegation"] = new SlotDefinition("__neg__", TypeOffset.nb_negative), ["op_UnaryPlus"] = new SlotDefinition("__pos__", TypeOffset.nb_positive), + ["op_OneComplement"] = new SlotDefinition("__invert__", TypeOffset.nb_invert), + ["op_GreaterThan"] = new SlotDefinition("__gt__", TypeOffset.tp_richcompare), + ["op_GreaterThanOrEqual"] = new SlotDefinition("__ge__", TypeOffset.tp_richcompare), + ["op_LessThan"] = new SlotDefinition("__lt__", TypeOffset.tp_richcompare), + ["op_LessThanOrEqual"] = new SlotDefinition("__le__", TypeOffset.tp_richcompare) }; } From bcbace35ba5ad003c8eb6336be81e4302397efdf Mon Sep 17 00:00:00 2001 From: Christabella Irwanto Date: Tue, 29 Dec 2020 13:43:20 +0000 Subject: [PATCH 02/12] Add tests for logical (bool) comparison operators --- src/embed_tests/TestOperator.cs | 90 ++++++++++++++++++++++++++++++++- 1 file changed, 89 insertions(+), 1 deletion(-) diff --git a/src/embed_tests/TestOperator.cs b/src/embed_tests/TestOperator.cs index ecdb0c1dc..c5548b72f 100644 --- a/src/embed_tests/TestOperator.cs +++ b/src/embed_tests/TestOperator.cs @@ -149,6 +149,58 @@ public OperableObject(int num) return new OperableObject(a.Num ^ b); } + public static bool operator <=(int a, OperableObject b) + { + return (a <= b.Num); + } + public static bool operator <=(OperableObject a, OperableObject b) + { + return (a.Num <= b.Num); + } + public static bool operator <=(OperableObject a, int b) + { + return (a.Num <= b); + } + + public static bool operator >=(int a, OperableObject b) + { + return (a >= b.Num); + } + public static bool operator >=(OperableObject a, OperableObject b) + { + return (a.Num >= b.Num); + } + public static bool operator >=(OperableObject a, int b) + { + return (a.Num >= b); + } + + public static bool operator<(int a, OperableObject b) + { + return (a < b.Num); + } + public static bool operator<(OperableObject a, OperableObject b) + { + return (a.Num < b.Num); + } + public static bool operator<(OperableObject a, int b) + { + return (a.Num < b); + } + + public static bool operator>(int a, OperableObject b) + { + return (a > b.Num); + } + public static bool operator>(OperableObject a, OperableObject b) + { + return (a.Num > b.Num); + } + public static bool operator >(OperableObject a, int b) + { + return (a.Num > b); + } + public static OperableObject operator <<(OperableObject a, int offset) { return new OperableObject(a.Num << offset); @@ -161,7 +213,7 @@ public OperableObject(int num) } [Test] - public void OperatorOverloads() + public void SymmetricalOperatorOverloads() { string name = string.Format("{0}.{1}", typeof(OperableObject).DeclaringType.Name, @@ -206,6 +258,18 @@ public void OperatorOverloads() c = a ^ b assert c.Num == a.Num ^ b.Num + +c = a <= b +assert c == (a.Num <= b.Num) + +c = a >= b +assert c == (a.Num >= b.Num) + +c = a < b +assert c == (a.Num < b.Num) + +c = a > b +assert c == (a.Num > b.Num) "); } @@ -263,6 +327,18 @@ public void ForwardOperatorOverloads() 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) "); } @@ -304,6 +380,18 @@ public void ReverseOperatorOverloads() 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) "); } From 31f3ed1309ddb7358f0a7ec315bdd006d048aaad Mon Sep 17 00:00:00 2001 From: Christabella Irwanto Date: Wed, 6 Jan 2021 17:46:28 +0200 Subject: [PATCH 03/12] Finish comparison operator impl and add tests --- src/embed_tests/TestOperator.cs | 18 +++++++++++++ src/runtime/classbase.cs | 46 +++++++++++++++++++++++++++++++++ src/runtime/classmanager.cs | 4 +++ src/runtime/operatormethod.cs | 10 ++++++- 4 files changed, 77 insertions(+), 1 deletion(-) diff --git a/src/embed_tests/TestOperator.cs b/src/embed_tests/TestOperator.cs index c5548b72f..37be36206 100644 --- a/src/embed_tests/TestOperator.cs +++ b/src/embed_tests/TestOperator.cs @@ -270,6 +270,12 @@ public void SymmetricalOperatorOverloads() c = a > b assert c == (a.Num > b.Num) + +c = a == b +assert c == (a.Num == b.Num) + +c = a != b +assert c == (a.Num != b.Num) "); } @@ -339,6 +345,12 @@ public void ForwardOperatorOverloads() c = a > b assert c == (a.Num > b) + +c = a == b +assert c == (a.Num == b) + +c = a != b +assert c == (a.Num != b) "); } @@ -392,6 +404,12 @@ public void ReverseOperatorOverloads() c = a > b assert c == (a > b.Num) + +c = a == b +assert c == (a == b.Num) + +c = a != b +assert c == (a != b.Num) "); } diff --git a/src/runtime/classbase.cs b/src/runtime/classbase.cs index 0ff4ba154..b3c5d8c4d 100644 --- a/src/runtime/classbase.cs +++ b/src/runtime/classbase.cs @@ -21,6 +21,7 @@ internal class ClassBase : ManagedType [NonSerialized] internal List dotNetMembers; internal Indexer indexer; + internal Hashtable richcompare; internal MaybeType type; internal ClassBase(Type tp) @@ -35,6 +36,15 @@ internal virtual bool CanSubclass() return !type.Value.IsEnum; } + public readonly static Dictionary PyToCilOpMap = new Dictionary + { + [Runtime.Py_EQ] = "op_Equality", + [Runtime.Py_NE] = "op_Inequality", + [Runtime.Py_GT] = "op_GreaterThan", + [Runtime.Py_GE] = "op_GreaterThanOrEqual", + [Runtime.Py_LT] = "op_LessThan", + [Runtime.Py_LE] = "op_LessThanOrEqual", + }; /// /// Default implementation of [] semantics for reflected types. @@ -72,6 +82,42 @@ public static IntPtr tp_richcompare(IntPtr ob, IntPtr other, int op) { CLRObject co1; CLRObject co2; + IntPtr tp = Runtime.PyObject_TYPE(ob); + var cls = (ClassBase)GetManagedObject(tp); + // C# operator methods take precedence over IComparable. + // We first check if there's a comparison operator by looking up the richcompare table, + // otherwise fallback to checking if an IComparable interface is handled. + if (PyToCilOpMap.ContainsKey(op)) { + string CilOp = PyToCilOpMap[op]; + if (cls.richcompare.Contains(CilOp)) { + var methodObject = (MethodObject)cls.richcompare[CilOp]; + IntPtr args = other; + var free = false; + if (!Runtime.PyTuple_Check(other)) + { + // Wrap the `other` argument of a binary comparison operator in a PyTuple. + args = Runtime.PyTuple_New(1); + Runtime.XIncref(other); + Runtime.PyTuple_SetItem(args, 0, other); + free = true; + } + + IntPtr value; + try + { + value = methodObject.Invoke(ob, args, IntPtr.Zero); + } + finally + { + if (free) + { + Runtime.XDecref(args); // Free args pytuple + } + } + return value; + } + } + switch (op) { case Runtime.Py_EQ: diff --git a/src/runtime/classmanager.cs b/src/runtime/classmanager.cs index 64c985ce7..f18741d95 100644 --- a/src/runtime/classmanager.cs +++ b/src/runtime/classmanager.cs @@ -259,6 +259,7 @@ private static void InitClassBase(Type type, ClassBase impl) ClassInfo info = GetClassInfo(type); impl.indexer = info.indexer; + impl.richcompare = new Hashtable(); // Now we allocate the Python type object to reflect the given // managed type, filling the Python type slots with thunks that @@ -284,6 +285,9 @@ private static void InitClassBase(Type type, ClassBase impl) Runtime.PyDict_SetItemString(dict, name, item.pyHandle); // Decref the item now that it's been used. item.DecrRefCount(); + if (ClassBase.PyToCilOpMap.ContainsValue(name)) { + impl.richcompare.Add(name, iter.Value); + } } // If class has constructors, generate an __doc__ attribute. diff --git a/src/runtime/operatormethod.cs b/src/runtime/operatormethod.cs index 6b0ee1926..422c4b803 100644 --- a/src/runtime/operatormethod.cs +++ b/src/runtime/operatormethod.cs @@ -50,6 +50,8 @@ static OperatorMethod() ["op_UnaryNegation"] = new SlotDefinition("__neg__", TypeOffset.nb_negative), ["op_UnaryPlus"] = new SlotDefinition("__pos__", TypeOffset.nb_positive), ["op_OneComplement"] = new SlotDefinition("__invert__", TypeOffset.nb_invert), + ["op_Equality"] = new SlotDefinition("__eq__", TypeOffset.tp_richcompare), + ["op_Inequality"] = new SlotDefinition("__ne__", TypeOffset.tp_richcompare), ["op_GreaterThan"] = new SlotDefinition("__gt__", TypeOffset.tp_richcompare), ["op_GreaterThanOrEqual"] = new SlotDefinition("__ge__", TypeOffset.tp_richcompare), ["op_LessThan"] = new SlotDefinition("__lt__", TypeOffset.tp_richcompare), @@ -79,6 +81,12 @@ public static bool IsOperatorMethod(MethodBase method) } return OpMethodMap.ContainsKey(method.Name); } + + public static bool IsComparisonOp(MethodInfo method) + { + return OpMethodMap[method.Name].TypeOffset == TypeOffset.tp_richcompare; + } + /// /// For the operator methods of a CLR type, set the special slots of the /// corresponding Python type's operator methods. @@ -91,7 +99,7 @@ public static void FixupSlots(IntPtr pyType, Type clrType) Debug.Assert(_opType != null); foreach (var method in clrType.GetMethods(flags)) { - if (!IsOperatorMethod(method)) + if (!IsOperatorMethod(method) || IsComparisonOp(method)) // We don't want to override ClassBase.tp_richcompare. { continue; } From ae7e774c4564de86b69c9f46ac6767db199eae80 Mon Sep 17 00:00:00 2001 From: Christabella Irwanto Date: Thu, 7 Jan 2021 09:07:45 +0200 Subject: [PATCH 04/12] Add eq and ineq operators, disallow reverse for comp --- src/embed_tests/TestOperator.cs | 26 ++++++++++++++++++++++++++ src/runtime/methodbinder.cs | 7 ++++--- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/embed_tests/TestOperator.cs b/src/embed_tests/TestOperator.cs index 37be36206..31ed0106f 100644 --- a/src/embed_tests/TestOperator.cs +++ b/src/embed_tests/TestOperator.cs @@ -149,6 +149,32 @@ public OperableObject(int num) return new OperableObject(a.Num ^ b); } + public static bool operator ==(int a, OperableObject b) + { + return (a == b.Num); + } + public static bool operator ==(OperableObject a, OperableObject b) + { + return (a.Num == b.Num); + } + public static bool operator ==(OperableObject a, int b) + { + return (a.Num == b); + } + + public static bool operator !=(int a, OperableObject b) + { + return (a != b.Num); + } + public static bool operator !=(OperableObject a, OperableObject b) + { + return (a.Num != b.Num); + } + public static bool operator !=(OperableObject a, int b) + { + return (a.Num != b); + } + public static bool operator <=(int a, OperableObject b) { return (a <= b.Num); diff --git a/src/runtime/methodbinder.cs b/src/runtime/methodbinder.cs index ba37c19c1..5de0ecc00 100644 --- a/src/runtime/methodbinder.cs +++ b/src/runtime/methodbinder.cs @@ -354,16 +354,17 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth int kwargsMatched; int defaultsNeeded; bool isOperator = OperatorMethod.IsOperatorMethod(mi); - int clrnargs = pi.Length; // 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 == clrnargs - 1; + isOperator = isOperator && pynargs == pi.Length - 1; + bool isReverse = isOperator && OperatorMethod.IsReverse((MethodInfo)mi); // 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 paramsArray, out defaultArgList, out kwargsMatched, out defaultsNeeded) && !isOperator) { continue; } // Preprocessing pi to remove either the first or second argument. - bool isReverse = isOperator && OperatorMethod.IsReverse((MethodInfo)mi); // Only cast if isOperator. 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. From 56857271287d3aa465160ba1bcfc9389dafeea19 Mon Sep 17 00:00:00 2001 From: Christabella Irwanto Date: Thu, 7 Jan 2021 09:45:04 +0200 Subject: [PATCH 05/12] Minor formatting (whitespace, ordering) --- src/embed_tests/TestOperator.cs | 46 ++++++++++++++++----------------- src/runtime/classbase.cs | 4 +-- src/runtime/operatormethod.cs | 4 +-- 3 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/embed_tests/TestOperator.cs b/src/embed_tests/TestOperator.cs index 31ed0106f..e727d9a77 100644 --- a/src/embed_tests/TestOperator.cs +++ b/src/embed_tests/TestOperator.cs @@ -201,24 +201,24 @@ public OperableObject(int num) return (a.Num >= b); } - public static bool operator<(int a, OperableObject b) + public static bool operator <(int a, OperableObject b) { return (a < b.Num); } - public static bool operator<(OperableObject a, OperableObject b) + public static bool operator <(OperableObject a, OperableObject b) { return (a.Num < b.Num); } - public static bool operator<(OperableObject a, int b) + public static bool operator <(OperableObject a, int b) { return (a.Num < b); } - public static bool operator>(int a, OperableObject b) + public static bool operator >(int a, OperableObject b) { return (a > b.Num); } - public static bool operator>(OperableObject a, OperableObject b) + public static bool operator >(OperableObject a, OperableObject b) { return (a.Num > b.Num); } @@ -285,6 +285,12 @@ public void SymmetricalOperatorOverloads() c = a ^ b assert c.Num == a.Num ^ b.Num +c = a == b +assert c == (a.Num == b.Num) + +c = a != b +assert c == (a.Num != b.Num) + c = a <= b assert c == (a.Num <= b.Num) @@ -296,12 +302,6 @@ public void SymmetricalOperatorOverloads() c = a > b assert c == (a.Num > b.Num) - -c = a == b -assert c == (a.Num == b.Num) - -c = a != b -assert c == (a.Num != b.Num) "); } @@ -360,6 +360,12 @@ public void ForwardOperatorOverloads() 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) @@ -371,12 +377,6 @@ public void ForwardOperatorOverloads() c = a > b assert c == (a.Num > b) - -c = a == b -assert c == (a.Num == b) - -c = a != b -assert c == (a.Num != b) "); } @@ -419,6 +419,12 @@ public void ReverseOperatorOverloads() 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) @@ -430,12 +436,6 @@ public void ReverseOperatorOverloads() c = a > b assert c == (a > b.Num) - -c = a == b -assert c == (a == b.Num) - -c = a != b -assert c == (a != b.Num) "); } diff --git a/src/runtime/classbase.cs b/src/runtime/classbase.cs index b3c5d8c4d..b8eebe326 100644 --- a/src/runtime/classbase.cs +++ b/src/runtime/classbase.cs @@ -40,10 +40,10 @@ internal virtual bool CanSubclass() { [Runtime.Py_EQ] = "op_Equality", [Runtime.Py_NE] = "op_Inequality", - [Runtime.Py_GT] = "op_GreaterThan", + [Runtime.Py_LE] = "op_LessThanOrEqual", [Runtime.Py_GE] = "op_GreaterThanOrEqual", [Runtime.Py_LT] = "op_LessThan", - [Runtime.Py_LE] = "op_LessThanOrEqual", + [Runtime.Py_GT] = "op_GreaterThan", }; /// diff --git a/src/runtime/operatormethod.cs b/src/runtime/operatormethod.cs index 422c4b803..a29623779 100644 --- a/src/runtime/operatormethod.cs +++ b/src/runtime/operatormethod.cs @@ -52,10 +52,10 @@ static OperatorMethod() ["op_OneComplement"] = new SlotDefinition("__invert__", TypeOffset.nb_invert), ["op_Equality"] = new SlotDefinition("__eq__", TypeOffset.tp_richcompare), ["op_Inequality"] = new SlotDefinition("__ne__", TypeOffset.tp_richcompare), - ["op_GreaterThan"] = new SlotDefinition("__gt__", TypeOffset.tp_richcompare), + ["op_LessThanOrEqual"] = new SlotDefinition("__le__", TypeOffset.tp_richcompare), ["op_GreaterThanOrEqual"] = new SlotDefinition("__ge__", TypeOffset.tp_richcompare), ["op_LessThan"] = new SlotDefinition("__lt__", TypeOffset.tp_richcompare), - ["op_LessThanOrEqual"] = new SlotDefinition("__le__", TypeOffset.tp_richcompare) + ["op_GreaterThan"] = new SlotDefinition("__gt__", TypeOffset.tp_richcompare), }; } From 03a73c08f5f1e70cd9ef1ce5d8ae7a2e47e92df7 Mon Sep 17 00:00:00 2001 From: Christabella Irwanto Date: Thu, 7 Jan 2021 09:59:23 +0200 Subject: [PATCH 06/12] Minor: use `out var` instead of declaring separately. --- src/runtime/classmanager.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/runtime/classmanager.cs b/src/runtime/classmanager.cs index f18741d95..f867cf0a9 100644 --- a/src/runtime/classmanager.cs +++ b/src/runtime/classmanager.cs @@ -557,8 +557,7 @@ private static ClassInfo GetClassInfo(Type type) { string pyName = OperatorMethod.GetPyMethodName(name); string pyNameReverse = OperatorMethod.ReversePyMethodName(pyName); - MethodInfo[] forwardMethods, reverseMethods; - OperatorMethod.FilterMethods(mlist, out forwardMethods, out reverseMethods); + OperatorMethod.FilterMethods(mlist, out var forwardMethods, out var reverseMethods); // Only methods where the left operand is the declaring type. if (forwardMethods.Length > 0) ci.members[pyName] = new MethodObject(type, name, forwardMethods); From 67928e1078b6290c781a4ed789177f4a7dc6e9cd Mon Sep 17 00:00:00 2001 From: Christabella Irwanto Date: Thu, 7 Jan 2021 11:34:51 +0200 Subject: [PATCH 07/12] Implement .Equals and .GetHashCode to fix warning --- src/embed_tests/TestOperator.cs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/embed_tests/TestOperator.cs b/src/embed_tests/TestOperator.cs index e727d9a77..5c12a2268 100644 --- a/src/embed_tests/TestOperator.cs +++ b/src/embed_tests/TestOperator.cs @@ -25,6 +25,17 @@ public class OperableObject { public int Num { get; set; } + public override int GetHashCode() + { + return 159832395 + Num.GetHashCode(); + } + + public override bool Equals(object obj) + { + return obj is OperableObject @object && + Num == @object.Num; + } + public OperableObject(int num) { Num = num; From 6a64678207ba251568d6f566975fa400872d29a9 Mon Sep 17 00:00:00 2001 From: Christabella Irwanto Date: Fri, 8 Jan 2021 16:31:01 +0200 Subject: [PATCH 08/12] Style (TryGetValue), formatting, and minor changes --- src/embed_tests/TestOperator.cs | 2 +- src/runtime/classbase.cs | 63 ++++++++++++++++----------------- src/runtime/classmanager.cs | 6 ++-- src/runtime/operatormethod.cs | 4 ++- 4 files changed, 37 insertions(+), 38 deletions(-) diff --git a/src/embed_tests/TestOperator.cs b/src/embed_tests/TestOperator.cs index 5c12a2268..5aa53b29f 100644 --- a/src/embed_tests/TestOperator.cs +++ b/src/embed_tests/TestOperator.cs @@ -27,7 +27,7 @@ public class OperableObject public override int GetHashCode() { - return 159832395 + Num.GetHashCode(); + return unchecked(159832395 + Num.GetHashCode()); } public override bool Equals(object obj) diff --git a/src/runtime/classbase.cs b/src/runtime/classbase.cs index b8eebe326..1b0ba9a91 100644 --- a/src/runtime/classbase.cs +++ b/src/runtime/classbase.cs @@ -21,7 +21,7 @@ internal class ClassBase : ManagedType [NonSerialized] internal List dotNetMembers; internal Indexer indexer; - internal Hashtable richcompare; + internal Dictionary richcompare; internal MaybeType type; internal ClassBase(Type tp) @@ -36,14 +36,14 @@ internal virtual bool CanSubclass() return !type.Value.IsEnum; } - public readonly static Dictionary PyToCilOpMap = new Dictionary + public readonly static Dictionary CilToPyOpMap = new Dictionary { - [Runtime.Py_EQ] = "op_Equality", - [Runtime.Py_NE] = "op_Inequality", - [Runtime.Py_LE] = "op_LessThanOrEqual", - [Runtime.Py_GE] = "op_GreaterThanOrEqual", - [Runtime.Py_LT] = "op_LessThan", - [Runtime.Py_GT] = "op_GreaterThan", + ["op_Equality"] = Runtime.Py_EQ, + ["op_Inequality"] = Runtime.Py_NE, + ["op_LessThanOrEqual"] = Runtime.Py_LE, + ["op_GreaterThanOrEqual"] = Runtime.Py_GE, + ["op_LessThan"] = Runtime.Py_LT, + ["op_GreaterThan"] = Runtime.Py_GT, }; /// @@ -87,35 +87,32 @@ public static IntPtr tp_richcompare(IntPtr ob, IntPtr other, int op) // C# operator methods take precedence over IComparable. // We first check if there's a comparison operator by looking up the richcompare table, // otherwise fallback to checking if an IComparable interface is handled. - if (PyToCilOpMap.ContainsKey(op)) { - string CilOp = PyToCilOpMap[op]; - if (cls.richcompare.Contains(CilOp)) { - var methodObject = (MethodObject)cls.richcompare[CilOp]; - IntPtr args = other; - var free = false; - if (!Runtime.PyTuple_Check(other)) - { - // Wrap the `other` argument of a binary comparison operator in a PyTuple. - args = Runtime.PyTuple_New(1); - Runtime.XIncref(other); - Runtime.PyTuple_SetItem(args, 0, other); - free = true; - } + if (cls.richcompare.TryGetValue(op, out var methodObject)) + { + IntPtr args = other; + var free = false; + if (true) + { + // Wrap the `other` argument of a binary comparison operator in a PyTuple. + args = Runtime.PyTuple_New(1); + Runtime.XIncref(other); + Runtime.PyTuple_SetItem(args, 0, other); + free = true; + } - IntPtr value; - try - { - value = methodObject.Invoke(ob, args, IntPtr.Zero); - } - finally + IntPtr value; + try + { + value = methodObject.Invoke(ob, args, IntPtr.Zero); + } + finally + { + if (free) { - if (free) - { - Runtime.XDecref(args); // Free args pytuple - } + Runtime.XDecref(args); // Free args pytuple } - return value; } + return value; } switch (op) diff --git a/src/runtime/classmanager.cs b/src/runtime/classmanager.cs index f867cf0a9..0cbff371f 100644 --- a/src/runtime/classmanager.cs +++ b/src/runtime/classmanager.cs @@ -259,7 +259,7 @@ private static void InitClassBase(Type type, ClassBase impl) ClassInfo info = GetClassInfo(type); impl.indexer = info.indexer; - impl.richcompare = new Hashtable(); + impl.richcompare = new Dictionary(); // Now we allocate the Python type object to reflect the given // managed type, filling the Python type slots with thunks that @@ -285,8 +285,8 @@ private static void InitClassBase(Type type, ClassBase impl) Runtime.PyDict_SetItemString(dict, name, item.pyHandle); // Decref the item now that it's been used. item.DecrRefCount(); - if (ClassBase.PyToCilOpMap.ContainsValue(name)) { - impl.richcompare.Add(name, iter.Value); + if (ClassBase.CilToPyOpMap.TryGetValue(name, out var pyOp)) { + impl.richcompare.Add(pyOp, (MethodObject)item); } } diff --git a/src/runtime/operatormethod.cs b/src/runtime/operatormethod.cs index a29623779..14392e25d 100644 --- a/src/runtime/operatormethod.cs +++ b/src/runtime/operatormethod.cs @@ -99,7 +99,9 @@ public static void FixupSlots(IntPtr pyType, Type clrType) Debug.Assert(_opType != null); foreach (var method in clrType.GetMethods(flags)) { - if (!IsOperatorMethod(method) || IsComparisonOp(method)) // We don't want to override ClassBase.tp_richcompare. + // We don't want to override slots for either non-operators or + // comparison operators, which are handled by ClassBase.tp_richcompare. + if (!IsOperatorMethod(method) || IsComparisonOp(method)) { continue; } From 9dd9162d89bddb8106563fa775a0c3439e95a876 Mon Sep 17 00:00:00 2001 From: Christabella Irwanto Date: Fri, 8 Jan 2021 16:41:12 +0200 Subject: [PATCH 09/12] Use a separate map for comparison operators --- src/runtime/operatormethod.cs | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/src/runtime/operatormethod.cs b/src/runtime/operatormethod.cs index 14392e25d..b155d9ebc 100644 --- a/src/runtime/operatormethod.cs +++ b/src/runtime/operatormethod.cs @@ -15,6 +15,7 @@ internal static class OperatorMethod /// that identifies that operator's slot (e.g. nb_add) in heap space. /// public static Dictionary OpMethodMap { get; private set; } + public static Dictionary ComparisonOpMap { get; private set; } public readonly struct SlotDefinition { public SlotDefinition(string methodName, int typeOffset) @@ -24,6 +25,7 @@ public SlotDefinition(string methodName, int typeOffset) } public string MethodName { get; } public int TypeOffset { get; } + } private static PyObject _opType; @@ -50,12 +52,15 @@ static OperatorMethod() ["op_UnaryNegation"] = new SlotDefinition("__neg__", TypeOffset.nb_negative), ["op_UnaryPlus"] = new SlotDefinition("__pos__", TypeOffset.nb_positive), ["op_OneComplement"] = new SlotDefinition("__invert__", TypeOffset.nb_invert), - ["op_Equality"] = new SlotDefinition("__eq__", TypeOffset.tp_richcompare), - ["op_Inequality"] = new SlotDefinition("__ne__", TypeOffset.tp_richcompare), - ["op_LessThanOrEqual"] = new SlotDefinition("__le__", TypeOffset.tp_richcompare), - ["op_GreaterThanOrEqual"] = new SlotDefinition("__ge__", TypeOffset.tp_richcompare), - ["op_LessThan"] = new SlotDefinition("__lt__", TypeOffset.tp_richcompare), - ["op_GreaterThan"] = new SlotDefinition("__gt__", TypeOffset.tp_richcompare), + }; + ComparisonOpMap = new Dictionary + { + ["op_Equality"] = "__eq__", + ["op_Inequality"] = "__ne__", + ["op_LessThanOrEqual"] = "__le__", + ["op_GreaterThanOrEqual"] = "__ge__", + ["op_LessThan"] = "__lt__", + ["op_GreaterThan"] = "__gt__", }; } @@ -79,12 +84,12 @@ public static bool IsOperatorMethod(MethodBase method) { return false; } - return OpMethodMap.ContainsKey(method.Name); + return OpMethodMap.ContainsKey(method.Name) || ComparisonOpMap.ContainsKey(method.Name); } public static bool IsComparisonOp(MethodInfo method) { - return OpMethodMap[method.Name].TypeOffset == TypeOffset.tp_richcompare; + return ComparisonOpMap.ContainsKey(method.Name); } /// @@ -114,13 +119,18 @@ public static void FixupSlots(IntPtr pyType, Type clrType) // when used with a Python operator. // https://tenthousandmeters.com/blog/python-behind-the-scenes-6-how-python-object-system-works/ Marshal.WriteIntPtr(pyType, offset, func); - } } public static string GetPyMethodName(string clrName) { - return OpMethodMap[clrName].MethodName; + if (OpMethodMap.ContainsKey(clrName)) + { + return OpMethodMap[clrName].MethodName; + } else + { + return ComparisonOpMap[clrName]; + } } private static string GenerateDummyCode() From a8c659158d1daff05063bbdff7710217ff4b697b Mon Sep 17 00:00:00 2001 From: Christabella Irwanto Date: Mon, 11 Jan 2021 13:40:37 +0200 Subject: [PATCH 10/12] Add tuple comparison operator tests (2/6) --- src/embed_tests/TestOperator.cs | 40 +++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/embed_tests/TestOperator.cs b/src/embed_tests/TestOperator.cs index 5aa53b29f..f99cf1848 100644 --- a/src/embed_tests/TestOperator.cs +++ b/src/embed_tests/TestOperator.cs @@ -212,6 +212,25 @@ public OperableObject(int num) return (a.Num >= b); } + public static bool operator >=(OperableObject a, PyObject b) + { + using (Py.GIL()) + { + // Assuming b is a tuple, take the first element. + int bNum = (int)b.ToArray()[0].AsManagedObject(typeof(int)); + return a.Num >= bNum; + } + } + public static bool operator <=(OperableObject a, PyObject b) + { + using (Py.GIL()) + { + // Assuming b is a tuple, take the first element. + int bNum = (int)b.ToArray()[0].AsManagedObject(typeof(int)); + return a.Num <= bNum; + } + } + public static bool operator <(int a, OperableObject b) { return (a < b.Num); @@ -391,6 +410,27 @@ public void ForwardOperatorOverloads() "); } + [Test] + public void TupleComparisonOperatorOverloads() + { + string name = string.Format("{0}.{1}", + typeof(OperableObject).DeclaringType.Name, + typeof(OperableObject).Name); + string module = MethodBase.GetCurrentMethod().DeclaringType.Namespace; + PythonEngine.Exec($@" +from {module} import * +cls = {name} +a = cls(2) +b = (1, 2) + +c = a >= b +assert c == (a.Num >= b[0]) + +c = a <= b +assert c == (a.Num <= b[0]) +"); + } + [Test] public void ReverseOperatorOverloads() From 6d4dec7ef5955f2edba5fac68f50cd9e010cef11 Mon Sep 17 00:00:00 2001 From: Christabella Irwanto Date: Mon, 11 Jan 2021 13:48:10 +0200 Subject: [PATCH 11/12] Add reverse case for tuple comparison op test --- src/embed_tests/TestOperator.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/embed_tests/TestOperator.cs b/src/embed_tests/TestOperator.cs index f99cf1848..abce2db18 100644 --- a/src/embed_tests/TestOperator.cs +++ b/src/embed_tests/TestOperator.cs @@ -428,6 +428,12 @@ public void TupleComparisonOperatorOverloads() c = a <= b assert c == (a.Num <= b[0]) + +c = b >= a +assert c == (b[0] >= a.Num) + +c = b <= a +assert c == (b[0] <= a.Num) "); } From 7a2b5e296475d72854f7b52ea1ba6652131c5836 Mon Sep 17 00:00:00 2001 From: Christabella Irwanto Date: Tue, 12 Jan 2021 09:59:43 +0200 Subject: [PATCH 12/12] Simplify PyObj casting, logic of if blocks. --- src/embed_tests/TestOperator.cs | 4 ++-- src/runtime/classbase.cs | 19 +++++-------------- src/runtime/operatormethod.cs | 4 ++-- 3 files changed, 9 insertions(+), 18 deletions(-) diff --git a/src/embed_tests/TestOperator.cs b/src/embed_tests/TestOperator.cs index abce2db18..8e9feb241 100644 --- a/src/embed_tests/TestOperator.cs +++ b/src/embed_tests/TestOperator.cs @@ -217,7 +217,7 @@ public OperableObject(int num) using (Py.GIL()) { // Assuming b is a tuple, take the first element. - int bNum = (int)b.ToArray()[0].AsManagedObject(typeof(int)); + int bNum = b[0].As(); return a.Num >= bNum; } } @@ -226,7 +226,7 @@ public OperableObject(int num) using (Py.GIL()) { // Assuming b is a tuple, take the first element. - int bNum = (int)b.ToArray()[0].AsManagedObject(typeof(int)); + int bNum = b[0].As(); return a.Num <= bNum; } } diff --git a/src/runtime/classbase.cs b/src/runtime/classbase.cs index 1b0ba9a91..872501267 100644 --- a/src/runtime/classbase.cs +++ b/src/runtime/classbase.cs @@ -89,16 +89,10 @@ public static IntPtr tp_richcompare(IntPtr ob, IntPtr other, int op) // otherwise fallback to checking if an IComparable interface is handled. if (cls.richcompare.TryGetValue(op, out var methodObject)) { - IntPtr args = other; - var free = false; - if (true) - { - // Wrap the `other` argument of a binary comparison operator in a PyTuple. - args = Runtime.PyTuple_New(1); - Runtime.XIncref(other); - Runtime.PyTuple_SetItem(args, 0, other); - free = true; - } + // Wrap the `other` argument of a binary comparison operator in a PyTuple. + IntPtr args = Runtime.PyTuple_New(1); + Runtime.XIncref(other); + Runtime.PyTuple_SetItem(args, 0, other); IntPtr value; try @@ -107,10 +101,7 @@ public static IntPtr tp_richcompare(IntPtr ob, IntPtr other, int op) } finally { - if (free) - { - Runtime.XDecref(args); // Free args pytuple - } + Runtime.XDecref(args); // Free args pytuple } return value; } diff --git a/src/runtime/operatormethod.cs b/src/runtime/operatormethod.cs index b155d9ebc..59bf944bc 100644 --- a/src/runtime/operatormethod.cs +++ b/src/runtime/operatormethod.cs @@ -104,9 +104,9 @@ public static void FixupSlots(IntPtr pyType, Type clrType) Debug.Assert(_opType != null); foreach (var method in clrType.GetMethods(flags)) { - // We don't want to override slots for either non-operators or + // We only want to override slots for operators excluding // comparison operators, which are handled by ClassBase.tp_richcompare. - if (!IsOperatorMethod(method) || IsComparisonOp(method)) + if (!OpMethodMap.ContainsKey(method.Name)) { continue; }