[pypy-commit] pypy list-strategies: fixed bug with index on pop

l.diekmann noreply at buildbot.pypy.org
Fri Sep 23 13:15:27 CEST 2011


Author: Lukas Diekmann <lukas.diekmann at uni-duesseldorf.de>
Branch: list-strategies
Changeset: r47545:0eb7982b4d22
Date: 2011-09-13 14:29 +0200
http://bitbucket.org/pypy/pypy/changeset/0eb7982b4d22/

Log:	fixed bug with index on pop

diff --git a/pypy/objspace/std/listobject.py b/pypy/objspace/std/listobject.py
--- a/pypy/objspace/std/listobject.py
+++ b/pypy/objspace/std/listobject.py
@@ -141,6 +141,7 @@
         self.strategy.deleteslice(self, start, step, length)
 
     def pop(self, index):
+        # XXX docstring: index already checked
         return self.strategy.pop(self, index)
 
     def setitem(self, index, w_item):
@@ -718,6 +719,8 @@
         l = self.unerase(w_list.lstorage)
         # not sure if RPython raises IndexError on pop
         # so check again here
+        if index < 0:
+            raise IndexError
         try:
             item = l.pop(index)
         except IndexError:
@@ -1144,10 +1147,6 @@
         raise OperationError(space.w_IndexError,
                              space.wrap("pop from empty list"))
     idx = space.int_w(w_idx)
-    # XXX this shows that the methods on W_ListObject clearly need to say
-    # whether their arguments are index-checked or not. because this is clearly
-    # a bug here: if idx < 0, you add the length. pop will do the same thing
-    # again!
     if idx < 0:
         idx += length
     try:
diff --git a/pypy/objspace/std/test/test_listobject.py b/pypy/objspace/std/test/test_listobject.py
--- a/pypy/objspace/std/test/test_listobject.py
+++ b/pypy/objspace/std/test/test_listobject.py
@@ -746,6 +746,22 @@
         l.pop()
         assert l == range(9)
 
+    def test_pop_negative(self):
+        l1 = [1,2,3,4]
+        l2 = ["1", "2", "3", "4"]
+        l3 = range(5)
+        l4 = [1, 2, 3, "4"]
+
+        raises(IndexError, l1.pop, -5)
+        raises(IndexError, l2.pop, -5)
+        raises(IndexError, l3.pop, -6)
+        raises(IndexError, l4.pop, -5)
+
+        assert l1.pop(-2) == 3
+        assert l2.pop(-2) == "3"
+        assert l3.pop(-2) == 3
+        assert l4.pop(-2) == 3
+
     def test_remove(self):
         c = list('hello world')
         c.remove('l')


More information about the pypy-commit mailing list