[ruby-dev:39660] [Bug:trunk] Enumerator.new {|y| y << 1 << 2 << 3 }

[Yusuke ENDOH]

遠藤です。

Enumerator.new の中で yielder << 1 << 2 << 3 と書けません。


  # OK
  generator = Enumerator.new do |yielder|
    yielder << 1
    yielder << 2
    yielder << 3
  end
  generator.each {|x| p x } #=> 1, 2, 3


  # NG
  generator = Enumerator.new do |yielder|
    yielder << 1 << 2 << 3
  end
  generator.each {|x| p x } #=> 1


Enumerator::Yielder#<< が {|x| p x } のブロックの戻り値を返すため
変なことになっています。
Yielder#<< は IO 出力のように、列挙したいものを流し込むように使う
ものだと思っています。この理解が正しければ、強烈に違和感のある挙動
だと思います。

今 Yielder#<< は Yielder#yield の alias になっていますが、yield は
元の挙動のまま、Yielder#<< だけ常に self を返すようにしてもいいで
しょうか。


diff --git a/enumerator.c b/enumerator.c
index e341c07..7c50f3d 100644
--- a/enumerator.c
+++ b/enumerator.c
@@ -1012,6 +1012,13 @@ yielder_yield(VALUE obj, VALUE args)
     return rb_proc_call(ptr->proc, args);
 }

+/* :nodoc: */
+static VALUE yielder_yield_push(VALUE obj, VALUE args)
+{
+    yielder_yield(obj, args);
+    return obj;
+}
+
 static VALUE
 yielder_yield_i(VALUE obj, VALUE memo, int argc, VALUE *argv)
 {
@@ -1228,7 +1235,7 @@ Init_Enumerator(void)
     rb_define_alloc_func(rb_cYielder, yielder_allocate);
     rb_define_method(rb_cYielder, "initialize", yielder_initialize, 0);
     rb_define_method(rb_cYielder, "yield", yielder_yield, -2);
-    rb_define_method(rb_cYielder, "<<", yielder_yield, -2);
+    rb_define_method(rb_cYielder, "<<", yielder_yield_push, -2);

     id_rewind = rb_intern("rewind");
     id_each = rb_intern("each");

--
Yusuke ENDOH <mame@...>

[Tanaka Akira-3]

In article <e0b1e5700911110537u2aacf835pc0aea13d89a92cef@...>,
  Yusuke ENDOH <mame@...> writes:

> 今 Yielder#<< は Yielder#yield の alias になっていますが、yield は
> 元の挙動のまま、Yielder#<< だけ常に self を返すようにしてもいいで
> しょうか。

あぁ、それは妥当だと思います。

それはそれとして、ドキュメントは必要でしょうねぇ。
--
[田中 哲][たなか あきら][Tanaka Akira]

[Yusuke ENDOH]

遠藤です。

2009年11月11日22:54 Tanaka Akira <akr@...>:
> In article <e0b1e5700911110537u2aacf835pc0aea13d89a92cef@...>,
>  Yusuke ENDOH <mame@...> writes:
>
>> 今 Yielder#<< は Yielder#yield の alias になっていますが、yield は
>> 元の挙動のまま、Yielder#<< だけ常に self を返すようにしてもいいで
>> しょうか。
>
> あぁ、それは妥当だと思います。

ありがとうございます。それではコミットします。


> それはそれとして、ドキュメントは必要でしょうねぇ。

Enumerator::Yielder と Enumerator::Generator は全部 nodoc だったので、
あわせて適当に書いてみました。これもコミットしようと思います。


書いていて気がついたんですが、Generator#each は self ではなく proc の
戻り値を返すようです。意図的なんでしょうか。

  g = Enumerator::Generator.new {|y| y << 1 << 2 << 3; :foo }
  p g.each {}  #=> :foo

まあ、Generator を直接使うことは普通はないので、問題になることはない
のかもしれませんが。


diff --git a/enumerator.c b/enumerator.c
index e341c07..e8e147a 100644
--- a/enumerator.c
+++ b/enumerator.c
@@ -994,7 +994,19 @@ yielder_init(VALUE obj, VALUE proc)
     return obj;
 }

-/* :nodoc: */
+/*
+ *  call-seq:
+ *     Enumerator::Yielder.new {|x| block }  => new_yielder
+ *
+ *  Returns a new yielder encapsulating a given <i>proc</i>.
+ *  <i>proc</i> is called with arguments that the yielder receives by
+ *  <code>Enumerator::Yielder#yield</code> or <<.
+ *
+ *     y = Enumerator::Yielder.new {|x| p x }
+ *     y << 1  #=> 1
+ *     y << 2  #=> 2
+ *     y << 3  #=> 3
+ */
 static VALUE
 yielder_initialize(VALUE obj)
 {
@@ -1003,7 +1015,18 @@ yielder_initialize(VALUE obj)
     return yielder_init(obj, rb_block_proc());
 }

-/* :nodoc: */
+/*
+ *  call-seq:
+ *     yielder.yield(val)  =>  value that proc returns
+ *
+ *  Invokes the encapsulated <i>proc</i> and passes val as an argument.
+ *  Returns value that the <i>proc</i> returns.
+ *
+ *     y = Enumerator::Yielder.new {|x| x * 10 }
+ *     p y.yield(1)  #=> 10
+ *     p y.yield(2)  #=> 20
+ *     p y.yield(3)  #=> 30
+ */
 static VALUE
 yielder_yield(VALUE obj, VALUE args)
 {
@@ -1012,6 +1035,22 @@ yielder_yield(VALUE obj, VALUE args)
     return rb_proc_call(ptr->proc, args);
 }

+/*
+ *  call-seq:
+ *     yielder << val  =>  self
+ *
+ *  Invokes the encapsulated <i>proc</i> and passes val as an argument.
+ *  Returns self.
+ *
+ *     y = Enumerator::Yielder.new {|x| p x }
+ *     y << 1 << 2 << 3  #=> 1, 2, 3
+ */
+static VALUE yielder_yield_push(VALUE obj, VALUE args)
+{
+    yielder_yield(obj, args);
+    return obj;
+}
+
 static VALUE
 yielder_yield_i(VALUE obj, VALUE memo, int argc, VALUE *argv)
 {
@@ -1092,7 +1131,18 @@ generator_init(VALUE obj, VALUE proc)

 VALUE rb_obj_is_proc(VALUE proc);

-/* :nodoc: */
+/*
+ *  call-seq:
+ *     Enumerator::Generator.new {|y| block }  => new_generator
+ *     Enumerator::Generator.new(proc)         => new_generator
+ *
+ *  Returns a new generator encapsulating a given <i>proc</i>.
+ *  <i>proc</i> receives an instance of Enumerator::Yielder.  The generator
+ *  enumerates a sequence of values that the yielder receives by
+ *  <code>Enumerator::Yielder#yield</code> or <<.
+ *
+ *     Enumerator::Generator.new {|y| y << 1 << 2 << 3 }.to_a  #=> [1, 2, 3]
+ */
 static VALUE
 generator_initialize(int argc, VALUE *argv, VALUE obj)
 {
@@ -1137,7 +1187,14 @@ generator_init_copy(VALUE obj, VALUE orig)
     return obj;
 }

-/* :nodoc: */
+/*  call-seq:
+ *     generator.each {|x| block }  =>  value that <i>proc</i> returns
+ *
+ *  Enumerates the sequence of the generator.
+ *
+ *     g = Enumerator::Generator.new {|y| y << 1 << 2 << 3 }
+ *     g.each {|x| p x }  #=> 1, 2, 3
+ */
 static VALUE
 generator_each(VALUE obj)
 {
@@ -1228,7 +1285,7 @@ Init_Enumerator(void)
     rb_define_alloc_func(rb_cYielder, yielder_allocate);
     rb_define_method(rb_cYielder, "initialize", yielder_initialize, 0);
     rb_define_method(rb_cYielder, "yield", yielder_yield, -2);
-    rb_define_method(rb_cYielder, "<<", yielder_yield, -2);
+    rb_define_method(rb_cYielder, "<<", yielder_yield_push, -2);

     id_rewind = rb_intern("rewind");
     id_each = rb_intern("each");

--
Yusuke ENDOH <mame@...>

[Yukihiro Matsumoto]

まつもと ゆきひろです

In message "Re: [ruby-dev:39663] Re: [Bug:trunk] Enumerator.new {|y| y << 1 << 2 << 3 }"
    on Wed, 11 Nov 2009 23:50:47 +0900, Yusuke ENDOH <mame@...> writes:

|書いていて気がついたんですが、Generator#each は self ではなく proc の
|戻り値を返すようです。意図的なんでしょうか。
|
|  g = Enumerator::Generator.new {|y| y << 1 << 2 << 3; :foo }
|  p g.each {}  #=> :foo
|
|まあ、Generator を直接使うことは普通はないので、問題になることはない
|のかもしれませんが。

意図的ではないと思います。

[Akinori MUSHA]

At Wed, 11 Nov 2009 23:50:47 +0900,
Yusuke ENDOH wrote:

> 2009年11月11日22:54 Tanaka Akira <akr@...>:
> > In article <e0b1e5700911110537u2aacf835pc0aea13d89a92cef@...>,
> >  Yusuke ENDOH <mame@...> writes:
> >
> >> 今 Yielder#<< は Yielder#yield の alias になっていますが、yield は
> >> 元の挙動のまま、Yielder#<< だけ常に self を返すようにしてもいいで
> >> しょうか。
> >
> > あぁ、それは妥当だと思います。
>
> ありがとうございます。それではコミットします。

　self を返すようにしていたはずだと思ったら、 r24587 で変更されて
いたんですね。ブロックの値を取れなかったわけか。#yield はその方が
いいですね。一方 #<< は self を返す方が自然と思うので賛成です。

> > それはそれとして、ドキュメントは必要でしょうねぇ。
>
> Enumerator::Yielder と Enumerator::Generator は全部 nodoc だったので、
> あわせて適当に書いてみました。これもコミットしようと思います。

　Yielder/Generator という組合せによる実装は実験的なもので、特に
後者は外部に公開する必要がないので敢えて伏せていました。公開仕様に
すると制約になるので、需要が生じるまでは非公開の方がいいのではない
でしょうか。

> 書いていて気がついたんですが、Generator#each は self ではなく proc の
> 戻り値を返すようです。意図的なんでしょうか。
>
>   g = Enumerator::Generator.new {|y| y << 1 << 2 << 3; :foo }
>   p g.each {}  #=> :foo
>
> まあ、Generator を直接使うことは普通はないので、問題になることはない
> のかもしれませんが。

　私が実装したときは self を返していましたが、こちらも r24587 で
変更されているので、田中さんには何らかの意図があるのだと思います。

--
Akinori MUSHA / http://akinori.org/

[Yusuke ENDOH]

遠藤です。

2009年11月12日0:38 Akinori MUSHA <knu@...>:
>> > それはそれとして、ドキュメントは必要でしょうねぇ。
>>
>> Enumerator::Yielder と Enumerator::Generator は全部 nodoc だったので、
>> あわせて適当に書いてみました。これもコミットしようと思います。
>
> 　Yielder/Generator という組合せによる実装は実験的なもので、特に
> 後者は外部に公開する必要がないので敢えて伏せていました。公開仕様に
> すると制約になるので、需要が生じるまでは非公開の方がいいのではない
> でしょうか。

ああ、やっぱりそういう意図なんですよね。そういうことならドキュメントは
コミットしないでおきます。

ただ、Enumerator.new にちょろっと説明があるとはいえ、Yielder のインス
タンスはユーザに丸見えなので、Yielder#yield と << まで nodoc というのは
あまりよくないかなとは思いました。

>> 書いていて気がついたんですが、Generator#each は self ではなく proc の
>> 戻り値を返すようです。意図的なんでしょうか。
>>
>>   g = Enumerator::Generator.new {|y| y << 1 << 2 << 3; :foo }
>>   p g.each {}  #=> :foo
>>
>> まあ、Generator を直接使うことは普通はないので、問題になることはない
>> のかもしれませんが。
>
> 　私が実装したときは self を返していましたが、こちらも r24587 で
> 変更されているので、田中さんには何らかの意図があるのだと思います。

なるほど。
意図的ということなら、せっかく得られる情報をわざと隠すこともない、と
いうことかなと思いました。
私にこだわりはないので、触らないでおきます。

--
Yusuke ENDOH <mame@...>

[Tanaka Akira-3]

In article <e0b1e5700911110650w2f414177t1b66be799313328b@...>,
  Yusuke ENDOH <mame@...> writes:

> 書いていて気がついたんですが、Generator#each は self ではなく proc の
> 戻り値を返すようです。意図的なんでしょうか。
>
>   g = Enumerator::Generator.new {|y| y << 1 << 2 << 3; :foo }
>   p g.each {}  #=> :foo

ちょっとはっきりした記憶がないのですが、

  e = Enumerator.new {|y| ... }
  e.each { ... }

という形式でも、each の返り値を Enumerator 側で決められるよ
うにするためだったような気がします。

[ruby-dev:39109] からの話は、イテレータメソッドの挙動を柔軟
に制御するという目的でしたからら。
--
[田中 哲][たなか あきら][Tanaka Akira]

[Vlad Why]

チケット #2356 が更新されました。 (by Yusuke Endoh)

ステータス OpenからClosedに変更
進捗 % 0から100に変更

This issue was solved with changeset r25721.
Yusuke, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

----------------------------------------
http://redmine.ruby-lang.org/issues/show/2356

----------------------------------------
http://redmine.ruby-lang.org

[Akinori MUSHA]

At Thu, 12 Nov 2009 01:23:56 +0900,
Yusuke ENDOH wrote:

> > 　Yielder/Generator という組合せによる実装は実験的なもので、特に
> > 後者は外部に公開する必要がないので敢えて伏せていました。公開仕様に
> > すると制約になるので、需要が生じるまでは非公開の方がいいのではない
> > でしょうか。
>
> ああ、やっぱりそういう意図なんですよね。そういうことならドキュメントは
> コミットしないでおきます。
>
> ただ、Enumerator.new にちょろっと説明があるとはいえ、Yielder のインス
> タンスはユーザに丸見えなので、Yielder#yield と << まで nodoc というのは
> あまりよくないかなとは思いました。

　こういうときは

class Enumerator
  # ブロックを渡すと(略)インスタンスが生成されて、
  # Enumerator::Yieldable なオブジェクトが渡されて呼ばれるよ
  def initialize
  end

  # レキシカルなコンテクスト外のブロックにyieldするためのモジュール
  # このモジュールは #yield を前提にしているよ
  module Yieldable
    # 渡されたvalueをyieldするよ
    # このモジュールをincludeするクラスが実装するよ
    def yield(value)
      raise NotImplementedError
    end

    # yieldしてselfを返すよ
    def <<(value)
      self.yield(value)
      self
    end
  end

  # :nodoc:
  class Yielder
    def yield(value)
      # ..
    end

    include Yieldable
  end
end

のような構成にすると、クラス名の明文化を避けつつドキュメントする
ことができて、将来の変更に対する制約を回避できるかもしれませんね。

　つまり、ユーザが y.instance_of?(Yielder) などとしだすともう特異
オブジェクトを返すように変えることはAPI変更になってしまいますが、
y.is_a?(Yieldable) と y.respond_to?(:yield) (および :<<)しか仮定
させないことで、 extend Yieldable した特異オブジェクトを返しても
互換性の問題なしというわけです。

--
Akinori MUSHA / http://akinori.org/

[NARUSE, Yui-2]

Yusuke ENDOH wrote:

> 遠藤です。
>
> 2009年11月12日0:38 Akinori MUSHA <knu@...>:
>>>> それはそれとして、ドキュメントは必要でしょうねぇ。
>>> Enumerator::Yielder と Enumerator::Generator は全部 nodoc だったので、
>>> あわせて適当に書いてみました。これもコミットしようと思います。
>> 　Yielder/Generator という組合せによる実装は実験的なもので、特に
>> 後者は外部に公開する必要がないので敢えて伏せていました。公開仕様に
>> すると制約になるので、需要が生じるまでは非公開の方がいいのではない
>> でしょうか。
>
> ああ、やっぱりそういう意図なんですよね。そういうことならドキュメントは
> コミットしないでおきます。

experimenal ならばその旨のドキュメントは入れておいた方がいいのではないでしょうか。

--
NARUSE, Yui  <naruse@...>

[Yusuke ENDOH]

遠藤です。

2009年11月12日21:11 Akinori MUSHA <knu@...>:
> つまり、ユーザが y.instance_of?(Yielder) などとしだすともう特異
> オブジェクトを返すように変えることはAPI変更になってしまいますが、
> y.is_a?(Yieldable) と y.respond_to?(:yield) (および :<<)しか仮定
> させないことで、 extend Yieldable した特異オブジェクトを返しても
> 互換性の問題なしというわけです。

なるほど。いいと思います。
Yielder を匿名クラスにすれば完璧だと思います。

--
Yusuke ENDOH <mame@...>

[Yusuke ENDOH]

遠藤です。

2009年11月12日21:17 NARUSE, Yui <naruse@...>:

>> 2009年11月12日0:38 Akinori MUSHA <knu@...>:
>>>>> それはそれとして、ドキュメントは必要でしょうねぇ。
>>>> Enumerator::Yielder と Enumerator::Generator は全部 nodoc だったので、
>>>> あわせて適当に書いてみました。これもコミットしようと思います。
>>> Yielder/Generator という組合せによる実装は実験的なもので、特に
>>> 後者は外部に公開する必要がないので敢えて伏せていました。公開仕様に
>>> すると制約になるので、需要が生じるまでは非公開の方がいいのではない
>>> でしょうか。
>>
>> ああ、やっぱりそういう意図なんですよね。そういうことならドキュメントは
>> コミットしないでおきます。
>
> experimenal ならばその旨のドキュメントは入れておいた方がいいのではないでしょうか。

「何やら yield メソッドと << メソッドが定義されたオブジェクトが渡される」
という仕様は fix されているけれど、それが Yielder のインスタンスであると
いうことは (現在の実装がそうなってるだけで) 保証していない、という意味だ
と思います。あってますよね？ > knu さん

そういう場合どこに doc を書くべきかなんですが、[ruby-dev:39672] のように
すればよさそうです。

--
Yusuke ENDOH <mame@...>

[Akinori MUSHA]

At Thu, 12 Nov 2009 21:33:23 +0900,
Yusuke ENDOH wrote:

>
> 遠藤です。
>
> 2009年11月12日21:17 NARUSE, Yui <naruse@...>:
> >> 2009年11月12日0:38 Akinori MUSHA <knu@...>:
> >>>>> それはそれとして、ドキュメントは必要でしょうねぇ。
> >>>> Enumerator::Yielder と Enumerator::Generator は全部 nodoc だったので、
> >>>> あわせて適当に書いてみました。これもコミットしようと思います。
> >>> Yielder/Generator という組合せによる実装は実験的なもので、特に
> >>> 後者は外部に公開する必要がないので敢えて伏せていました。公開仕様に
> >>> すると制約になるので、需要が生じるまでは非公開の方がいいのではない
> >>> でしょうか。
> >>
> >> ああ、やっぱりそういう意図なんですよね。そういうことならドキュメントは
> >> コミットしないでおきます。
> >
> > experimenal ならばその旨のドキュメントは入れておいた方がいいのではないでしょうか。
>
> 「何やら yield メソッドと << メソッドが定義されたオブジェクトが渡される」
> という仕様は fix されているけれど、それが Yielder のインスタンスであると
> いうことは (現在の実装がそうなってるだけで) 保証していない、という意味だ
> と思います。あってますよね？ > knu さん

　はい、その通りです。

> そういう場合どこに doc を書くべきかなんですが、[ruby-dev:39672] のように
> すればよさそうです。

　遠藤さんは匿名クラス化を提案されていますが、 :nodoc: あるいは
「このクラスは実装都合だから依存禁止」と書くのでは甘いですかね。
SEGV等の危険ではなく互換性の仮定を避けるための匿名化というのは、
プログラマを信用しないという点でRuby的ではなく、もし慣習化すると
ライブラリを書く上で面倒な制約になってしまうのではないかと危惧
します。


P.S.
Redmineが私の投稿を拾ってくれないのはPGP署名のせいかな？
署名なしで送ってみます。

--
Akinori MUSHA / http://akinori.org/

[Yusuke ENDOH]

遠藤です。

2009年11月12日21:55 Akinori MUSHA <knu@...>:
>> そういう場合どこに doc を書くべきかなんですが、[ruby-dev:39672] のように
>> すればよさそうです。
>
> 　遠藤さんは匿名クラス化を提案されていますが、 :nodoc: あるいは
> 「このクラスは実装都合だから依存禁止」と書くのでは甘いですかね。
> SEGV等の危険ではなく互換性の仮定を避けるための匿名化というのは、
> プログラマを信用しないという点でRuby的ではなく、もし慣習化すると
> ライブラリを書く上で面倒な制約になってしまうのではないかと危惧
> します。

ユーザが普通に呼び出すメソッド (ここでは #yield や #<<) が :nodoc: に
なってしまうのが問題だったので、Yieldable が公開 API になるならそれで
いいと思います。

いいと思うんですが、まあコアの中くらいはむやみに内部が見えないように
しとく方がいいんじゃないかな、という気持ちもないことはないです。
RubyVM とか怪しいですけどね。

--
Yusuke ENDOH <mame@...>