[ruby-dev:43321] Re: iseq_compile_each()でのマジックナンバ

遠藤です。

2011年3月6日12:17 SASADA Koichi <[e-mail]>:
> (2011/03/06 11:37), きたざわけんいち wrote:
>>> # ちなみにこのコメントは今でも iseq_set_exception_table の中に
>>> # 残っていますが、Dirty Hack 自体は消えています。コメントが追い
>>> # ついてない……。
>>
>> iseq_set_exception_table(rb_iseq_t *iseq)
>> {
>> （略）
>>           /* TODO: Dirty Hack!  Fix me */
>>           if (entry->type == CATCH_TYPE_RESCUE ||
>>               entry->type == CATCH_TYPE_BREAK ||
>>               entry->type == CATCH_TYPE_NEXT) {
>>               entry->sp--;
>>           }
>>       }
>>
>> このコメントですね？

そうです。


> 　この sp-- するのが，なんで sp-- だけでいいのかわからなかった，という
> か，確か，別の場合もあるような気がするけど，まぁ test-all が動くからいい
> か，と思って，もっとちゃんとしないといかんなぁ，と思って書いていたんじゃ
> ないかと思います．

いや、元はもっとひどいコードだったんですよ。

           /* TODO: Dirty Hack!  Fix me */
           if (entry->type == CATCH_TYPE_RESCUE ||
               entry->type == CATCH_TYPE_BREAK ||
               (((ptr[0] & 0x10000) == 0)
                && entry->type == CATCH_TYPE_NEXT)) {
               entry->sp--;
           }

rescue と break と next には引数があるという共通点があるので、
現状のコードはそれなりに妥当だと思います。
0x10000 なんてマジックナンバーではなく CATCH_TYPE_NEXT_WITH_NO_ARG
とか CATCH_TYPE_NEXT_FOR_WHILE とか別のラベルを定義していれば
Dirty とまでは思わなかったですが。


# ちなみに当時の IRC ログ抜粋

13:44:10 <ko1_ndk> catch_type を増やしたらもっと単純になる？
13:46:19 <mame> next をキャッチするけど引数をスタックに載せない catch_type と載せる catch_type ですか
13:46:41 <ko1_ndk> そうそう
13:47:37 <mame> どっちが幸せなのかなあ
13:50:49 <mame> そもそも while の中の next の引数が無意味で気持ち悪い


> 実行時の sp の遷移は，もうちょっとフォーマルにきっちり決められるように
> したいなぁ，と思っていたりして，放ったらかしです（今は，バグがあったらア
> ドホックに直す，って感じ）．遠藤さんがきっちり決まるように，色々変更して
> くれた版があったと思うんだけど，大きすぎて reject しちゃったんですかね．

正式に提案したかどうか覚えてないですが、sp 遷移関係のバグを
撲滅できる (+ コードがきれいになる) 程度のメリットしかなかっ
たので、積極的に取り入れる理由がない、とかだった気がします。


> 本当は，フローグラフかなんかをきちんと作って，きっちりしたいところなん
> ですが．

いちいちメモリ上にグラフを構成まではしないものの、sp をきっ
ちり決めるという私のパッチはそういうパッチだったはず。
コンパイラのコードでフローを表現しているという意味で。

-- 
Yusuke Endoh <[e-mail]>