[pypy-commit] pypy cpyext-gc-support: Fix PyDict_Next (still broken complexity)

[arigo]

Author: Armin Rigo <arigo at tunes.org>
Branch: cpyext-gc-support
Changeset: r80409:03b0d3536b4c
Date: 2015-10-23 14:03 +0200
http://bitbucket.org/pypy/pypy/changeset/03b0d3536b4c/

Log:	Fix PyDict_Next (still broken complexity)

diff --git a/pypy/module/cpyext/dictobject.py b/pypy/module/cpyext/dictobject.py
--- a/pypy/module/cpyext/dictobject.py
+++ b/pypy/module/cpyext/dictobject.py
@@ -2,7 +2,7 @@
 from pypy.module.cpyext.api import (
     cpython_api, CANNOT_FAIL, build_type_checkers, Py_ssize_t,
     Py_ssize_tP, CONST_STRING)
-from pypy.module.cpyext.pyobject import PyObject, PyObjectP, as_xpyobj
+from pypy.module.cpyext.pyobject import PyObject, PyObjectP, as_xpyobj, as_pyobj
 from pypy.module.cpyext.pyerrors import PyErr_BadInternalCall
 from pypy.interpreter.error import OperationError
 from pypy.objspace.std.dictmultiobject import W_DictMultiObject
@@ -164,28 +164,26 @@
         }
         Py_DECREF(o);
     }"""
-    if w_dict is None:
+    if not isinstance(w_dict, W_DictMultiObject):
         return 0
 
-    # Note: this is not efficient. Storing an iterator would probably
+    # XXX XXX PyDict_Next is not efficient. Storing an iterator would probably
     # work, but we can't work out how to not leak it if iteration does
-    # not complete.
-    ZZZ
+    # not complete.  Alternatively, we could add some RPython-only
+    # dict-iterator method to move forward by N steps.
 
+    w_dict.ensure_object_strategy()
+    w_iter = space.call_method(space.w_dict, "iteritems", w_dict)
     try:
-        w_iter = space.call_method(space.w_dict, "iteritems", w_dict)
-        pos = ppos[0]
-        while pos:
+        for i in range(ppos[0]):
             space.call_method(w_iter, "next")
-            pos -= 1
 
         w_item = space.call_method(w_iter, "next")
         w_key, w_value = space.fixedview(w_item, 2)
-        state = space.fromcache(RefcountState)
         if pkey:
-            pkey[0]   = state.make_borrowed(w_dict, w_key)
+            pkey[0]   = as_pyobj(space, w_key)
         if pvalue:
-            pvalue[0] = state.make_borrowed(w_dict, w_value)
+            pvalue[0] = as_pyobj(space, w_value)
         ppos[0] += 1
     except OperationError, e:
         if not e.match(space, space.w_StopIteration):
diff --git a/pypy/module/cpyext/test/test_dictobject.py b/pypy/module/cpyext/test/test_dictobject.py
--- a/pypy/module/cpyext/test/test_dictobject.py
+++ b/pypy/module/cpyext/test/test_dictobject.py
@@ -1,7 +1,7 @@
 from rpython.rtyper.lltypesystem import rffi, lltype
 from pypy.module.cpyext.test.test_api import BaseApiTest
 from pypy.module.cpyext.api import Py_ssize_tP, PyObjectP
-from pypy.module.cpyext.pyobject import from_pyobj
+from pypy.module.cpyext.pyobject import from_pyobj, as_pyobj
 from pypy.interpreter.error import OperationError
 
 class TestDictObject(BaseApiTest):
@@ -91,7 +91,6 @@
 
     def test_iter(self, space, api):
         w_dict = space.sys.getdict(space)
-        py_dict = make_ref(space, w_dict)
 
         ppos = lltype.malloc(Py_ssize_tP.TO, 1, flavor='raw')
         ppos[0] = 0
@@ -109,14 +108,11 @@
             lltype.free(pkey, flavor='raw')
             lltype.free(pvalue, flavor='raw')
 
-        api.Py_DecRef(py_dict) # release borrowed references
-
         assert space.eq_w(space.len(w_copy), space.len(w_dict))
         assert space.eq_w(w_copy, w_dict)
 
     def test_iterkeys(self, space, api):
         w_dict = space.sys.getdict(space)
-        py_dict = make_ref(space, w_dict)
 
         ppos = lltype.malloc(Py_ssize_tP.TO, 1, flavor='raw')
         pkey = lltype.malloc(PyObjectP.TO, 1, flavor='raw')
@@ -138,8 +134,6 @@
             lltype.free(pkey, flavor='raw')
             lltype.free(pvalue, flavor='raw')
 
-        api.Py_DecRef(py_dict) # release borrowed references
-
         assert space.eq_w(space.newlist(keys_w),
                           space.call_method(w_dict, "keys"))
         assert space.eq_w(space.newlist(values_w),
diff --git a/pypy/objspace/std/dictmultiobject.py b/pypy/objspace/std/dictmultiobject.py
--- a/pypy/objspace/std/dictmultiobject.py
+++ b/pypy/objspace/std/dictmultiobject.py
@@ -336,6 +336,11 @@
         F: D[k] = F[k]"""
         init_or_update(space, self, __args__, 'dict.update')
 
+    def ensure_object_strategy(self):    # for cpyext
+        object_strategy = self.space.fromcache(ObjectDictStrategy)
+        if self.strategy  is not object_strategy:
+            self.strategy.switch_to_object_strategy(self)
+
 
 def _add_indirections():
     dict_methods = "getitem getitem_str setitem setdefault \