Style (TryGetValue), formatting, and minor changes

pythonnet · lostmsu · Jan 12, 2021 · Dec 29, 2020 · Dec 29, 2020 · Jan 6, 2021
commit 6a64678207ba251568d6f566975fa400872d29a9
diff --git 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
@@ -21,7 +21,7 @@ internal class ClassBase : ManagedType
         [NonSerialized]
         internal List<string> dotNetMembers;
         internal Indexer indexer;
-        internal Hashtable richcompare;
+        internal Dictionary<int, MethodObject> richcompare;
         internal MaybeType type;
 
         internal ClassBase(Type tp)
@@ -36,14 +36,14 @@ internal virtual bool CanSubclass()
             return !type.Value.IsEnum;
         }
 
-        public readonly static Dictionary<int, string> PyToCilOpMap = new Dictionary<int, string>
+        public readonly static Dictionary<string, int> CilToPyOpMap = new Dictionary<string, int>
         {
-            [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,
         };
 
         /// <summary>
@@ -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
@@ -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<int, MethodObject>();
 
             // 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
@@ -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;
                 }