Split PyTruffle_Type_Modified

Author: msimacek

graalpython/com.oracle.graal.python.cext/src/typeobject.c

@@ -303,17 +303,50 @@ _PyTypes_Fini(PyInterpreterState *interp)
         clear_slotdefs();
     }
 }
-#endif // GraalPy change
 
 
 void
 PyType_Modified(PyTypeObject *type)
 {
-    // GraalPy change: different implementation
-    GraalPyTruffle_Type_Modified(type, type->tp_name, type->tp_mro);
+    /* Invalidate any cached data for the specified type and all
+       subclasses.  This function is called after the base
+       classes, mro, or attributes of the type are altered.
+
+       Invariants:
+
+       - before Py_TPFLAGS_VALID_VERSION_TAG can be set on a type,
+         it must first be set on all super types.
+
+       This function clears the Py_TPFLAGS_VALID_VERSION_TAG of a
+       type (so it must first clear it on all subclasses).  The
+       tp_version_tag value is meaningless unless this flag is set.
+       We don't assign new version tags eagerly, but only as
+       needed.
+     */
+    if (!_PyType_HasFeature(type, Py_TPFLAGS_VALID_VERSION_TAG)) {
+        return;
+    }
+
+    PyObject *subclasses = type->tp_subclasses;
+    if (subclasses != NULL) {
+        assert(PyDict_CheckExact(subclasses));
+
+        Py_ssize_t i = 0;
+        PyObject *ref;
+        while (PyDict_Next(subclasses, &i, NULL, &ref)) {
+            assert(PyWeakref_CheckRef(ref));
+            PyObject *obj = PyWeakref_GET_OBJECT(ref);
+            if (obj == Py_None) {
+                continue;
+            }
+            PyType_Modified(_PyType_CAST(obj));
+        }
+    }
+
+    type->tp_flags &= ~Py_TPFLAGS_VALID_VERSION_TAG;
+    type->tp_version_tag = 0; /* 0 is not a valid version tag */
 }
 
-#if 0 // GraalPy change
 static void
 type_mro_modified(PyTypeObject *type, PyObject *bases) {
     /*
@@ -6555,7 +6588,7 @@ PyType_Ready(PyTypeObject *type)
         type->tp_flags |= Py_TPFLAGS_IMMUTABLETYPE;
     }
 
-    /* GraalPy change: Types are often just static mem; so register them to be able to rule out invalid accesses.  */
+    // GraalPy change
     if (PyTruffle_Trace_Memory()) {
         GraalPyTruffle_Trace_Type(type, type->tp_name != NULL);
     }
@@ -6575,8 +6608,8 @@ PyType_Ready(PyTypeObject *type)
     type->tp_flags = (type->tp_flags & ~Py_TPFLAGS_READYING) | Py_TPFLAGS_READY;
     assert(_PyType_CheckConsistency(type));
 
-    // GraalPy change: it may be that the type was used uninitialized
-	GraalPyTruffle_Type_Modified(type, type->tp_name, NULL);
+    // GraalPy change
+	GraalPyTruffle_InitializeOldStyleSlots(type);
 
 	// GraalPy change: for reason, see first call to Py_INCREF in this function
 	Py_DECREF(type);

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextTypeBuiltins.java

@@ -63,7 +63,6 @@
 import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiBinaryBuiltinNode;
 import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiBuiltin;
 import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiCallPath;
-import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiTernaryBuiltinNode;
 import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiUnaryBuiltinNode;
 import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.PyObjectSetAttrNode;
 import com.oracle.graal.python.builtins.objects.PNone;
@@ -78,6 +77,7 @@
 import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodes.GetterRoot;
 import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodes.PExternalFunctionWrapper;
 import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodes.SetterRoot;
+import com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor;
 import com.oracle.graal.python.builtins.objects.cext.common.CArrayWrappers.CArrayWrapper;
 import com.oracle.graal.python.builtins.objects.cext.common.CExtCommonNodes.EnsureExecutableNode;
 import com.oracle.graal.python.builtins.objects.cext.common.CExtContext;
@@ -89,7 +89,6 @@
 import com.oracle.graal.python.builtins.objects.function.PBuiltinFunction;
 import com.oracle.graal.python.builtins.objects.getsetdescriptor.GetSetDescriptor;
 import com.oracle.graal.python.builtins.objects.object.PythonObject;
-import com.oracle.graal.python.builtins.objects.tuple.PTuple;
 import com.oracle.graal.python.builtins.objects.type.PythonAbstractClass;
 import com.oracle.graal.python.builtins.objects.type.PythonManagedClass;
 import com.oracle.graal.python.builtins.objects.type.SpecialMethodSlot;
@@ -106,7 +105,6 @@
 import com.oracle.graal.python.runtime.PythonContext;
 import com.oracle.graal.python.runtime.object.PythonObjectFactory;
 import com.oracle.graal.python.runtime.sequence.storage.MroSequenceStorage;
-import com.oracle.graal.python.runtime.sequence.storage.SequenceStorage;
 import com.oracle.graal.python.util.Function;
 import com.oracle.graal.python.util.PythonUtils;
 import com.oracle.truffle.api.CompilerDirectives;
@@ -217,51 +215,35 @@ static PDict doGeneric(PythonNativeClass nativeClass) {
         }
     }
 
-    @CApiBuiltin(ret = Int, args = {PyTypeObject, ConstCharPtrAsTruffleString, PyObject}, call = Ignored)
-    abstract static class PyTruffle_Type_Modified extends CApiTernaryBuiltinNode {
+    @CApiBuiltin(ret = ArgDescriptor.Void, args = {PyTypeObject}, call = Ignored)
+    abstract static class PyTruffle_InitializeOldStyleSlots extends CApiUnaryBuiltinNode {
 
         @TruffleBoundary
-        @Specialization(guards = "isNoValue(mroTuple)")
-        static int doIt(PythonAbstractNativeObject clazz, TruffleString name, @SuppressWarnings("unused") PNone mroTuple,
+        @Specialization
+        static Object doIt(PythonAbstractNativeObject clazz,
                         @Bind("this") Node inliningTarget) {
-            PythonContext context = PythonContext.get(inliningTarget);
-            CyclicAssumption nativeClassStableAssumption = context.getNativeClassStableAssumption(clazz, false);
-            if (nativeClassStableAssumption != null) {
-                nativeClassStableAssumption.invalidate("PyType_Modified(\"" + name.toJavaStringUncached() + "\") (without MRO) called");
-            }
-            SpecialMethodSlot.reinitializeSpecialMethodSlots(clazz, context.getLanguage());
-            // TODO: this is called from two places: at the end of PyType_Ready, and theoretically
-            // could be called from:
-            //
-            // void PyType_Modified(PyTypeObject* type) -> GraalPyTruffle_Type_Modified(type,
-            // type->tp_name, type->tp_mro);
-            //
-            // in unlikely (impossible?) case that type->tp_mro was NULL. Should we distinguish
-            // the two cases? As a cleanup if it is impossible situation (separate two different
-            // upcalls), or because at the end of PyType_Ready, we do not want to call
-            // TpSlots.updateAllSlots(clazz), but from PyType_Modified we do.
-            return 0;
+            SpecialMethodSlot.reinitializeSpecialMethodSlots(clazz, PythonLanguage.get(inliningTarget));
+            return PNone.NO_VALUE;
         }
+    }
+
+    @CApiBuiltin(ret = ArgDescriptor.Void, args = {PyTypeObject}, call = Direct)
+    abstract static class PyType_Modified extends CApiUnaryBuiltinNode {
 
         @TruffleBoundary
         @Specialization
-        static int doIt(PythonAbstractNativeObject clazz, TruffleString name, PTuple mroTuple,
+        static Object doIt(PythonAbstractNativeObject clazz,
                         @Bind("this") Node inliningTarget) {
             PythonContext context = PythonContext.get(inliningTarget);
             CyclicAssumption nativeClassStableAssumption = context.getNativeClassStableAssumption(clazz, false);
             if (nativeClassStableAssumption != null) {
-                nativeClassStableAssumption.invalidate("PyType_Modified(\"" + name.toJavaStringUncached() + "\") called");
+                nativeClassStableAssumption.invalidate("PyType_Modified(\"" + TypeNodes.GetNameNode.executeUncached(clazz).toJavaStringUncached() + "\") called");
             }
-            SequenceStorage sequenceStorage = mroTuple.getSequenceStorage();
-            if (sequenceStorage instanceof MroSequenceStorage) {
-                ((MroSequenceStorage) sequenceStorage).lookupChanged();
-            } else {
-                CompilerDirectives.transferToInterpreterAndInvalidate();
-                throw new IllegalStateException("invalid MRO object for native type \"" + name.toJavaStringUncached() + "\"");
-            }
-            SpecialMethodSlot.reinitializeSpecialMethodSlots(PythonNativeClass.cast(clazz), context.getLanguage());
+            MroSequenceStorage mroStorage = TypeNodes.GetMroStorageNode.executeUncached(clazz);
+            mroStorage.lookupChanged();
+            // Reload slots from native, which also invalidates cached slot lookups
             clazz.setTpSlots(TpSlots.fromNative(clazz, context));
-            return 0;
+            return PNone.NO_VALUE;
         }
     }
 

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/CApiFunction.java

@@ -482,7 +482,6 @@ public final class CApiFunction {
     @CApiBuiltin(name = "PyType_GetQualName", ret = PyObject, args = {PyTypeObject}, call = CImpl)
     @CApiBuiltin(name = "PyType_GetSlot", ret = Pointer, args = {PyTypeObject, Int}, call = CImpl)
     @CApiBuiltin(name = "PyType_IsSubtype", ret = Int, args = {PyTypeObject, PyTypeObject}, call = CImpl)
-    @CApiBuiltin(name = "PyType_Modified", ret = Void, args = {PyTypeObject}, call = CImpl)
     @CApiBuiltin(name = "PyType_Ready", ret = Int, args = {PyTypeObject}, call = CImpl)
     @CApiBuiltin(name = "PyUnicode_Append", ret = Void, args = {PyObjectPtr, PyObject}, call = CImpl)
     @CApiBuiltin(name = "PyUnicode_AppendAndDel", ret = Void, args = {PyObjectPtr, PyObject}, call = CImpl)