Merge pull request #1205 from undingen/BoxIteratorGeneric_fix

BoxIteratorGeneric call PyIter_Next() lazily

from_cpython/Modules/_io/textio.c

@@ -305,9 +305,8 @@ _PyIncrementalNewlineDecoder_decode(PyObject *_self,
     if (!final) {
         if (output_len > 0
             && PyUnicode_AS_UNICODE(output)[output_len - 1] == '\r') {
-            // Pyston change:
-            // if (Py_REFCNT(output) == 1) {
-            if (0) {
+
+            if (Py_REFCNT(output) == 1) {
                 if (PyUnicode_Resize(&output, output_len - 1) < 0)
                     goto error;
             }
@@ -406,9 +405,7 @@ _PyIncrementalNewlineDecoder_decode(PyObject *_self,
             PyObject *translated = NULL;
             Py_UNICODE *out_str;
             Py_UNICODE *in, *out, *end;
-            // Pyston change
-            // if (Py_REFCNT(output) != 1) {
-            if (1) {
+            if (Py_REFCNT(output) != 1) {
                 /* We could try to optimize this so that we only do a copy
                    when there is something to translate. On the other hand,
                    most decoders should only output non-shared strings, i.e.
@@ -416,7 +413,7 @@ _PyIncrementalNewlineDecoder_decode(PyObject *_self,
                 translated = PyUnicode_FromUnicode(NULL, len);
                 if (translated == NULL)
                     goto error;
-                // assert(Py_REFCNT(translated) == 1); Pyston change
+                assert(Py_REFCNT(translated) == 1);
                 memcpy(PyUnicode_AS_UNICODE(translated),
                        PyUnicode_AS_UNICODE(output),
                        len * sizeof(Py_UNICODE));
@@ -1801,9 +1798,7 @@ _textiowrapper_readline(textio *self, Py_ssize_t limit)
         /* Our line ends in the current buffer */
         self->decoded_chars_used = endpos - offset_to_buffer;
         if (start > 0 || endpos < PyUnicode_GET_SIZE(line)) {
-            // Pyston change:
-            // if (start == 0 && Py_REFCNT(line) == 1) {
-            if (0) {
+            if (start == 0 && Py_REFCNT(line) == 1) {
                 if (PyUnicode_Resize(&line, endpos) < 0)
                     goto error;
             }

src/runtime/iterators.cpp

@@ -23,22 +23,15 @@ class BoxIteratorGeneric : public BoxIteratorImpl {
 private:
     Box* iterator;
     Box* value;
+    bool need_to_fetch_value;
 
 public:
-    BoxIteratorGeneric(Box* container) : iterator(nullptr), value(nullptr) {
+    BoxIteratorGeneric(Box* container) : iterator(nullptr), value(nullptr), need_to_fetch_value(false) {
         if (container) {
             // TODO: this should probably call getPystonIter
             iterator = getiter(container);
             if (iterator) {
-                // try catch block to manually decref the iterator because if the constructor throwes the destructor
-                // won't get called
-                // but we should probably just change the code to not call next inside the constructor...
-                try {
-                    next();
-                } catch (ExcInfo e) {
-                    Py_CLEAR(iterator);
-                    throw e;
-                }
+                need_to_fetch_value = true;
             } else
                 *this = *end();
         }
@@ -50,21 +43,13 @@ public:
     }
 
     void next() override {
-        STAT_TIMER(t0, "us_timer_iteratorgeneric_next", 0);
-        assert(!value);
-
-        Box* next = PyIter_Next(iterator);
-        if (next) {
-            value = next;
-        } else {
-            if (PyErr_Occurred())
-                throwCAPIException();
-            Py_CLEAR(iterator);
-            *this = *end();
-        }
+        assert(!need_to_fetch_value);
+        need_to_fetch_value = true;
     }
 
     Box* getValue() override {
+        if (need_to_fetch_value)
+            fetchNextValue();
         Box* r = value;
         assert(r);
         value = NULL;
@@ -73,6 +58,9 @@ public:
 
     bool isSame(const BoxIteratorImpl* _rhs) override {
         const BoxIteratorGeneric* rhs = (const BoxIteratorGeneric*)_rhs;
+        assert(!rhs->need_to_fetch_value); // we can't fetch the value here because rhs is const
+        if (need_to_fetch_value)
+            fetchNextValue();
         return iterator == rhs->iterator && value == rhs->value;
     }
 
@@ -87,6 +75,23 @@ public:
         static BoxIteratorGeneric _end(nullptr);
         return &_end;
     }
+
+private:
+    void fetchNextValue() {
+        STAT_TIMER(t0, "us_timer_iteratorgeneric_next", 0);
+        assert(!value);
+        assert(need_to_fetch_value);
+        Box* next = PyIter_Next(iterator);
+        need_to_fetch_value = false;
+        if (next) {
+            value = next;
+        } else {
+            if (PyErr_Occurred())
+                throwCAPIException();
+            Py_CLEAR(iterator);
+            *this = *end();
+        }
+    }
 };
 
 

test/extra/numpy_fulltest.py

@@ -89,7 +89,7 @@ except:
 
 try:
     test_helper.run_test(['sh', '-c', '. %s/bin/activate && python %s/numpy/tools/test-installed-numpy.py' % (ENV_DIR, ENV_DIR)],
-            ENV_NAME, [dict(ran=5781, errors=2, failures=2)])
+            ENV_NAME, [dict(ran=5781, errors=2, failures=1)])
 finally:
     if USE_CUSTOM_PATCHES:
         print_progress_header("Unpatching NumPy...")

test/tests/enumerate_test.py

@@ -4,3 +4,8 @@ print list(enumerate(range(100), sys.maxint-50))
 
 # cycle collection:
 print enumerate(range(100)).next()
+
+it = iter(range(5))
+e = enumerate(it)
+print e.next(), e.next()
+print list(it)