Add comments and remove unused in operatormethod.cs

christabella · christabella · commit b2053a773d1a · 2020-12-18T14:32:51.000Z
Also move fixupslots before pytype_ready.
Also apply access violation fix in classderived.
diff --git a/src/runtime/classderived.cs b/src/runtime/classderived.cs
@@ -66,7 +66,12 @@ internal ClassDerivedObject(Type tp) : base(tp)
         public new static void tp_dealloc(IntPtr ob)
         {
             var self = (CLRObject)GetManagedObject(ob);
-
+            // Amos's fix https://github.com/pythonnet/pythonnet/pull/1260#issuecomment-726719595
+            if (self.tpHandle == IntPtr.Zero)
+            {
+                ClassBase.tp_dealloc(ob);
+                return;
+            }
             // don't let the python GC destroy this object
             Runtime.PyObject_GC_UnTrack(self.pyHandle);
 
diff --git a/src/runtime/operatormethod.cs b/src/runtime/operatormethod.cs
@@ -9,9 +9,11 @@ namespace Python.Runtime
 {
     internal static class OperatorMethod
     {
+        // Maps the compiled method name in .NET CIL (e.g. op_Addition) to
+        // the equivalent Python operator (e.g. __add__) as well as the offset
+        // that identifies that operator's slot (e.g. nb_add) in heap space.
         public static Dictionary<string, Tuple<string, int>> OpMethodMap { get; private set; }
 
-        private static Dictionary<string, string> _pyOpNames;
         private static PyObject _opType;
 
         static OperatorMethod()
@@ -35,12 +37,6 @@ static OperatorMethod()
                 ["op_Modulus"] = Tuple.Create("__mod__", TypeOffset.nb_remainder),
                 ["op_OneComplement"] = Tuple.Create("__invert__", TypeOffset.nb_invert)
             };
-
-            _pyOpNames = new Dictionary<string, string>();
-            foreach (string name in OpMethodMap.Keys)
-            {
-                _pyOpNames.Add(GetPyMethodName(name), name);
-            }
         }
 
         public static void Initialize()
@@ -62,11 +58,6 @@ public static bool IsOperatorMethod(string methodName)
             return OpMethodMap.ContainsKey(methodName);
         }
 
-        public static bool IsPyOperatorMethod(string pyMethodName)
-        {
-            return _pyOpNames.ContainsKey(pyMethodName);
-        }
-
         public static bool IsOperatorMethod(MethodBase method)
         {
             if (!method.IsSpecialName)
@@ -75,7 +66,12 @@ public static bool IsOperatorMethod(MethodBase method)
             }
             return OpMethodMap.ContainsKey(method.Name);
         }
-
+        /// <summary>
+        /// For the operator methods of a CLR type, set the special slots of the
+        /// corresponding Python type's operator methods.
+        /// </summary>
+        /// <param name="pyType"></param>
+        /// <param name="clrType"></param>
         public static void FixupSlots(IntPtr pyType, Type clrType)
         {
             const BindingFlags flags = BindingFlags.Public | BindingFlags.Static;
@@ -86,10 +82,16 @@ public static void FixupSlots(IntPtr pyType, Type clrType)
                 {
                     continue;
                 }
-                var slotdef = OpMethodMap[method.Name];
-                int offset = slotdef.Item2;
+                int offset = OpMethodMap[method.Name].Item2;
+                // Copy the default implementation of e.g. the nb_add slot,
+                // which simply calls __add__ on the type.
                 IntPtr func = Marshal.ReadIntPtr(_opType.Handle, offset);
+                // Write the slot definition of the target Python type, so
+                // that we can later modify __add___ and it will be called
+                // 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);
+
             }
         }
 
@@ -116,7 +118,9 @@ private static PyObject GetOperatorType()
             {
                 // A hack way for getting typeobject.c::slotdefs
                 string code = GenerateDummyCode();
+                // The resulting OperatorMethod class is stored in a PyDict.
                 PythonEngine.Exec(code, null, locals.Handle);
+                // Return the class itself, which is a type.
                 return locals.GetItem("OperatorMethod");
             }
         }
diff --git a/src/runtime/typemanager.cs b/src/runtime/typemanager.cs
@@ -273,6 +273,7 @@ internal static IntPtr CreateType(ManagedType impl, Type clrType)
                             | TypeFlags.HaveGC;
             Util.WriteCLong(type, TypeOffset.tp_flags, flags);
 
+            OperatorMethod.FixupSlots(type, clrType);
             // Leverage followup initialization from the Python runtime. Note
             // that the type of the new type must PyType_Type at the time we
             // call this, else PyType_Ready will skip some slot initialization.
@@ -281,7 +282,6 @@ internal static IntPtr CreateType(ManagedType impl, Type clrType)
             {
                 throw new PythonException();
             }
-            OperatorMethod.FixupSlots(type, clrType);
 
             IntPtr dict = Marshal.ReadIntPtr(type, TypeOffset.tp_dict);
             string mn = clrType.Namespace ?? "";

Original file line number	Diff line number	Diff line change
`@@ -9,9 +9,11 @@ namespace Python.Runtime`
`9`	`9`	`{`
`10`	`10`	`internal static class OperatorMethod`
`11`	`11`	`{`
	`12`	`+ // Maps the compiled method name in .NET CIL (e.g. op_Addition) to`
	`13`	`+ // the equivalent Python operator (e.g. __add__) as well as the offset`
	`14`	`+ // that identifies that operator's slot (e.g. nb_add) in heap space.`
`12`	`15`	`public static Dictionary<string, Tuple<string, int>> OpMethodMap { get; private set; }`
`13`	`16`
`14`		`- private static Dictionary<string, string> _pyOpNames;`
`15`	`17`	`private static PyObject _opType;`
`16`	`18`
`17`	`19`	`static OperatorMethod()`
`@@ -35,12 +37,6 @@ static OperatorMethod()`
`35`	`37`	`["op_Modulus"] = Tuple.Create("__mod__", TypeOffset.nb_remainder),`
`36`	`38`	`["op_OneComplement"] = Tuple.Create("__invert__", TypeOffset.nb_invert)`
`37`	`39`	`};`
`38`		`-`
`39`		`- _pyOpNames = new Dictionary<string, string>();`
`40`		`- foreach (string name in OpMethodMap.Keys)`
`41`		`- {`
`42`		`- _pyOpNames.Add(GetPyMethodName(name), name);`
`43`		`- }`
`44`	`40`	`}`
`45`	`41`
`46`	`42`	`public static void Initialize()`
`@@ -62,11 +58,6 @@ public static bool IsOperatorMethod(string methodName)`
`62`	`58`	`return OpMethodMap.ContainsKey(methodName);`
`63`	`59`	`}`
`64`	`60`
`65`		`- public static bool IsPyOperatorMethod(string pyMethodName)`
`66`		`- {`
`67`		`- return _pyOpNames.ContainsKey(pyMethodName);`
`68`		`- }`
`69`		`-`
`70`	`61`	`public static bool IsOperatorMethod(MethodBase method)`
`71`	`62`	`{`
`72`	`63`	`if (!method.IsSpecialName)`
`@@ -75,7 +66,12 @@ public static bool IsOperatorMethod(MethodBase method)`
`75`	`66`	`}`
`76`	`67`	`return OpMethodMap.ContainsKey(method.Name);`
`77`	`68`	`}`
`78`		`-`
	`69`	`+ /// <summary>`
	`70`	`+ /// For the operator methods of a CLR type, set the special slots of the`
	`71`	`+ /// corresponding Python type's operator methods.`
	`72`	`+ /// </summary>`
	`73`	`+ /// <param name="pyType"></param>`
	`74`	`+ /// <param name="clrType"></param>`
`79`	`75`	`public static void FixupSlots(IntPtr pyType, Type clrType)`
`80`	`76`	`{`
`81`	`77`	`const BindingFlags flags = BindingFlags.Public \| BindingFlags.Static;`
`@@ -86,10 +82,16 @@ public static void FixupSlots(IntPtr pyType, Type clrType)`
`86`	`82`	`{`
`87`	`83`	`continue;`
`88`	`84`	`}`
`89`		`- var slotdef = OpMethodMap[method.Name];`
`90`		`- int offset = slotdef.Item2;`
	`85`	`+ int offset = OpMethodMap[method.Name].Item2;`
	`86`	`+ // Copy the default implementation of e.g. the nb_add slot,`
	`87`	`+ // which simply calls __add__ on the type.`
`91`	`88`	`IntPtr func = Marshal.ReadIntPtr(_opType.Handle, offset);`
	`89`	`+ // Write the slot definition of the target Python type, so`
	`90`	`+ // that we can later modify __add___ and it will be called`
	`91`	`+ // when used with a Python operator.`
	`92`	`+ // https://tenthousandmeters.com/blog/python-behind-the-scenes-6-how-python-object-system-works/`
`92`	`93`	`Marshal.WriteIntPtr(pyType, offset, func);`
	`94`	`+`
`93`	`95`	`}`
`94`	`96`	`}`
`95`	`97`
`@@ -116,7 +118,9 @@ private static PyObject GetOperatorType()`
`116`	`118`	`{`
`117`	`119`	`// A hack way for getting typeobject.c::slotdefs`
`118`	`120`	`string code = GenerateDummyCode();`
	`121`	`+ // The resulting OperatorMethod class is stored in a PyDict.`
`119`	`122`	`PythonEngine.Exec(code, null, locals.Handle);`
	`123`	`+ // Return the class itself, which is a type.`
`120`	`124`	`return locals.GetItem("OperatorMethod");`
`121`	`125`	`}`
`122`	`126`	`}`
Original file line number	Diff line number	Diff line change
`@@ -273,6 +273,7 @@ internal static IntPtr CreateType(ManagedType impl, Type clrType)`
`273`	`273`	`\| TypeFlags.HaveGC;`
`274`	`274`	`Util.WriteCLong(type, TypeOffset.tp_flags, flags);`
`275`	`275`
	`276`	`+ OperatorMethod.FixupSlots(type, clrType);`
`276`	`277`	`// Leverage followup initialization from the Python runtime. Note`
`277`	`278`	`// that the type of the new type must PyType_Type at the time we`
`278`	`279`	`// call this, else PyType_Ready will skip some slot initialization.`
`@@ -281,7 +282,6 @@ internal static IntPtr CreateType(ManagedType impl, Type clrType)`
`281`	`282`	`{`
`282`	`283`	`throw new PythonException();`
`283`	`284`	`}`
`284`		`- OperatorMethod.FixupSlots(type, clrType);`
`285`	`285`
`286`	`286`	`IntPtr dict = Marshal.ReadIntPtr(type, TypeOffset.tp_dict);`
`287`	`287`	`string mn = clrType.Namespace ?? "";`