https://redmine.ruby-lang.org/https://redmine.ruby-lang.org/favicon.ico?17113305112013-01-16T17:39:40ZRuby Issue Tracking SystemRuby master - Bug #7696: Lazy enumerators with state can't be rewoundhttps://redmine.ruby-lang.org/issues/7696?journal_id=354442013-01-16T17:39:40Zshugo (Shugo Maeda)
<ul></ul><p>marcandre (Marc-Andre Lafortune) wrote:</p>
<blockquote>
<p>The 4 lazy enumerators requiring internal state, i.e. {take|drop}{_while}, don't work as expected after a couple <code>next</code> and a call to <code>rewind</code>.</p>
<p>For example:</p>
<pre><code>e=(1..42).lazy.take(2)
e.next # => 1
e.next # => 2
e.rewind
e.next # => 1
e.next # => StopIteration: iteration reached an end, expected 2
</code></pre>
<p>This is related to <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: 3 bugs with Lazy enumerators with state (Closed)" href="https://redmine.ruby-lang.org/issues/7691">#7691</a>; the current API does not give an easy way to handle state.</p>
</blockquote>
<p><a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: 3 bugs with Lazy enumerators with state (Closed)" href="https://redmine.ruby-lang.org/issues/7691">#7691</a> is the basically same issue as <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: Enumerable::Lazy#zip doesn't rewind internal enumerators (Closed)" href="https://redmine.ruby-lang.org/issues/6142">#6142</a>.</p>
<p>Do you have a draft API spec in mind?<br>
It might be too late to introduce a new API for 2.0.0, though.</p> Ruby master - Bug #7696: Lazy enumerators with state can't be rewoundhttps://redmine.ruby-lang.org/issues/7696?journal_id=354492013-01-17T02:01:10Zmarcandre (Marc-Andre Lafortune)marcandre-ruby-core@marc-andre.ca
<ul></ul><p>I had not see <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: Enumerable::Lazy#zip doesn't rewind internal enumerators (Closed)" href="https://redmine.ruby-lang.org/issues/6142">#6142</a>. Yes, it's the same.</p>
<ol>
<li>
<p>Add attr_accessor to Yielder and document the fact that the yielder is unique to a single enumeration run.</p>
<p>def drop(n)<br>
n = Backports::coerce_to_int(n)<br>
Lazy.new(self) do |yielder, *values|<br>
yielder.memo ||= n<br>
if yielder.memo > 0<br>
yielder.memo -= 1<br>
else<br>
yielder.yield(*values)<br>
end<br>
end<br>
end</p>
</li>
<li>
<p>Optional named argument <code>prepare</code> for Lazy.new to be called before <code>each</code>:</p>
<p>def drop(n)<br>
remain = nil<br>
Lazy.new(self, prepare: ->{remain = n}) do |yielder, *values|<br>
if remain > 0<br>
remain -= 1<br>
else<br>
yielder.yield(*values)<br>
end<br>
end<br>
end</p>
</li>
<li>
<p>Add a method 'prepare' method to Enumerator::Lazy and encourage subclassing. <code>prepare</code> would be called before <code>each</code></p>
<p>class LazyDropper < Enumerator::Lazy<br>
def initialize(obj, n)<br>
@n = n<br>
super do |yielder, *values|<br>
if @remain > 0<br>
@remain -= 1<br>
else<br>
yielder.yield(*values)<br>
end<br>
end</p>
<pre><code>def prepare
@remain = @n
end
</code></pre>
<p>end</p>
</li>
</ol>
<p>MRI should use the same kind of mechanism for the enums requiring state</p> Ruby master - Bug #7696: Lazy enumerators with state can't be rewoundhttps://redmine.ruby-lang.org/issues/7696?journal_id=354502013-01-17T02:02:47Zmarcandre (Marc-Andre Lafortune)marcandre-ruby-core@marc-andre.ca
<ul></ul><p>Oh oh.</p>
<p>Let me add a third example to problems of handling states:</p>
<pre><code>e = (1..6).lazy.drop(3)
e.flat_map{e}.force # => [*(1..6)]*3, should be [*(1..3)]*3
</code></pre>
<p>I believe only the first solution would work for this.</p> Ruby master - Bug #7696: Lazy enumerators with state can't be rewoundhttps://redmine.ruby-lang.org/issues/7696?journal_id=354512013-01-17T03:11:56Zmarcandre (Marc-Andre Lafortune)marcandre-ruby-core@marc-andre.ca
<ul></ul><p>Here's a patch for <code>take</code>. Does it look ok?</p>
<p>diff --git a/enumerator.c b/enumerator.c<br>
index b65712f..1522a3f 100644<br>
--- a/enumerator.c<br>
+++ b/enumerator.c<br>
@@ -105,7 +105,7 @@<br>
VALUE rb_cEnumerator;<br>
VALUE rb_cLazy;<br>
static ID id_rewind, id_each, id_new, id_initialize, id_yield, id_call, id_size;<br>
-static ID id_eqq, id_next, id_result, id_lazy, id_receiver, id_arguments, id_method, id_force;<br>
+static ID id_eqq, id_next, id_result, id_lazy, id_receiver, id_arguments, id_memo, id_method, id_force;<br>
static VALUE sym_each, sym_cycle;</p>
<p>VALUE rb_eStopIteration;<br>
@@ -1641,14 +1641,18 @@ lazy_zip(int argc, VALUE *argv, VALUE obj)<br>
static VALUE<br>
lazy_take_func(VALUE val, VALUE args, int argc, VALUE *argv)<br>
{</p>
<ul>
<li>NODE *memo = RNODE(args);</li>
</ul>
<ul>
<li>
<p>long remain;</p>
</li>
<li>
<p>VALUE memo = rb_ivar_get(argv[0], id_memo);</p>
</li>
<li>
<p>if (NIL_P(memo)) {</p>
</li>
<li>
<pre><code> memo = args;
</code></pre>
</li>
<li>
<p>}</p>
<p>rb_funcall2(argv[0], id_yield, argc - 1, argv + 1);</p>
</li>
</ul>
<ul>
<li>if (--memo->u3.cnt == 0) {</li>
<li>
<pre><code> memo->u3.cnt = memo->u2.argc;
</code></pre>
</li>
</ul>
<ul>
<li>if ((remain = NUM2LONG(memo)-1) == 0) {<br>
return Qundef;<br>
}<br>
else {</li>
<li>
<pre><code> rb_ivar_set(argv[0], id_memo, LONG2NUM(remain));
return Qnil;
</code></pre>
}<br>
}<br>
@@ -1666,7 +1670,6 @@ lazy_take_size(VALUE lazy)<br>
static VALUE<br>
lazy_take(VALUE obj, VALUE n)<br>
{</li>
</ul>
<ul>
<li>NODE *memo;<br>
long len = NUM2LONG(n);<br>
int argc = 1;<br>
VALUE argv[3];<br>
@@ -1680,9 +1683,8 @@ lazy_take(VALUE obj, VALUE n)<br>
argv[2] = INT2NUM(0);<br>
argc = 3;<br>
}</li>
<li>memo = NEW_MEMO(0, len, len);<br>
return lazy_set_method(rb_block_call(rb_cLazy, id_new, argc, argv,</li>
<li>
<pre><code> lazy_take_func, (VALUE) memo),
</code></pre>
</li>
</ul>
<ul>
<li>
<pre><code> lazy_take_func, n),
rb_ary_new3(1, n), lazy_take_size);
</code></pre>
</li>
</ul>
<p>}</p>
<p>@@ -1955,6 +1957,7 @@ Init_Enumerator(void)<br>
id_eqq = rb_intern("===");<br>
id_receiver = rb_intern("receiver");<br>
id_arguments = rb_intern("arguments");</p>
<ul>
<li>
<p>id_memo = rb_intern("memo");<br>
id_method = rb_intern("method");<br>
id_force = rb_intern("force");<br>
sym_each = ID2SYM(id_each);<br>
diff --git a/test/ruby/test_lazy_enumerator.rb b/test/ruby/test_lazy_enumerator.rb<br>
index acd4843..35e92c9 100644<br>
--- a/test/ruby/test_lazy_enumerator.rb<br>
+++ b/test/ruby/test_lazy_enumerator.rb<br>
@@ -243,6 +243,23 @@ class TestLazyEnumerator < Test::Unit::TestCase<br>
assert_equal((1..5).to_a, take5.force, bug6428)<br>
end</p>
</li>
<li>
<p>def test_take_nested</p>
</li>
<li>
<p>bug7696 = '<a href="/issues/7696">[ruby-core:51470]</a>'</p>
</li>
<li>
<p>a = Step.new(1..10)</p>
</li>
<li>
<p>take5 = a.lazy.take(5)</p>
</li>
<li>
<p>assert_equal([*(1..5)]*5, take5.flat_map{take5}.force, bug7696)</p>
</li>
<li>
<p>end</p>
</li>
<li>
<li>
<p>def test_take_rewound</p>
</li>
<li>
<p>bug7696 = '<a href="/issues/7696">[ruby-core:51470]</a>'</p>
</li>
<li>
<p>e=(1..42).lazy.take(2)</p>
</li>
<li>
<p>assert_equal 1, e.next</p>
</li>
<li>
<p>assert_equal 2, e.next</p>
</li>
<li>
<p>e.rewind</p>
</li>
<li>
<p>assert_equal 1, e.next</p>
</li>
<li>
<p>assert_equal 2, e.next</p>
</li>
<li>
<p>end</p>
</li>
<li>
<p>def test_take_while<br>
a = Step.new(1..10)<br>
assert_equal(1, a.take_while {|i| i < 5}.first)</p>
</li>
</ul> Ruby master - Bug #7696: Lazy enumerators with state can't be rewoundhttps://redmine.ruby-lang.org/issues/7696?journal_id=354812013-01-19T10:33:18Zshugo (Shugo Maeda)
<ul></ul><p>marcandre (Marc-Andre Lafortune) wrote:</p>
<blockquote>
<p>Here's a patch for <code>take</code>. Does it look ok?</p>
</blockquote>
<p>Does it work well for zip?<br>
I wonder how arguments are rewound.</p> Ruby master - Bug #7696: Lazy enumerators with state can't be rewoundhttps://redmine.ruby-lang.org/issues/7696?journal_id=354882013-01-20T02:33:15Zmarcandre (Marc-Andre Lafortune)marcandre-ruby-core@marc-andre.ca
<ul></ul><p>The same idea will work for zip too; the arguments must be converted to enumerators only when yielder.memo is not set, i.e. every new yielder.</p>
<p>The arguments are never really rewound, but the first yielder holding them will not be reused after enum.rewind; a new yielder is given.<br>
I haven't checked if that's the current behavior in other implementations, but if yielder is to be the state holder, that's the way it should work.</p>
<pre><code>enum = (1..2).lazy.zip(1..2)
enum.next # => yielder.memo was nil, so (1..2).each is called and stored in the memo
enum.rewind # => the original yielder is discarded
enum.next # => the second yielder.memo is nil, so (1..2).each is called again and stored in the memo
</code></pre>
<p>I'm using the same idea in my <code>backports</code> gem to implement this in pure Ruby:</p>
<pre><code>https://github.com/marcandre/backports/blob/master/lib/backports/2.0.0/enumerator/lazy.rb
</code></pre>
<p>The only other possibility that would work is to pass yield extra 'state' argument when required, like:</p>
<p>def drop(n)<br>
Lazy.new(self, state: ->{{remain: n}}) do |yielder, state, *values|<br>
if state[:remain] > 0<br>
state[:remain] -= 1<br>
else<br>
yielder.yield(*values)<br>
end<br>
end<br>
end</p>
<p>Maybe the :state option could be an object instead of a lmabda, which would be dupped before each enumerations:</p>
<p>def drop(n)<br>
Lazy.new(self, state: {remain: n}) do |yielder, state, *values|<br>
if state[:remain] > 0<br>
state[:remain] -= 1<br>
else<br>
yielder.yield(*values)<br>
end<br>
end<br>
end</p>
<p>Both solutions seem complex to me, but would definitely put the correct emphasis on how to deal with state.</p>
<p>So, does the patch look acceptable as far as MRI is concerned?<br>
For the public api, should there be a public Yielder#memo and a guarantee that there is exactly one yielder object per iteration? or instead an extra state yielded when required?</p> Ruby master - Bug #7696: Lazy enumerators with state can't be rewoundhttps://redmine.ruby-lang.org/issues/7696?journal_id=355112013-01-21T18:14:23Zshugo (Shugo Maeda)
<ul></ul><p>marcandre (Marc-Andre Lafortune) wrote:</p>
<blockquote>
<p>The same idea will work for zip too; the arguments must be converted to enumerators only when yielder.memo is not set, i.e. every new yielder.</p>
<p>The arguments are never really rewound, but the first yielder holding them will not be reused after enum.rewind; a new yielder is given.<br>
I haven't checked if that's the current behavior in other implementations, but if yielder is to be the state holder, that's the way it should work.</p>
</blockquote>
<p>So, the following behavior is intended, right?</p>
<p>$ ruby1.9.3 -I ~/src/backports/lib -r backports -r backports/2.0.0/enumerable -e "a = (1..3).lazy.zip(('a'..'z').each); p a.to_a; p a.to_a"<br>
[[1, "a"], [2, "b"], [3, "c"]]<br>
[[1, "d"], [2, "e"], [3, "f"]]</p>
<blockquote>
<p>So, does the patch look acceptable as far as MRI is concerned?</p>
</blockquote>
<p>If the above behavior is intended, the patch looks acceptable.</p>
<blockquote>
<p>For the public api, should there be a public Yielder#memo and a guarantee that there is exactly one yielder object per iteration? or instead an extra state yielded when required?</p>
</blockquote>
<p>It might be too late to introduce a new API for 2.0.0, so how about to move it to next minor?</p> Ruby master - Bug #7696: Lazy enumerators with state can't be rewoundhttps://redmine.ruby-lang.org/issues/7696?journal_id=355162013-01-22T04:44:13Zmarcandre (Marc-Andre Lafortune)marcandre-ruby-core@marc-andre.ca
<ul></ul><p>shugo (Shugo Maeda) wrote:</p>
<blockquote>
<p>So, the following behavior is intended, right?</p>
<p>$ ruby1.9.3 -I ~/src/backports/lib -r backports -r backports/2.0.0/enumerable -e "a = (1..3).lazy.zip(('a'..'z').each); p a.to_a; p a.to_a"<br>
[[1, "a"], [2, "b"], [3, "c"]]<br>
[[1, "d"], [2, "e"], [3, "f"]]</p>
</blockquote>
<p>That's a very good question.</p>
<p>It probably would be best to call <code>to_enum</code> instead of <code>each</code>. Calling <code>next|rewind</code> on an enumerator should really only affect other calls to <code>next</code>. With <code>to_enum</code>, we'll get the same result every time.</p>
<p>Similarly, we expect <code>(1..3).zip(('a'..'z').each.tap(&:next))</code> to return <code>[[1, 'a'], ...</code>, and not <code>[[1, 'b'], ...</code> even though <code>next</code> was called on the given enumerator.</p>
<blockquote>
<p>If the above behavior is intended, the patch looks acceptable.</p>
</blockquote>
<p>Thanks for reviewing it. I'll commit it, changing the call to <code>each</code> to <code>to_enum</code> (unless there are objections). I'll use the same technique to fix the other 3 lazy enumerators with state.</p>
<blockquote>
<blockquote>
<p>For the public api, should there be a public Yielder#memo and a guarantee that there is exactly one yielder object per iteration? or instead an extra state yielded when required?</p>
</blockquote>
<p>It might be too late to introduce a new API for 2.0.0, so how about to move it to next minor?</p>
</blockquote>
<p>I understand. On the other hand, the API for Lazy.new was never decided upon and really should be finalized before 2.0.0!</p>
<p>If we opt for the Yielder#memo way (and agree on the name), maybe we can convince mame to accept such a trivial change. In that case, the biggest "change" is the explicit guarantee of a different yielder per iteration. It's already the case (also for JRuby and rubinius), but AFAIK it's never been official.</p>
<p>With the modified yielding way, it would be nice to include it in Lazy.new's api in this version, especially since it is still being finalized.</p>
<p>At the very least, a note in the documentation about handling state would be needed for 2.0.0.</p> Ruby master - Bug #7696: Lazy enumerators with state can't be rewoundhttps://redmine.ruby-lang.org/issues/7696?journal_id=355192013-01-22T10:24:09Zshugo (Shugo Maeda)
<ul><li><strong>Assignee</strong> set to <i>matz (Yukihiro Matsumoto)</i></li></ul><p>marcandre (Marc-Andre Lafortune) wrote:</p>
<blockquote>
<p>shugo (Shugo Maeda) wrote:</p>
<blockquote>
<p>So, the following behavior is intended, right?</p>
<p>$ ruby1.9.3 -I ~/src/backports/lib -r backports -r backports/2.0.0/enumerable -e "a = (1..3).lazy.zip(('a'..'z').each); p a.to_a; p a.to_a"<br>
[[1, "a"], [2, "b"], [3, "c"]]<br>
[[1, "d"], [2, "e"], [3, "f"]]</p>
</blockquote>
<p>That's a very good question.</p>
<p>It probably would be best to call <code>to_enum</code> instead of <code>each</code>. Calling <code>next|rewind</code> on an enumerator should really only affect other calls to <code>next</code>. With <code>to_enum</code>, we'll get the same result every time.</p>
</blockquote>
<p>Even if to_enum is called, IO instances never be rewound, but I guess IO instances need not be rewound.</p>
<blockquote>
<blockquote>
<p>It might be too late to introduce a new API for 2.0.0, so how about to move it to next minor?</p>
</blockquote>
<p>I understand. On the other hand, the API for Lazy.new was never decided upon and really should be finalized before 2.0.0!</p>
<p>If we opt for the Yielder#memo way (and agree on the name), maybe we can convince mame to accept such a trivial change. In that case, the biggest "change" is the explicit guarantee of a different yielder per iteration. It's already the case (also for JRuby and rubinius), but AFAIK it's never been official.</p>
</blockquote>
<p>`The explicit guarantee of a different yielder per iteration' sounds acceptable for me, but I'm not sure whether Yielder#memo is a good name.</p>
<blockquote>
<p>With the modified yielding way, it would be nice to include it in Lazy.new's api in this version, especially since it is still being finalized.</p>
<p>At the very least, a note in the documentation about handling state would be needed for 2.0.0.</p>
</blockquote>
<p>I believe Matz should decide it.</p> Ruby master - Bug #7696: Lazy enumerators with state can't be rewoundhttps://redmine.ruby-lang.org/issues/7696?journal_id=355202013-01-22T10:24:17Zshugo (Shugo Maeda)
<ul><li><strong>Status</strong> changed from <i>Open</i> to <i>Assigned</i></li></ul> Ruby master - Bug #7696: Lazy enumerators with state can't be rewoundhttps://redmine.ruby-lang.org/issues/7696?journal_id=355702013-01-24T15:22:36Zmarcandre (Marc-Andre Lafortune)marcandre-ruby-core@marc-andre.ca
<ul><li><strong>Status</strong> changed from <i>Assigned</i> to <i>Closed</i></li><li><strong>% Done</strong> changed from <i>0</i> to <i>100</i></li></ul><p>This issue was solved with changeset r38920.<br>
Marc-Andre, thank you for reporting this issue.<br>
Your contribution to Ruby is greatly appreciated.<br>
May Ruby be with you.</p>
<hr>
<ul>
<li>
<p>enumerator.c: Fix state handling for Lazy#take<br>
[bug <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: Lazy enumerators with state can't be rewound (Closed)" href="https://redmine.ruby-lang.org/issues/7696">#7696</a>]</p>
</li>
<li>
<p>test/ruby/test_lazy_enumerator.rb: test for above</p>
</li>
</ul> Ruby master - Bug #7696: Lazy enumerators with state can't be rewoundhttps://redmine.ruby-lang.org/issues/7696?journal_id=355722013-01-24T15:26:59Zmarcandre (Marc-Andre Lafortune)marcandre-ruby-core@marc-andre.ca
<ul><li><strong>Status</strong> changed from <i>Closed</i> to <i>Open</i></li></ul><p>Bugs in MRI are fixed, but keeping open so Matz can decide how users should handle state.</p> Ruby master - Bug #7696: Lazy enumerators with state can't be rewoundhttps://redmine.ruby-lang.org/issues/7696?journal_id=364092013-02-17T15:50:32Zmame (Yusuke Endoh)mame@ruby-lang.org
<ul><li><strong>Subject</strong> changed from <i>Lazy enumerators with state can't be rewound</i> to <i>Lazy enumerators with state can&#x27;t be rewound</i></li><li><strong>Status</strong> changed from <i>Open</i> to <i>Assigned</i></li><li><strong>Target version</strong> changed from <i>2.0.0</i> to <i>2.6</i></li></ul> Ruby master - Bug #7696: Lazy enumerators with state can't be rewoundhttps://redmine.ruby-lang.org/issues/7696?journal_id=409562013-08-07T12:21:15Znaruse (Yui NARUSE)naruse@airemix.jp
<ul><li><strong>Target version</strong> changed from <i>2.6</i> to <i>2.1.0</i></li></ul> Ruby master - Bug #7696: Lazy enumerators with state can't be rewoundhttps://redmine.ruby-lang.org/issues/7696?journal_id=414432013-08-31T07:57:58Zmarcandre (Marc-Andre Lafortune)marcandre-ruby-core@marc-andre.ca
<ul><li><strong>Status</strong> changed from <i>Assigned</i> to <i>Closed</i></li></ul><p>I created a new feature request <a class="issue tracker-2 status-6 priority-4 priority-default closed" title="Feature: Yielder#state (Rejected)" href="https://redmine.ruby-lang.org/issues/8840">#8840</a>, so I'm closing this.</p>