[GR-9687] Reimplement list.remove node.

PullRequest: graalpython-open/33

Author: ppisl

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

@@ -316,7 +316,7 @@ def test_pop(self):
 #        self.assertRaises(IndexError, a.pop)
         self.assertRaises(TypeError, a.pop, 42, 42)
         a = self.type2test([0, 10, 20, 30, 40])
-    '''
+    
     def test_remove(self):
         a = self.type2test([0, 0, 1])
         a.remove(1)
@@ -361,7 +361,7 @@ def __eq__(self, other):
         for x, y in zip(d, e):
             # verify that original order and values are retained.
             self.assertIs(x, y)
-
+    '''
     def test_count(self):
         a = self.type2test([0, 1, 2])*3
         self.assertEqual(a.count(0), 3)

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

@@ -280,6 +280,29 @@ def test_set_slice_generalize_storage(self):
         a[1:5] = [1.1, 2.2, 3.3]
         self.assertEqual([1,1.1, 2.2, 3.3], a)
 
+    def test_remove_spec(self):
+        a = [1,2]
+        a.remove(2);
+        self.assertEqual([1], a)
+        a.remove(1)
+        self.assertEqual([], a)
+        
+        a = [0,1,0,1,2]
+        a.remove(True)
+        self.assertEqual([0,0,1,2], a)
+        a.remove(False)
+        self.assertEqual([0,1,2], a)
+
+        a = list([LONG_NUMBER, LONG_NUMBER + 1])
+        a.remove(LONG_NUMBER + 1)
+        self.assertEqual([LONG_NUMBER], a)
+        
+        class MyInt(int):
+            pass
+        
+        a = [1,2,3]
+        a.remove(MyInt(2))
+        self.assertEqual([1,3], a)
     def test_insert_spec(self):
         a = [1,2]
         self.assertRaises(TypeError, a.insert, [1,2,3], 1)

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

@@ -613,15 +613,73 @@ protected ListInsertNode createListInsertNode() {
     @GenerateNodeFactory
     public abstract static class ListRemoveNode extends PythonBuiltinNode {
 
-        @Specialization
-        public PList remove(PList list, Object arg) {
-            int index = list.index(arg);
-            if (index >= 0) {
-                list.delItem(index);
-                return list;
-            } else {
-                throw raise(PythonErrorType.ValueError, "list.remove(x): x not in list");
+        private static String NOT_IN_LIST_MESSAGE = "list.index(x): x not in list";
+
+        @Specialization(guards = "isIntStorage(list)")
+        public PNone removeInt(PList list, int value) {
+            IntSequenceStorage store = (IntSequenceStorage) list.getSequenceStorage();
+            for (int index = 0; index < store.length(); index++) {
+                if (value == store.getIntItemNormalized(index)) {
+                    store.delItemInBound(index);
+                    return PNone.NONE;
+                }
+            }
+            throw raise(PythonErrorType.ValueError, NOT_IN_LIST_MESSAGE);
+        }
+
+        @Specialization(guards = "isLongStorage(list)")
+        public PNone removeLong(PList list, int value) {
+            LongSequenceStorage store = (LongSequenceStorage) list.getSequenceStorage();
+            for (int index = 0; index < store.length(); index++) {
+                if (value == store.getLongItemNormalized(index)) {
+                    store.delItemInBound(index);
+                    return PNone.NONE;
+                }
+            }
+            throw raise(PythonErrorType.ValueError, NOT_IN_LIST_MESSAGE);
+        }
+
+        @Specialization(guards = "isLongStorage(list)")
+        public PNone removeLong(PList list, long value) {
+            LongSequenceStorage store = (LongSequenceStorage) list.getSequenceStorage();
+            for (int index = 0; index < store.length(); index++) {
+                if (value == store.getLongItemNormalized(index)) {
+                    store.delItemInBound(index);
+                    return PNone.NONE;
+                }
             }
+            throw raise(PythonErrorType.ValueError, NOT_IN_LIST_MESSAGE);
+        }
+
+        @Specialization(guards = "isDoubleStorage(list)")
+        public PNone removeDouble(PList list, double value) {
+            DoubleSequenceStorage store = (DoubleSequenceStorage) list.getSequenceStorage();
+            for (int index = 0; index < store.length(); index++) {
+                if (value == store.getDoubleItemNormalized(index)) {
+                    store.delItemInBound(index);
+                    return PNone.NONE;
+                }
+            }
+            throw raise(PythonErrorType.ValueError, NOT_IN_LIST_MESSAGE);
+        }
+
+        @Specialization(guards = "isNotSpecialCase(list, value)")
+        public PNone remove(PList list, Object value,
+                        @Cached("create(__EQ__, __EQ__, __EQ__)") BinaryComparisonNode eqNode) {
+            int len = list.len();
+            for (int i = 0; i < len; i++) {
+                Object object = list.getItem(i);
+                if (eqNode.executeBool(object, value)) {
+                    list.delItem(i);
+                    return PNone.NONE;
+                }
+            }
+            throw raise(PythonErrorType.ValueError, NOT_IN_LIST_MESSAGE);
+        }
+
+        protected boolean isNotSpecialCase(PList list, Object value) {
+            return !((PGuards.isIntStorage(list) && value instanceof Integer) || (PGuards.isLongStorage(list) && (value instanceof Integer || value instanceof Long)) ||
+                            PGuards.isDoubleStorage(list) && value instanceof Double);
         }
     }