[ruby-dev:46162] Re: [ruby-changes:24978] nobu:r37030 (trunk): insns.def: super from at_exit

[ruby-dev:46162] Re: [ruby-changes:24978] nobu:r37030 (trunk): insns.def: super from at_exit

[SASADA Koichi]

2点ほど．

(1) ChangeLog 間違ってない？　あと2つ修正したファイルが．

(2) この修正でいいんでしょうか．
　　nil check 追加するんじゃなくて，
　　そもそも nil 入ってる状況がまずいといことはない？
　（cfp->klasss の扱いって，まだよくわかってないのですが）


(2012/09/25 10:01), nobu wrote:
> nobu	2012-09-25 09:59:29 +0900 (Tue, 25 Sep 2012)
> 
>   New Revision: 37030
> 
>   http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=rev&revision=37030
> 
>   Log:
>     insns.def: super from at_exit
>     
>     * insns.def (invokesuper): klass in cfp is not valid in at_exit and
>       END blocks.  [ruby-core:47680] [Bug #7064]
> 
>   Modified files:
>     trunk/ChangeLog
>     trunk/insns.def
>     trunk/test/ruby/test_super.rb
>     trunk/vm_insnhelper.c
> 
> Index: ChangeLog
> ===================================================================
> --- ChangeLog	(revision 37029)
> +++ ChangeLog	(revision 37030)
> @@ -1,3 +1,8 @@
> +Tue Sep 25 09:59:26 2012  Nobuyoshi Nakada  <nobu@ruby-lang.org>
> +
> +	* insns.def (invokesuper): klass in cfp is not valid in at_exit and
> +	  END blocks.  [ruby-core:47680] [Bug #7064]
> +
>  Tue Sep 25 08:11:11 2012  NARUSE, Yui  <naruse@ruby-lang.org>
>  
>  	* iseq.c (rb_iseq_defined_string): the index of defined_strings must
> Index: insns.def
> ===================================================================
> --- insns.def	(revision 37029)
> +++ insns.def	(revision 37030)
> @@ -1035,6 +1035,9 @@
>      flag = VM_CALL_SUPER_BIT | VM_CALL_FCALL_BIT;
>  
>      klass = GET_CFP()->klass;
> +    if (NIL_P(klass)) {
> +	vm_super_outside();
> +    }
>      if (!NIL_P(RCLASS_REFINED_CLASS(klass))) {
>  	klass = RCLASS_REFINED_CLASS(klass);
>      }
> Index: vm_insnhelper.c
> ===================================================================
> --- vm_insnhelper.c	(revision 37029)
> +++ vm_insnhelper.c	(revision 37030)
> @@ -1460,6 +1460,12 @@
>  }
>  
>  static void
> +vm_super_outside(void)
> +{
> +    rb_raise(rb_eNoMethodError, "super called outside of method");
> +}
> +
> +static void
>  vm_search_superclass(rb_control_frame_t *reg_cfp, rb_iseq_t *iseq,
>  		     VALUE sigval,
>  		     ID *idp, VALUE *klassp)
> @@ -1472,7 +1478,7 @@
>      }
>  
>      if (iseq == 0) {
> -	rb_raise(rb_eNoMethodError, "super called outside of method");
> +	vm_super_outside();
>      }
>  
>      id = iseq->defined_method_id;
> @@ -1492,8 +1498,7 @@
>  	    while (1) {
>  		lcfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(lcfp);
>  		if (RUBY_VM_CONTROL_FRAME_STACK_OVERFLOW_P(th, lcfp)) {
> -		    rb_raise(rb_eNoMethodError,
> -			     "super called outside of method");
> +		    vm_super_outside();
>  		}
>  		if (lcfp->ep == tep) {
>  		    break;
> @@ -1503,7 +1508,7 @@
>  
>  	/* temporary measure for [Bug #2420] [Bug #3136] */
>  	if (!lcfp->me) {
> -	    rb_raise(rb_eNoMethodError, "super called outside of method");
> +	    vm_super_outside();
>  	}
>  
>  	id = lcfp->me->def->original_id;
> Index: test/ruby/test_super.rb
> ===================================================================
> --- test/ruby/test_super.rb	(revision 37029)
> +++ test/ruby/test_super.rb	(revision 37030)
> @@ -1,4 +1,5 @@
>  require 'test/unit'
> +require_relative 'envutil'
>  
>  class TestSuper < Test::Unit::TestCase
>    class Base
> @@ -347,4 +348,14 @@
>  
>      assert_equal 'hoge', foo.new.bar
>    end
> +
> +  def test_super_in_at_exit
> +    bug7064 = '[ruby-core:47680]'
> +    assert_normal_exit "at_exit {super}", bug7064
> +  end
> +
> +  def test_super_in_END
> +    bug7064 = '[ruby-core:47680]'
> +    assert_normal_exit "END {super}", bug7064
> +  end
>  end
> 


-- 
// SASADA Koichi at atdot dot net

[ruby-dev:46167] Re: [ruby-changes:24978] nobu:r37030 (trunk): insns.def: super from at_exit

[Nobuyoshi Nakada]

なかだです。

At Wed, 26 Sep 2012 01:06:48 +0900,
SASADA Koichi wrote in [ruby-dev:46162]:
> 
> 2点ほど．
> 
> (1) ChangeLog 間違ってない？　あと2つ修正したファイルが．

メインはinsns.defなので省きました。

> (2) この修正でいいんでしょうか．
> 　　nil check 追加するんじゃなくて，
> 　　そもそも nil 入ってる状況がまずいといことはない？
> 　（cfp->klasss の扱いって，まだよくわかってないのですが）

トップレベルのブロック、つまりメソッド外から呼ばれているので
klassがnilなのは正常です。refined classのチェックが入ったのでそ
の前にさらにnilチェックが必要になったわけです。

-- 
--- 僕の前にBugはない。
--- 僕の後ろにBugはできる。
    中田 伸悦

[ruby-dev:46168] Re: [ruby-changes:24978] nobu:r37030 (trunk): insns.def: super from at_exit

[SASADA Koichi]

(2012/09/27 11:51), Nobuyoshi Nakada wrote:
>> > (1) ChangeLog 間違ってない？　あと2つ修正したファイルが．
> メインはinsns.defなので省きました。

　ポリシーを確認しておきたいんですが，メインなファイルだけでいいんでしょ
うか．付随する修正とか，テストとか，とりあえず全部書いているんですが，確
かに意味があまりないといえばないかも．


>> > (2) この修正でいいんでしょうか．
>> > 　　nil check 追加するんじゃなくて，
>> > 　　そもそも nil 入ってる状況がまずいといことはない？
>> > 　（cfp->klasss の扱いって，まだよくわかってないのですが）
> トップレベルのブロック、つまりメソッド外から呼ばれているので
> klassがnilなのは正常です。refined classのチェックが入ったのでそ
> の前にさらにnilチェックが必要になったわけです。

つまりメソッドじゃないと klass は nil なんですね．この辺は自分で一度考え
直さないと...．

-- 
// SASADA Koichi at atdot dot net

[ruby-dev:46171] Re: [ruby-changes:24978] nobu:r37030 (trunk): insns.def: super from at_exit

[Nobuyoshi Nakada]

なかだです。

At Thu, 27 Sep 2012 12:01:07 +0900,
SASADA Koichi wrote in [ruby-dev:46168]:
> (2012/09/27 11:51), Nobuyoshi Nakada wrote:
> >> > (1) ChangeLog 間違ってない？　あと2つ修正したファイルが．
> > メインはinsns.defなので省きました。
> 
> 　ポリシーを確認しておきたいんですが，メインなファイルだけでいいんでしょ
> うか．付随する修正とか，テストとか，とりあえず全部書いているんですが，確
> かに意味があまりないといえばないかも．

あくまで個人的かつ基本的な方針ですが、以下のようなものは省いてます。
* テスト自体の修正以外でないテストの追加
* 些細あるいは自明と思われるもの

-- 
--- 僕の前にBugはない。
--- 僕の後ろにBugはできる。
    中田 伸悦