[GR-9549] List set items doesn't work for all slices combinations.

PullRequest: graalpython-open/24

Author: ppisl

graalpython/com.oracle.graal.python.test/src/tests/list_tests.py

@@ -39,13 +39,14 @@
 
 LONG_NUMBER = 6227020800;
 
+import list_tests
 
 
-class ListTest(seq_tests.CommonTest):
-
-    type2test = tuple
-
+class ListTest(list_tests.CommonTest):
+    type2test = list
+    
     # ======== Specific test for Graal Python ======
+
     def test_getitem(self):
         l = [1, 2, 3]
         self.assertEqual(1, l[False])
@@ -161,7 +162,6 @@ def test_slice(self):
         self.slice_test(list(range(0,20)), slice(-15,6,-5), [])
         self.slice_test(list(range(0,20)), slice(-2, -21, -4), [18, 14, 10, 6, 2])
 
-        
     def del_slice(self, l, s, expected):
         tmplist = list(l)
         del(tmplist[s])
@@ -172,7 +172,6 @@ def del_slice(self, l, s, expected):
         else:
             del(tmplist[s.start:s.stop:s.step])
         self.assertEqual(tmplist, expected, "del(list( slice({}, {}, {}))) expected: {}, get: {}".format(s.start, s.stop, s.step, expected, tmplist))
-
 
     def test_del_slice(self):
         self.del_slice(list(range(0, 20)), slice(1, 19), [0, 19])
@@ -202,3 +201,81 @@ def test_del_slice_step(self):
         self.del_slice(list(range(0, 20)), slice(-4, -18, -3), [0, 1, 2, 3, 5, 6, 8, 9, 11, 12, 14, 15, 17, 18, 19])
         self.del_slice(list(range(0, 20)), slice(-3, -55, -4), [0, 2, 3, 4, 6, 7, 8, 10, 11, 12, 14, 15, 16, 18, 19])
         self.del_slice(list(range(0, 20)), slice(-1, -55, -4), [0, 1, 2, 4, 5, 6, 8, 9, 10, 12, 13, 14, 16, 17, 18])
+
+    def test_ininicialization_with_slice(self):
+        r = []
+        l = [1,2,3,4]
+        r[:] = l
+        self.assertEqual(l, r)
+    
+    def test_set_slice(self):
+        a = [1,2]
+        a[1:2] = [7,6,5,4]
+        self.assertEqual([1, 7, 6, 5, 4], a)
+        a = [1, 2, 3, 4]
+        a[1:8] = [33]
+        self.assertEqual([1, 33], a)
+        a = [1,2,3,4]
+        a[1:8] = [33,34,35,36,37,38]
+        self.assertEqual([1, 33,34,35,36,37,38], a)
+        a = list(range(20))
+        a[1:19] = [55, 55]
+        self.assertEqual([0,55,55,19],a)
+        a = [1,2,3,4]
+        a[1:3] =[11] 
+        self.assertEqual([1, 11, 4], a)
+        a = [1,2,3,4]
+        a[1:3] =[11,12,13,14,15,16] 
+        self.assertEqual([1, 11,12,13,14,15,16, 4], a)
+        a = [1,2]
+        a[:] = (1, 2, 4, 5)
+        self.assertEqual([1,2,4,5], a)
+
+    def test_set_slice_class_iter(self):
+        class MyIter():
+            def __init__(self, base):
+                self.itera = iter(base)
+            def __next__(self):
+                return next(self.itera)
+            def __iter__(self):
+                return self
+
+        a = list(range(10))
+        a[::2] = MyIter(tuple(range(5)))
+        self.assertEqual([0, 1, 1, 3, 2, 5, 3, 7, 4, 9], a)
+
+    def test_set_slice_class_getitem(self):
+        class MyIter2():
+            def __init__(self, base):
+                self.base = base
+            def __getitem__(self, key):
+                return self.base[key]
+
+        a = [1,2,3,4]
+        a[2:] = MyIter2([33,44,55,66])
+        self.assertEqual([1,2,33,44,55,66], a)
+
+    def test_set_strange_slice(self):
+        a = list(range(20))
+        a[18:2] = [4,3,5]
+        self.assertEqual([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 4, 3, 5, 18, 19], a)
+        a = list(range(20))
+        a[18:0:-2] = [11,22,33,44,55,66,77,88,99]
+        self.assertEqual([0, 1, 99, 3, 88, 5, 77, 7, 66, 9, 55, 11, 44, 13, 33, 15, 22, 17, 11, 19], a)
+        a = list(range(20))
+        a[18:-5] = [11,22,33,44,55,66,77,88,99]
+        self.assertEqual([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 11, 22, 33, 44, 55, 66, 77, 88, 99, 18, 19], a)
+        a = list(range(20))
+        a[-2:-20:-2] = [11,22,33,44,55,66,77,88,99]
+        self.assertEqual([0, 1, 99, 3, 88, 5, 77, 7, 66, 9, 55, 11, 44, 13, 33, 15, 22, 17, 11, 19], a)
+        a = list(range(20))
+        a[20:-20] = [11,22,33,44,55,66,77,88,99]
+        self.assertEqual([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 11, 22, 33, 44, 55, 66, 77, 88, 99], a)
+
+    def test_set_slice_generalize_storage(self):
+        a = [1,2]
+        a[:] = 'ahoj'
+        self.assertEqual(['a', 'h', 'o', 'j'], a)
+        a = [1,2]
+        a[1:5] = [1.1, 2.2, 3.3]
+        self.assertEqual([1,1.1, 2.2, 3.3], a)

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

@@ -54,6 +54,7 @@
 import com.oracle.graal.python.nodes.PGuards;
 import com.oracle.graal.python.nodes.PNode;
 import com.oracle.graal.python.nodes.SpecialMethodNames;
+import com.oracle.graal.python.nodes.builtins.ListNodes;
 import com.oracle.graal.python.nodes.call.special.LookupAndCallBinaryNode;
 import com.oracle.graal.python.nodes.call.special.LookupAndCallUnaryNode;
 import com.oracle.graal.python.nodes.control.GetIteratorNode;
@@ -326,9 +327,9 @@ public abstract static class SetItemNode extends PythonTernaryBuiltinNode {
         @Child private NormalizeIndexNode normalize = NormalizeIndexNode.create();
 
         @Specialization
-        public Object doPList(PList primary, PSlice slice, PSequence value) {
-            primary.setSlice(slice, value);
-            return PNone.NONE;
+        public PNone doPList(PList list, PSlice slice, Object value,
+                        @Cached("create()") ListNodes.SetSliceNode sliceNode) {
+            return sliceNode.execute(list, slice, value);
         }
 
         @Specialization(guards = "isIntStorage(primary)")
@@ -401,7 +402,7 @@ protected boolean isValidIndexType(Object idx) {
     // list.append(x)
     @Builtin(name = "append", fixedNumOfArguments = 2)
     @GenerateNodeFactory
-    public abstract static class ListAppendNode extends PythonBuiltinNode {
+    public abstract static class ListAppendNode extends PythonBinaryBuiltinNode {
 
         @Specialization(guards = "isEmptyStorage(list)")
         public PNone appendEmpty(PList list, Object arg) {
@@ -444,7 +445,7 @@ public PNone appendTuple(PList list, PTuple arg) {
             return PNone.NONE;
         }
 
-        @Specialization(rewriteOn = {SequenceStoreException.class})
+        @Specialization(guards = {"!isKnownStorage(list)"}, rewriteOn = {SequenceStoreException.class})
         public PNone appendObject(PList list, Object arg) throws SequenceStoreException {
             list.getSequenceStorage().append(arg);
             return PNone.NONE;
@@ -455,6 +456,11 @@ public PNone appendObjectGeneric(PList list, Object arg) {
             list.append(arg);
             return PNone.NONE;
         }
+
+        protected boolean isKnownStorage(PList list) {
+            return PGuards.isEmptyStorage(list) || PGuards.isIntStorage(list) || PGuards.isLongStorage(list) || PGuards.isDoubleStorage(list) || PGuards.isListStorage(list) ||
+                            PGuards.isTupleStorage(list);
+        }
     }
 
     // list.extend(L)

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/list/ListBuiltins.java

@@ -67,29 +67,15 @@ public final Object getSlice(PythonObjectFactory factory, int start, int stop, i
 
     @Override
     public final void setSlice(PSlice slice, PSequence value) {
-        setSlice(slice.getStart(), slice.getStop(), slice.getStep(), value);
+        // Should not be used. Replaces with ListNodes.SetSliceNode.
+        // When it will be replaced in other PSequence implementeations,
+        // then the setSlice from PSequence can be removed.
+        throw new UnsupportedOperationException();
     }
 
     @Override
     public final void setSlice(int start, int stop, int step, PSequence value) {
-        final int normalizedStart = SequenceUtil.normalizeSliceStart(start, step, store.length(), "list assignment index out of range");
-        int normalizedStop = SequenceUtil.normalizeSliceStop(stop, step, store.length(), "list assignment index out of range");
-
-        if (normalizedStop < normalizedStart) {
-            normalizedStop = normalizedStart;
-        }
-
-        try {
-            store.setSliceInBound(normalizedStart, normalizedStop, step, value.getSequenceStorage());
-        } catch (SequenceStoreException e) {
-            store = store.generalizeFor(value.getSequenceStorage().getIndicativeValue());
-
-            try {
-                store.setSliceInBound(start, stop, step, value.getSequenceStorage());
-            } catch (SequenceStoreException ex) {
-                throw new IllegalStateException();
-            }
-        }
+        throw new UnsupportedOperationException();
     }
 
     @Override