K.Maebashi's BBS

こんにちは、@rednaxelafxことKrisと申します。 たぶん「はじめまして」ではないかと、Twitterで検索したところ、たしかにもっと前から挨拶をしました(笑) https://twitter.com/rednaxelafx/status/14597568550 Web連載も結構前から読みましたが、「プログラミング言語を作る」が本になって本当に嬉しかったです。 去年中国の編集さんにこの本を強く薦めました。いまはやっとその中国語翻訳版が買えるようになりまして、自分のブログでこの本を押したいと思います。 このたびはcrowbarについてちょっと聞きたいことがあります。 本のサンプルコードをこちらからダウンロードしました： http://kmaebashi.com/programmer/devlang/book/unix_utf8_20091228.tgz その中のcrowbar_book_0_4ですが、interface.cでのrelease_global_strings()がこうなっています： static void release_global_strings(CRB_Interpreter *interpreter) { while (interpreter->variable) { Variable *temp = interpreter->variable; interpreter->variable = temp->next; } } この関数がもともと何をしようとしてますか？このままだと、ループしなくても、直接に interpreter->variable = NULL でも同じではないですか？ もしかしたらこうしたかったとか： static void release_global_strings(CRB_Interpreter *interpreter) { while (interpreter->variable) { Variable *temp = interpreter->variable; Variable *temp_next = temp->next; temp->next = NULL; interpreter->variable = temp_next; } }

>こんにちは、@rednaxelafxことKrisと申します。 中国からありがとうございます。 (中国語版がもう出版されているということを私も今知りました) >このたびはcrowbarについてちょっと聞きたいことがあります。 … >その中のcrowbar_book_0_4ですが、interface.cでのrelease_global_strings()が >こうなっています： >static void >release_global_strings(CRB_Interpreter *interpreter) { > while (interpreter->variable) { > Variable *temp = interpreter->variable; > interpreter->variable = temp->next; > } >} 確認しました。確かに無駄なことをしているように見えます。 そもそも関数名がrelease_global_stringsなのに、文字列に対して 何かしているように見えません。 どうしてこうなったのかと思い、調べたのですが、どうも ver.0.1のこのコードが原因なのかと思います。 http://kmaebashi.com/programmer/devlang/crowbar_src_0_1_01/S/15.html#76 static void release_global_strings(CRB_Interpreter *interpreter) { while (interpreter->variable) { Variable *temp = interpreter->variable; interpreter->variable = temp->next; if (temp->value.type == CRB_STRING_VALUE) { crb_release_string(temp->value.u.string_value); } } } ver.0.1時点では、参照カウンタのGCしかなかったので、インタプリタを 破棄するときには文字列の領域はfreeする必要がありました。その処理が 上記のコードです。 ver.0.2でGCを組み込んだので、interpreter->variableをNULLにしておけば、 文字列の領域もcrb_garbage_collect()の呼び出しで解放されます。 ただ、ver.0.2を作ったとき、ヘッダファイルを直してコンパイルエラーを取って、 という作業をしているときに、コンパイルエラーの原因のところだけ削って 関数そのものが不要であることに気付かなかったのではないかと思います。 すみませんでした。再確認のうえ、正誤表に載せる等対応します。 細かいところまでソースを読み込んでいただき、作者冥利に尽きます。 ありがとうございました。

はるほど、納得いたしました。 お忙しい中、丁寧なご返事、ありがとうございました。 - Kris >確認しました。確かに無駄なことをしているように見えます。 >そもそも関数名がrelease_global_stringsなのに、文字列に対して >何かしているように見えません。 > >どうしてこうなったのかと思い、調べたのですが、どうも >ver.0.1のこのコードが原因なのかと思います。 > >http://kmaebashi.com/programmer/devlang/crowbar_src_0_1_01/S/15.html#76 >static void >release_global_strings(CRB_Interpreter *interpreter) { > while (interpreter->variable) { > Variable *temp = interpreter->variable; > interpreter->variable = temp->next; > if (temp->value.type == CRB_STRING_VALUE) { > crb_release_string(temp->value.u.string_value); > } > } >} > >ver.0.1時点では、参照カウンタのGCしかなかったので、インタプリタを >破棄するときには文字列の領域はfreeする必要がありました。その処理が >上記のコードです。 > >ver.0.2でGCを組み込んだので、interpreter->variableをNULLにしておけば、 >文字列の領域もcrb_garbage_collect()の呼び出しで解放されます。 >ただ、ver.0.2を作ったとき、ヘッダファイルを直してコンパイルエラーを取って、 >という作業をしているときに、コンパイルエラーの原因のところだけ削って >関数そのものが不要であることに気付かなかったのではないかと思います。 >すみませんでした。再確認のうえ、正誤表に載せる等対応します。 > >細かいところまでソースを読み込んでいただき、作者冥利に尽きます。 >ありがとうございました。 >

前橋様、 もう一つ、crowbarスタックとネーティブ仮引数について聞きたいことがあります。 crowbar_book_0_4では、crowbarスタックはrealloc()を使って拡張します。そしてネイティブ関数に渡す引数の配列がはcrowbarスタックに積んでいます。realloc()が対象データを移動しかねないので、移動した場合、もしスタックのなかでネイティブ関数に渡す引数があれば、それを指すポインタ（arg_p）が無効化される恐れがありますね。 ついでに、CRB_create_interpreter()で、interpreter->stack.stack_alloc_sizeの初期値が0になってますが、それがSTACK_ALLOC_SIZEになったほうがいいではないでしょうか。stack_alloc_sizeの初期値が0なら、最初にpush_value()を呼び出すと、stack_pointer == stack_alloc_sizeので無駄にMEM_realloc()でcrowbarスタック拡張をします。 - Kris

どたばたしておりまして対応が遅れました。すみません。 >realloc()が対象データを移動しかねないので、移動した場合、もしスタックのなかでネイティブ関数に渡す引数があれば、それを指すポインタ（arg_p）が無効化される恐れがありますね。 本当ですね……初歩的なミスでした。ご指摘ありがとうございます。 直すなら、ポインタでなく添字を渡して関数経由でアクセスするようにするのかと思いますが、ネイティブ関数全体に影響が出ます。少し考えさせてください。 ご指摘ありがとうございました。 >ついでに、CRB_create_interpreter()で、interpreter->stack.stack_alloc_sizeの初期値が0になってますが、それがSTACK_ALLOC_SIZEになったほうがいいではないでしょうか。 NULLを第一引数とするreallocはmallocと同じなので、どちらでもよいのではないでしょうか。

>どたばたしておりまして対応が遅れました。すみません。 いえ、大丈夫です。返事を頂いてありがとうございます。 >>realloc()が対象データを移動しかねないので、移動した場合、もしスタックのなかでネイティブ関数に渡す引数があれば、それを指すポインタ（arg_p）が無効化される恐れがありますね。 > >本当ですね……初歩的なミスでした。ご指摘ありがとうございます。 >直すなら、ポインタでなく添字を渡して関数経由でアクセスするようにするのかと思いますが、ネイティブ関数全体に影響が出ます。少し考えさせてください。 >ご指摘ありがとうございました。 たぶんですけど、直すのに影響があんまり出ないやり方があります。crowbarスタックを一つの配列ではなく、複数の配列をリンクするによりスタックの拡張ができます。スタック拡張する場合には新たな配列をmalloc()して、それをもとにあったスタックとリンクして、それでrealloc()を回避できますますね。これが「chunked stack」とも言います。 >>ついでに、CRB_create_interpreter()で、interpreter->stack.stack_alloc_sizeの初期値が0になってますが、それがSTACK_ALLOC_SIZEになったほうがいいではないでしょうか。 > >NULLを第一引数とするreallocはmallocと同じなので、どちらでもよいのではないでしょうか。 はい、どちらでも良いですね。 - Kris

>たぶんですけど、直すのに影響があんまり出ないやり方があります。crowbarスタックを >一つの配列ではなく、複数の配列をリンクするによりスタックの拡張ができます。 >スタック拡張する場合には新たな配列をmalloc()して、それをもとにあったスタックと >リンクして、それでrealloc()を回避できますますね。これが「chunked stack」とも >言います。 全体を確認していませんが、ざっと見てみると、現状のcrowbarの実装だと スタック操作がpush_value()とかpeek_stack()にカプセル化されているので、push_value()のところで新たにmalloc()してリンクリスト等で管理し、 peek_stack()にて、もし配列の下端より下の領域を参照されたらひとつ前の 領域から算出して返す、ということはできそうですね…… 実のところ同じ現象はDiksamにもあるはずで、こちらはVMの中で直接参照して います。ただし、Diksamは関数呼び出し時にスタックの拡張があるかどうかを 判定していますし、ひとつの関数の実行中だけ連続したスタック領域が見えれば 十分でしょうから、なんとかなるように思います。 (すみません、ちょっとすぐには直せませんが) ご意見ありがとうございました。