define _PyList_SET_ITEM to map PyList_SET_ITEM macro behaviour

Author: timfel

graalpython/com.oracle.graal.python.cext/include/cpython/listobject.h

@@ -35,5 +35,6 @@ PyAPI_FUNC(void) _PyList_DebugMallocStats(FILE *out);
 #define _PyList_CAST(op) (assert(PyList_Check(op)), (PyListObject *)(op))
 
 #define PyList_GET_ITEM(op, i) (PyList_GetItem((PyObject *)(op), (i)))
-#define PyList_SET_ITEM(op, i, v) (PyList_SetItem((PyObject *)(op), (i), (v)))
+PyAPI_FUNC(void) _PyList_SET_ITEM(PyObject *, Py_ssize_t, PyObject *);
+#define PyList_SET_ITEM(op, i, v) (_PyList_SET_ITEM((PyObject *)(op), (i), (v)))
 #define PyList_GET_SIZE(op)    Py_SIZE(_PyList_CAST(op))

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

@@ -2397,6 +2397,10 @@ PyAPI_FUNC(void) _PyErr_WriteUnraisableMsg(const char* a, PyObject* b) {
 PyAPI_FUNC(PyObject*) _PyList_Extend(PyListObject* a, PyObject* b) {
     return Graal_PyList_Extend(a, b);
 }
+#undef _PyList_SET_ITEM
+PyAPI_FUNC(void) _PyList_SET_ITEM(PyObject* a, Py_ssize_t b, PyObject* c) {
+    Graal_PyList_SET_ITEM(a, b, c);
+}
 #undef _PyLong_Sign
 PyAPI_FUNC(int) _PyLong_Sign(PyObject* a) {
     return Graal_PyLong_Sign(a);

graalpython/com.oracle.graal.python.cext/src/capi.h

@@ -607,6 +607,7 @@ typedef struct {
     BUILTIN(_PyErr_BadInternalCall, void, const char*, int) \
     BUILTIN(_PyErr_WriteUnraisableMsg, void, const char*, PyObject*) \
     BUILTIN(_PyList_Extend, PyObject*, PyListObject*, PyObject*) \
+    BUILTIN(_PyList_SET_ITEM, void, PyObject*, Py_ssize_t, PyObject*) \
     BUILTIN(_PyLong_Sign, int, PyObject*) \
     BUILTIN(_PyNamespace_New, PyObject*, PyObject*) \
     BUILTIN(_PyNumber_Index, PyObject*, PyObject*) \

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

@@ -40,4 +40,6 @@
  */
 #include "capi.h"
 
-
+void* PyTruffle_NativeListItems(PyListObject* list) {
+    return polyglot_from_PyObjectPtr_array(list->ob_item, list->ob_base.ob_size);
+}

graalpython/com.oracle.graal.python.jni/src/capi_forwards.h

@@ -1361,6 +1361,7 @@ void unimplemented(const char* name) {
 #undef _PyInterpreterState_SetEvalFrameFunc
 #undef _PyList_DebugMallocStats
 #undef _PyList_Extend
+#undef _PyList_SET_ITEM
 #undef _PyLong_AsByteArray
 #undef _PyLong_AsInt
 #undef _PyLong_AsTime_t
@@ -5163,6 +5164,9 @@ PyAPI_FUNC(PyObject*) _PyList_Extend(PyListObject* a, PyObject* b) {
     PyObject* result = (PyObject*) Graal_PyList_Extend(a, b);
     return result;
 }
+PyAPI_FUNC(void) _PyList_SET_ITEM(PyObject* a, Py_ssize_t b, PyObject* c) {
+    Graal_PyList_SET_ITEM(a, b, c);
+}
 PyAPI_FUNC(time_t) _PyLong_AsTime_t(PyObject* a) {
     unimplemented("_PyLong_AsTime_t"); exit(-1);
 }

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

@@ -41,7 +41,7 @@
 from . import CPyExtTestCase, CPyExtFunction, CPyExtFunctionOutVars, unhandled_error_compare, GRAALPYTHON
 __dir__ = __file__.rpartition("/")[0]
 
-    
+
 def _reference_new_list(args):
     n = args[0]
     if n < 0:
@@ -199,13 +199,26 @@ def compile_module(self, name):
     test_PyList_SET_ITEM = CPyExtFunction(
         _wrap_list_fun(_reference_SET_ITEM),
         lambda: (
-            ([1,2,3,4], 0, 0),
-            ([1,2,3,4], 3, 5),
+            ([1,2,3,4], 0, _reference_SET_ITEM),
+            ([1,2,3,4], 3, _reference_SET_ITEM),
         ),
         code='''PyObject* wrap_PyList_SET_ITEM(PyObject* op, Py_ssize_t idx, PyObject* newitem) {
             Py_INCREF(newitem);
             PyList_SET_ITEM(op, idx, newitem);
-            return op;
+            int refcnt1 = Py_REFCNT(newitem);
+            PyList_SET_ITEM(op, idx, Py_None);
+            int refcnt2 = Py_REFCNT(newitem);
+            PyList_SET_ITEM(op, idx, newitem);
+            int refcnt3 = Py_REFCNT(newitem);
+            // Important! PyList_SET_ITEM does not change the refcnts,
+            // but PyList_SetItem decrefs the element that was
+            // previously at idx. Let's make sure that semantic
+            // difference is observed
+            if (refcnt1 != refcnt2 || refcnt2 != refcnt3) {
+                return Py_BuildValue("iii", refcnt1, refcnt2, refcnt3);
+            } else {
+                return op;
+            }
         }
         ''',
         resultspec="O",

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

@@ -44,6 +44,7 @@
 import static com.oracle.graal.python.builtins.PythonBuiltinClassType.SystemError;
 import static com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiCallPath.Direct;
 import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.Int;
+import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.Void;
 import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.PyListObject;
 import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.PyObject;
 import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.PyObjectBorrowed;
@@ -61,10 +62,13 @@
 import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiUnaryBuiltinNode;
 import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.PromoteBorrowedValue;
 import com.oracle.graal.python.builtins.objects.PNone;
+import com.oracle.graal.python.builtins.objects.cext.PythonAbstractNativeObject;
 import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes;
 import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.GetItemScalarNode;
 import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.ListGeneralizationNode;
+import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.SetItemNode;
 import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.SetItemScalarNode;
+import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodesFactory.GetItemScalarNodeGen;
 import com.oracle.graal.python.builtins.objects.list.ListBuiltins;
 import com.oracle.graal.python.builtins.objects.list.ListBuiltins.ListExtendNode;
 import com.oracle.graal.python.builtins.objects.list.ListBuiltins.ListInsertNode;
@@ -73,7 +77,9 @@
 import com.oracle.graal.python.lib.PySliceNew;
 import com.oracle.graal.python.nodes.ErrorMessages;
 import com.oracle.graal.python.nodes.builtins.ListNodes.AppendNode;
+import com.oracle.graal.python.nodes.builtins.ListNodes.GetNativeListStorage;
 import com.oracle.graal.python.nodes.builtins.TupleNodes.ConstructTupleNode;
+import com.oracle.graal.python.runtime.sequence.storage.NativeSequenceStorage;
 import com.oracle.graal.python.runtime.sequence.storage.SequenceStorage;
 import com.oracle.graal.python.util.PythonUtils;
 import com.oracle.truffle.api.dsl.Cached;
@@ -287,6 +293,49 @@ int fallback(Object list, @SuppressWarnings("unused") Object i, @SuppressWarning
         }
     }
 
+    @CApiBuiltin(ret = Void, args = {PyObject, Py_ssize_t, PyObject}, call = Direct)
+    abstract static class _PyList_SET_ITEM extends CApiTernaryBuiltinNode {
+        @Specialization
+        int doManaged(PList list, long index, Object element,
+                        @Cached("createSetItem()") SequenceStorageNodes.SetItemNode setItemNode,
+                        @Cached ConditionProfile generalizedProfile) {
+            SequenceStorage sequenceStorage = list.getSequenceStorage();
+            checkBounds(sequenceStorage, index);
+            SequenceStorage newStorage = setItemNode.execute(null, sequenceStorage, (int) index, element);
+            if (generalizedProfile.profile(list.getSequenceStorage() != newStorage)) {
+                list.setSequenceStorage(newStorage);
+            }
+            return 0;
+        }
+
+        @Specialization
+        int doNative(PythonAbstractNativeObject list, long index, Object element,
+                        @Cached GetNativeListStorage asNativeStorage,
+                        @Cached SequenceStorageNodes.InitializeNativeItemScalarNode setItemNode) {
+            NativeSequenceStorage sequenceStorage = asNativeStorage.execute(list);
+            checkBounds(sequenceStorage, index);
+            setItemNode.execute(sequenceStorage, (int) index, element);
+            return 0;
+        }
+
+        @Fallback
+        @SuppressWarnings("unused")
+        Object fallback(Object list, Object index, Object element) {
+            throw raiseFallback(list, PythonBuiltinClassType.PList);
+        }
+
+        private void checkBounds(SequenceStorage sequenceStorage, long index) {
+            // we must do a bounds-check but we must not normalize the index
+            if (index < 0 || index >= sequenceStorage.length()) {
+                throw raise(IndexError, ErrorMessages.INDEX_OUT_OF_BOUNDS);
+            }
+        }
+
+        protected static SetItemNode createSetItem() {
+            return SetItemNode.create(null, ListGeneralizationNode::create);
+        }
+    }
+
     @CApiBuiltin(ret = Int, args = {PyObject}, call = Direct)
     abstract static class PyList_Reverse extends CApiUnaryBuiltinNode {
         @Specialization

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

@@ -218,6 +218,7 @@ public enum NativeCAPISymbol implements NativeCExtSymbol {
     FUN_PY_TRUFFLE_OBJECT_ARRAY_FREE("PyTruffle_ObjectArrayFree"),
     FUN_PY_TRUFFLE_SET_STORAGE_ITEM("PyTruffle_SetStorageItem"),
     FUN_PY_TRUFFLE_INITIALIZE_STORAGE_ITEM("PyTruffle_InitializeStorageItem"),
+    FUN_PY_TRUFFLE_NATIVE_LIST_ITEMS("PyTruffle_NativeListItems"),
     FUN_PY_TRUFFLE_NATIVE_TUPLE_ITEMS("PyTruffle_NativeTupleItems"),
     FUN_PY_TRUFFLE_NATIVE_BYTES_ITEMS("PyTruffle_NativeBytesItems"),
     FUN_PY_OBJECT_GENERIC_GET_DICT("_PyObject_GenericGetDict"),

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/builtins/ListNodes.java

@@ -48,6 +48,9 @@
 import com.oracle.graal.python.builtins.PythonBuiltinClassType;
 import com.oracle.graal.python.builtins.modules.MathGuards;
 import com.oracle.graal.python.builtins.objects.PNone;
+import com.oracle.graal.python.builtins.objects.cext.PythonAbstractNativeObject;
+import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.PCallCapiFunction;
+import com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol;
 import com.oracle.graal.python.builtins.objects.common.SequenceNodes;
 import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes;
 import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.ListGeneralizationNode;
@@ -66,19 +69,24 @@
 import com.oracle.graal.python.nodes.builtins.ListNodesFactory.ConstructListNodeGen;
 import com.oracle.graal.python.nodes.builtins.ListNodesFactory.IndexNodeGen;
 import com.oracle.graal.python.nodes.call.special.LookupAndCallUnaryNode;
+import com.oracle.graal.python.nodes.classes.IsSubtypeNode;
 import com.oracle.graal.python.nodes.object.GetClassNode;
+import com.oracle.graal.python.nodes.object.InlinedGetClassNode;
 import com.oracle.graal.python.nodes.truffle.PythonArithmeticTypes;
 import com.oracle.graal.python.nodes.util.CastToTruffleStringNode;
 import com.oracle.graal.python.runtime.PythonOptions;
 import com.oracle.graal.python.runtime.object.PythonObjectFactory;
 import com.oracle.graal.python.runtime.sequence.PSequence;
 import com.oracle.graal.python.runtime.sequence.storage.BasicSequenceStorage;
+import com.oracle.graal.python.runtime.sequence.storage.NativeSequenceStorage;
 import com.oracle.graal.python.runtime.sequence.storage.SequenceStorage;
+import com.oracle.graal.python.runtime.sequence.storage.SequenceStorage.ListStorageType;
 import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.dsl.Bind;
 import com.oracle.truffle.api.dsl.Cached;
 import com.oracle.truffle.api.dsl.Cached.Shared;
 import com.oracle.truffle.api.dsl.Fallback;
+import com.oracle.truffle.api.dsl.GenerateInline;
 import com.oracle.truffle.api.dsl.GenerateUncached;
 import com.oracle.truffle.api.dsl.Idempotent;
 import com.oracle.truffle.api.dsl.ImportStatic;
@@ -87,6 +95,9 @@
 import com.oracle.truffle.api.dsl.TypeSystemReference;
 import com.oracle.truffle.api.frame.Frame;
 import com.oracle.truffle.api.frame.VirtualFrame;
+import com.oracle.truffle.api.interop.InteropLibrary;
+import com.oracle.truffle.api.interop.UnsupportedMessageException;
+import com.oracle.truffle.api.library.CachedLibrary;
 import com.oracle.truffle.api.nodes.Node;
 import com.oracle.truffle.api.profiles.BranchProfile;
 import com.oracle.truffle.api.strings.TruffleString;
@@ -346,4 +357,23 @@ public static AppendNode getUncached() {
             return AppendNodeGen.getUncached();
         }
     }
+
+    @GenerateInline(false)
+    public abstract static class GetNativeListStorage extends Node {
+        public abstract NativeSequenceStorage execute(PythonAbstractNativeObject list);
+
+        @Specialization
+        NativeSequenceStorage getNative(PythonAbstractNativeObject list,
+                        @CachedLibrary(limit = "1") InteropLibrary lib,
+                        @Cached PCallCapiFunction callCapiFunction) {
+            assert IsSubtypeNode.getUncached().execute(InlinedGetClassNode.executeUncached(list), PythonBuiltinClassType.PList);
+            try {
+                Object interopArray = callCapiFunction.call(NativeCAPISymbol.FUN_PY_TRUFFLE_NATIVE_LIST_ITEMS, list.getPtr());
+                int size = (int) lib.getArraySize(interopArray);
+                return NativeSequenceStorage.create(interopArray, size, size, ListStorageType.Generic, false);
+            } catch (UnsupportedMessageException e) {
+                throw CompilerDirectives.shouldNotReachHere(e);
+            }
+        }
+    }
 }