PyType_Modified should not modify native slots

Author: msimacek

graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_tp_slots.py

@@ -598,4 +598,32 @@ def __get__(self,obj,objtype=None): return "dummy"
         def __setattr__(self,name,value): return None
 
     assert 3 * Managed1() == 3
-    assert Managed1() * 3 == "__mul__result: 3"
+    assert Managed1() * 3 == "__mul__result: 3"
+
+
+def test_PyType_Modified_doesnt_change_slots():
+    TypeWithSqItemAndMpSubscr = CPyExtType(
+        "TypeWithSqItemAndMpSubscr",
+        code='''
+        static PyObject* sq_item(PyObject* self, Py_ssize_t index) {
+            return PyUnicode_FromString("sq_item");
+        }
+        static PyObject* mp_subscript(PyObject* self, PyObject* index) {
+            return PyUnicode_FromString("mp_subscript");
+        }
+        static PyObject* call_PySequence_GetItem(PyObject* self, PyObject* index) {
+            Py_ssize_t i = PyLong_AsSsize_t(index);
+            if (i == -1 && PyErr_Occurred())
+                return NULL;
+            return PySequence_GetItem(self, i);
+        }
+        ''',
+        sq_item="sq_item",
+        mp_subscript="mp_subscript",
+        tp_methods='{"call_PySequence_GetItem", (PyCFunction)call_PySequence_GetItem, METH_O, ""}',
+        post_ready_code="PyType_Modified(&TypeWithSqItemAndMpSubscrType);"
+    )
+    tester = TypeWithSqItemAndMpSubscr()
+    assert tester[1] == 'mp_subscript'
+    assert tester.call_PySequence_GetItem(1) == 'sq_item'
+

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

@@ -103,6 +103,7 @@
 import com.oracle.graal.python.nodes.attributes.WriteAttributeToPythonObjectNode;
 import com.oracle.graal.python.nodes.object.GetDictIfExistsNode;
 import com.oracle.graal.python.nodes.util.CastToTruffleStringNode;
+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;
@@ -127,7 +128,6 @@
 import com.oracle.truffle.api.object.DynamicObject;
 import com.oracle.truffle.api.object.DynamicObjectLibrary;
 import com.oracle.truffle.api.object.Shape;
-import com.oracle.truffle.api.profiles.InlinedExactClassProfile;
 import com.oracle.truffle.api.strings.TruffleString;
 import com.oracle.truffle.api.utilities.CyclicAssumption;
 
@@ -222,12 +222,14 @@ abstract static class PyTruffle_Type_Modified extends CApiTernaryBuiltinNode {
 
         @TruffleBoundary
         @Specialization(guards = "isNoValue(mroTuple)")
-        int doIt(PythonNativeClass clazz, TruffleString name, @SuppressWarnings("unused") PNone mroTuple) {
-            CyclicAssumption nativeClassStableAssumption = getContext().getNativeClassStableAssumption(clazz, false);
+        static int doIt(PythonAbstractNativeObject clazz, TruffleString name, @SuppressWarnings("unused") PNone mroTuple,
+                        @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(PythonNativeClass.cast(clazz), getLanguage());
+            SpecialMethodSlot.reinitializeSpecialMethodSlots(clazz, context.getLanguage());
             // TODO: this is called from two places: at the end of PyType_Ready, and theoretically
             // could be called from:
             //
@@ -243,22 +245,22 @@ int doIt(PythonNativeClass clazz, TruffleString name, @SuppressWarnings("unused"
 
         @TruffleBoundary
         @Specialization
-        int doIt(PythonNativeClass clazz, TruffleString name, PTuple mroTuple,
-                        @Bind("this") Node inliningTarget,
-                        @Cached InlinedExactClassProfile profile) {
-            CyclicAssumption nativeClassStableAssumption = getContext().getNativeClassStableAssumption(clazz, false);
+        static int doIt(PythonAbstractNativeObject clazz, TruffleString name, PTuple mroTuple,
+                        @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");
             }
-            SequenceStorage sequenceStorage = profile.profile(inliningTarget, mroTuple.getSequenceStorage());
+            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), getLanguage());
-            TpSlots.updateAllSlots(clazz);
+            SpecialMethodSlot.reinitializeSpecialMethodSlots(PythonNativeClass.cast(clazz), context.getLanguage());
+            clazz.setTpSlots(TpSlots.fromNative(clazz, context));
             return 0;
         }
     }