From: Yusuke ENDOH Date: 2011-03-06T22:53:30+09:00 Subject: [ruby-dev:43321] Re: iseq_compile_each()でのマジックナンバ 遠藤です。 2011年3月6日12:17 SASADA Koichi : > (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 catch_type を増やしたらもっと単純になる？ 13:46:19 next をキャッチするけど引数をスタックに載せない catch_type と載せる catch_type ですか 13:46:41 そうそう 13:47:37 どっちが幸せなのかなあ 13:50:49 そもそも while の中の next の引数が無意味で気持ち悪い > 実行時の sp の遷移は，もうちょっとフォーマルにきっちり決められるように > したいなぁ，と思っていたりして，放ったらかしです（今は，バグがあったらア > ドホックに直す，って感じ）．遠藤さんがきっちり決まるように，色々変更して > くれた版があったと思うんだけど，大きすぎて reject しちゃったんですかね． 正式に提案したかどうか覚えてないですが、sp 遷移関係のバグを 撲滅できる (+ コードがきれいになる) 程度のメリットしかなかっ たので、積極的に取り入れる理由がない、とかだった気がします。 > 本当は，フローグラフかなんかをきちんと作って，きっちりしたいところなん > ですが． いちいちメモリ上にグラフを構成まではしないものの、sp をきっ ちり決めるという私のパッチはそういうパッチだったはず。 コンパイラのコードでフローを表現しているという意味で。 -- Yusuke Endoh