[pypy-commit] pypy list-strategies: (l.diekmann, cfbolz):

[l.diekmann]

Author: Lukas Diekmann <lukas.diekmann at uni-duesseldorf.de>
Branch: list-strategies
Changeset: r47489:c9f9f040c318
Date: 2011-03-29 13:39 +0200
http://bitbucket.org/pypy/pypy/changeset/c9f9f040c318/

Log:	(l.diekmann, cfbolz): Implemented getitems_copy used in
	objspace.unpackiterable (+ tests) Now getitems() in
	ObjectListStrategy doesn't copy anymore

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
@@ -105,6 +105,8 @@
     def getitems(self):
         return self.strategy.getitems(self)
 
+    def getitems_copy(self):
+        return self.strategy.getitems_copy(self)
     # ___________________________________________________
 
     def inplace_mul(self, times):
@@ -160,6 +162,9 @@
     def getitems(self, w_list):
         raise NotImplementedError
 
+    def getitems_copy(self, w_list):
+        raise NotImplementedError
+
     def append(self, w_list, w_item):
         raise NotImplementedError
 
@@ -216,6 +221,9 @@
         # cache result XXX
         return []
 
+    def getitems_copy(self, w_list):
+        return []
+
     def append(self, w_list, w_item):
         w_list.__init__(self.space, [w_item])
 
@@ -290,6 +298,8 @@
     def getitems(self, w_list):
         return self._getitems_range(w_list, True)
 
+    getitems_copy = getitems
+
     @specialize.arg(2)
     def _getitems_range(self, w_list, wrap_items):
         l = self.cast_from_void_star(w_list.lstorage)
@@ -435,9 +445,11 @@
             raise
         return self.wrap(r)
 
-    def getitems(self, w_list):
+    def getitems_copy(self, w_list):
         return [self.wrap(item) for item in self.cast_from_void_star(w_list.lstorage)]
 
+    getitems = getitems_copy
+
     def getslice(self, w_list, start, stop, step, length):
         if step == 1:
             l = self.cast_from_void_star(w_list.lstorage)
@@ -630,7 +642,8 @@
     def init_from_list_w(self, w_list, list_w):
         w_list.lstorage = self.cast_to_void_star(list_w)
 
-    # XXX implement getitems without copying here
+    def getitems(self, w_list):
+        return self.cast_from_void_star(w_list.lstorage)
 
 class IntegerListStrategy(AbstractUnwrappedStrategy, ListStrategy):
     _none_value = 0
@@ -1047,7 +1060,7 @@
         # The list is temporarily made empty, so that mutations performed
         # by comparison functions can't affect the slice of memory we're
         # sorting (allowing mutations during sorting is an IndexError or
-        # core-dump factory, since wrappeditems may change).
+        # core-dump factory, since the storage may change).
         w_list.__init__(space, [])
 
         # wrap each item in a KeyContainer if needed
diff --git a/pypy/objspace/std/objspace.py b/pypy/objspace/std/objspace.py
--- a/pypy/objspace/std/objspace.py
+++ b/pypy/objspace/std/objspace.py
@@ -387,8 +387,8 @@
     def unpackiterable(self, w_obj, expected_length=-1):
         if isinstance(w_obj, W_TupleObject):
             t = w_obj.wrappeditems[:]
-        elif isinstance(w_obj, W_ListObject): # XXX enable fast path again
-            t = w_obj.getitems()
+        elif isinstance(w_obj, W_ListObject):
+            t = w_obj.getitems_copy()
         else:
             return ObjSpace.unpackiterable(self, w_obj, expected_length)
         if expected_length != -1 and len(t) != expected_length:
@@ -402,6 +402,7 @@
         if isinstance(w_obj, W_TupleObject):
             t = w_obj.wrappeditems
         elif isinstance(w_obj, W_ListObject):
+            # XXX this can copy twice
             t = w_obj.getitems()[:]
         else:
             if unroll:
diff --git a/pypy/objspace/std/test/test_liststrategies.py b/pypy/objspace/std/test/test_liststrategies.py
--- a/pypy/objspace/std/test/test_liststrategies.py
+++ b/pypy/objspace/std/test/test_liststrategies.py
@@ -232,3 +232,15 @@
         assert isinstance(l.strategy, RangeListStrategy)
         l.setslice(0, 1, 3, [self.space.wrap(1), self.space.wrap(2), self.space.wrap(3)])
         assert isinstance(l.strategy, IntegerListStrategy)
+
+    def test_get_items_copy(self):
+        l1 = W_ListObject(self.space, [self.space.wrap(1), self.space.wrap(2), self.space.wrap(3)])
+        l2 = l1.getitems()
+        l2.append(self.space.wrap(4))
+        assert not l2 == l1.getitems()
+
+        l1 = W_ListObject(self.space, [self.space.wrap(1), self.space.wrap("two"), self.space.wrap(3)])
+        l2 = l1.getitems()
+        l2.append(self.space.wrap("four"))
+        assert l2 == l1.getitems()
+