From: Yusuke Endoh <redmine@...>
Date: 2010-01-26T19:59:20+09:00
Subject: [ruby-dev:40158] [Bug #2657] rubyspec: The return keyword within define_method goes through the method via a closure FAILED

Bug #2657: rubyspec: The return keyword within define_method goes through the method via a closure FAILED
http://redmine.ruby-lang.org/issues/show/2657

起票者: Yusuke Endoh
ステータス: Open, 優先度: Normal
担当者: Koichi Sasada, カテゴリ: core, Target version: 1.9.x
ruby -v: ruby 1.9.2dev (2010-01-26 trunk 26420) [i686-linux]

ささださん
遠藤です。

rubyspec の以下のエラーを調べてみました。


  9)
  The return keyword within define_method goes through the method via a closure FAILED
  Expected :bad
   to equal :good

  /home/mame/work/ruby/spec/rubyspec/language/return_spec.rb:269:in `block (3 levels) in <top (required)>'
  /home/mame/work/ruby/spec/rubyspec/language/return_spec.rb:4:in `<top (required)>'


○問題

  $ ./ruby -e '
  class C
    define_method(:foo) do |&x|
      x.call
    end
    def outer
      foo { return :good }
      return :bad
    end
  end
  p C.new.outer
  '
  ok
  :bad

というプログラムが、:good でなく :bad を表示してしまいます。ついでに
謎の ok も出ています。

1.8 では :good が出ますし、define_method を def に置き換えると 1.9 でも
:good が出ますので、:good と出るのが正しそうです。


○原因

rb_vm_invoke_proc の中で lambda の中の TAG_RETURN を止めてしまうことが
原因です。

この処理は一見すると、「lambda 中の return が lambda で止まるために
必要」と思ってしまいますが、TAG_RETURN は普通に vm_exec の中で処理され
goto finish_vme; によって止められますので、不要だと思われます。

この処理が不要であることの状況証拠として、

- この処理を消してもテストのエラーは増えない (上記の rubyspec の分減る)

- カバレッジを見ても、上記の rubyspec のテスト以外では実行されていない
  (http://dame.dyndns.org:7001/20100126/ruby/vm.c の rb_vm_invoke_proc
   の中のこのチェックのカバレッジが 1 回だけ)

- この処理にはデバッグ用の printf 文が残っている (2008-06-17 の r17390
  からずっと) のに、普通に lambda { return }.call などしても出会わない

というわけで、当時は必要だったのかもしれませんが、少なくとも現在では
この処理は害でしかないと思います。


○パッチ

問題の処理を消します。

diff --git a/vm.c b/vm.c
index bb19ecb..f03cf31 100644
--- a/vm.c
+++ b/vm.c
@@ -609,21 +609,6 @@ rb_vm_invoke_proc(rb_thread_t *th, rb_proc_t *proc, VALUE self,
     }
 
     if (state) {
-	if (state == TAG_RETURN && proc->is_lambda) {
-	    VALUE err = th->errinfo;
-	    VALUE *escape_dfp = GET_THROWOBJ_CATCH_POINT(err);
-
-	    if (escape_dfp == cfp->dfp) {
-		printf("ok\n");
-		state = 0;
-		th->errinfo = Qnil;
-		th->cfp = cfp;
-		val = GET_THROWOBJ_VAL(err);
-	    }
-	}
-    }
-
-    if (state) {
 	JUMP_TAG(state);
     }
     return val;

-- 
Yusuke ENDOH <mame@tsg.ne.jp>


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