[GR-9687] Reimplement list.remove node.

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

@@ -279,3 +279,27 @@ def test_set_slice_generalize_storage(self):
         a = [1,2]
         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)

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

@@ -519,16 +519,76 @@ public PList insert(PList list, Object i, Object arg1) {
     @Builtin(name = "remove", fixedNumOfArguments = 2)
     @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.popInBound(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.popInBound(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.popInBound(index);
+                    return PNone.NONE;
+                }
+            }
+            throw raise(PythonErrorType.ValueError, NOT_IN_LIST_MESSAGE);
+        }
+        
+        @Specialization(guards = "isDoubleStorage(list)")
+        public PNone removeLong(PList list, double value) {
+            DoubleSequenceStorage store = (DoubleSequenceStorage)list.getSequenceStorage();
+            for(int index = 0; index < store.length(); index++) {
+                if (value == store.getDoubleItemNormalized(index)) {
+                    store.popInBound(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("createBinaryProfile()") ConditionProfile errorProfile,
+                @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);
         }
     }