Remove methods 'SequenceStorage.delItemInBound/popInBound'.

Author: fangerer

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

@@ -470,6 +470,14 @@ class MyInt(int):
         a.remove(MyInt(2))
         self.assertEqual([1, 3], a)
 
+        a = ["1", "2"]
+        a.remove("2");
+        self.assertEqual(["1"], a)
+
+        a = [1.1, 2.2, 3.3]
+        a.remove(2.2);
+        self.assertEqual([1.1, 3.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/common/SequenceStorageNodes.java

@@ -2708,11 +2708,11 @@ public DeleteNode(NormalizeIndexNode normalizeIndexNode, String keyTypeErrorMess
             this.keyTypeErrorMessage = keyTypeErrorMessage;
         }
 
-        public abstract void execute(SequenceStorage s, Object key);
+        public abstract void execute(SequenceStorage s, Object indexOrSlice);
 
-        public abstract void execute(SequenceStorage s, int key);
+        public abstract void execute(SequenceStorage s, int index);
 
-        public abstract void execute(SequenceStorage s, long key);
+        public abstract void execute(SequenceStorage s, long index);
 
         @Specialization
         protected void doScalarInt(SequenceStorage storage, int idx) {

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

@@ -77,6 +77,7 @@
 import com.oracle.graal.python.nodes.control.GetIteratorNode;
 import com.oracle.graal.python.nodes.control.GetNextNode;
 import com.oracle.graal.python.nodes.expression.BinaryComparisonNode;
+import com.oracle.graal.python.nodes.expression.CastToBooleanNode;
 import com.oracle.graal.python.nodes.function.PythonBuiltinBaseNode;
 import com.oracle.graal.python.nodes.function.PythonBuiltinNode;
 import com.oracle.graal.python.nodes.function.builtins.PythonBinaryBuiltinNode;
@@ -400,78 +401,28 @@ protected ListInsertNode createListInsertNode() {
     // list.remove(x)
     @Builtin(name = "remove", fixedNumOfPositionalArgs = 2)
     @GenerateNodeFactory
-    public abstract static class ListRemoveNode extends PythonBuiltinNode {
+    public abstract static class ListRemoveNode extends PythonBinaryBuiltinNode {
 
-        private static String NOT_IN_LIST_MESSAGE = "list.index(x): x not in list";
+        private static final 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)")
+        @Specialization
         public PNone remove(PList list, Object value,
                         @Cached("createNotNormalized()") SequenceStorageNodes.GetItemNode getItemNode,
+                        @Cached("create()") SequenceStorageNodes.DeleteNode deleteNode,
+                        @Cached("create()") SequenceStorageNodes.LenNode lenNode,
+                        @Cached("createIfTrueNode()") CastToBooleanNode castToBooleanNode,
                         @Cached("create(__EQ__, __EQ__, __EQ__)") BinaryComparisonNode eqNode) {
-            int len = list.len();
             SequenceStorage listStore = list.getSequenceStorage();
+            int len = lenNode.execute(listStore);
             for (int i = 0; i < len; i++) {
                 Object object = getItemNode.execute(listStore, i);
-                if (eqNode.executeBool(object, value)) {
-                    listStore.delItemInBound(i);
+                if (castToBooleanNode.executeWith(eqNode.executeWith(object, value))) {
+                    deleteNode.execute(listStore, 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);
-        }
     }
 
     // list.pop([i])

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/sequence/storage/BoolSequenceStorage.java

@@ -171,27 +171,6 @@ public void setBoolSliceInBound(int start, int stop, int step, BoolSequenceStora
         length = length > stop ? length : stop;
     }
 
-    @Override
-    public void delItemInBound(int idx) {
-        if (values.length - 1 == idx) {
-            popBool();
-        } else {
-            popInBound(idx);
-        }
-    }
-
-    @Override
-    public Object popInBound(int idx) {
-        boolean pop = values[idx];
-
-        for (int i = idx; i < values.length - 1; i++) {
-            values[i] = values[i + 1];
-        }
-
-        length--;
-        return pop;
-    }
-
     public boolean popBool() {
         boolean pop = values[capacity - 1];
         length--;

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/sequence/storage/ByteSequenceStorage.java

@@ -253,27 +253,6 @@ public void setByteSliceInBound(int start, int stop, int step, ByteSequenceStora
         length = newLength;
     }
 
-    @Override
-    public void delItemInBound(int idx) {
-        if (values.length - 1 == idx) {
-            popInt();
-        } else {
-            popInBound(idx);
-        }
-    }
-
-    @Override
-    public Object popInBound(int idx) {
-        int pop = values[idx] & 0xFF;
-
-        for (int i = idx; i < values.length - 1; i++) {
-            values[i] = values[i + 1];
-        }
-
-        length--;
-        return pop;
-    }
-
     public int popInt() {
         int pop = values[capacity - 1] & 0xFF;
         length--;

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/sequence/storage/CharSequenceStorage.java

@@ -114,16 +114,6 @@ public CharSequenceStorage getSliceInBound(int start, int stop, int step, int sl
         throw new UnsupportedOperationException();
     }
 
-    @Override
-    public void delItemInBound(int idx) {
-        throw new UnsupportedOperationException();
-    }
-
-    @Override
-    public Object popInBound(int idx) {
-        throw new UnsupportedOperationException();
-    }
-
     @Override
     public int index(Object value) {
         throw new UnsupportedOperationException();

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/sequence/storage/DoubleSequenceStorage.java

@@ -179,23 +179,6 @@ public void setDoubleSliceInBound(int start, int stop, int step, DoubleSequenceS
         length = length > stop ? length : stop;
     }
 
-    @Override
-    public void delItemInBound(int idx) {
-        popInBound(idx);
-    }
-
-    @Override
-    public Object popInBound(int idx) {
-        double pop = values[idx];
-
-        for (int i = idx; i < values.length - 1; i++) {
-            values[i] = values[i + 1];
-        }
-
-        length--;
-        return pop;
-    }
-
     public double popDouble() {
         double pop = values[length - 1];
         length--;

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/sequence/storage/EmptySequenceStorage.java

@@ -136,16 +136,6 @@ public SequenceStorage getSliceInBound(int start, int stop, int step, int length
         return this;
     }
 
-    @Override
-    public void delItemInBound(int idx) {
-        throw new UnsupportedOperationException("Cannot delete from empty storage");
-    }
-
-    @Override
-    public Object popInBound(int idx) {
-        return new UnsupportedOperationException();
-    }
-
     @Override
     public void reverse() {
     }

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/sequence/storage/IntSequenceStorage.java

@@ -179,27 +179,6 @@ public void setIntSliceInBound(int start, int stop, int step, IntSequenceStorage
         length = length > stop ? length : stop;
     }
 
-    @Override
-    public void delItemInBound(int idx) {
-        if (values.length - 1 == idx) {
-            popInt();
-        } else {
-            popInBound(idx);
-        }
-    }
-
-    @Override
-    public Object popInBound(int idx) {
-        int pop = values[idx];
-
-        for (int i = idx; i < values.length - 1; i++) {
-            values[i] = values[i + 1];
-        }
-
-        length--;
-        return pop;
-    }
-
     public int popInt() {
         int pop = values[length - 1];
         length--;

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/sequence/storage/ListSequenceStorage.java

@@ -205,27 +205,6 @@ public void setListSliceInBound(int start, int stop, int step, ListSequenceStora
         length = length > stop ? length : stop;
     }
 
-    @Override
-    public void delItemInBound(int idx) {
-        if (values.length - 1 == idx) {
-            popList();
-        } else {
-            popInBound(idx);
-        }
-    }
-
-    @Override
-    public Object popInBound(int idx) {
-        PList pop = values[idx];
-
-        for (int i = idx; i < values.length - 1; i++) {
-            values[i] = values[i + 1];
-        }
-
-        length--;
-        return pop;
-    }
-
     public PList popList() {
         PList pop = values[capacity - 1];
         length--;