Feature #2674: RubyVM::InstructionSequence to accept IOs

• Status: Closed
• Assignee: shyouhei (Shyouhei Urabe)
• Target version: 2.0.0

[shyouhei (Shyouhei Urabe)]

=begin
RipperはRipper#initializeでIOを受け付けますが、ISeqはそうはなっていません。
非対称なのでISeqもIOを受け付けるようにするのはどうでしょうか。

Signed-off-by: Urabe, Shyuohei [e-mail]

iseq.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/iseq.c b/iseq.c
index 3c957c7..2d86f5d 100644
--- a/iseq.c
+++ b/iseq.c
@@ -538,10 +538,14 @@ rb_iseq_compile_with_option(VALUE src, VALUE file, VALUE line, VALUE opt)
rb_compile_option_t option;
const char *fn = StringValueCStr(file);
int ln = NUM2INT(line);

• NODE *node = parse_string(StringValue(src), fn, ln);

• NODE *node;
  rb_thread_t *th = GET_THREAD();
  make_compile_option(&option, opt);

• if (rb_obj_respond_to(src, rb_intern("gets"), 0))

•    node = rb_compile_file(fn, src, ln);
  

• else

•    node = rb_compile_string(fn, StringValue(src), ln);
  

  if (th->base_block && th->base_block->iseq) {
  return rb_iseq_new_with_opt(node, th->base_block->iseq->name,
  file, line, th->base_block->iseq->self,
  -- 1.6.0.4
  =end

[shyouhei (Shyouhei Urabe)]

• Category set to YARV
• Status changed from Open to Assigned
• Assignee set to ko1 (Koichi Sasada)
• Target version set to 1.9.2

=begin

=end

[naruse (Yui NARUSE)]

• Priority changed from 3 to Normal

=begin

=end

[wanabe (_ wanabe)]

=begin
ワナベと申します。

せっかく rb_compile_file があるので、IO を受け付けてくれるなら
その方が嬉しいと思います。ですので、このパッチに賛成です。
=end

[ko1 (Koichi Sasada)]

=begin
(2010/03/31 2:28), _ wanabe wrote::

せっかく rb_compile_file があるので、IO を受け付けてくれるなら
その方が嬉しいと思います。ですので、このパッチに賛成です。

　すみません，この件忘れていました．IO を受け付けるのはいいんです
が，gets のありなし，で見分けるのが正しいかだけ気になっています．そこだ
け，Ruby 的に OK とか NG とか，誰か判断してもらえれば．

ここで使うのは gets だけなんだっけ？

--
// SASADA Koichi at atdot dot net

=end

[wanabe (_ wanabe)]

=begin
ありがとうございます。
NG なケースを見つけてしまったので報告します。これで SEGV します。

a = [nil]
def a.gets
raise
end
RubyVM::InstructionSequence.compile(a)

中を見ると、lex_io_gets から最終的に rb_io_getline_1 を呼び出して
GetOpenFile で RFILE() を使っているようですので、
src が T_FILE かどうかで判断するのがいいのではないかと思います。

あるいは rb_parser_compile_file 内部で、file が T_FILE かどうかで
lex_gets を変える（rb_io_gets を呼ぶか rb_funcall 経由で gets するか）
というのも考えましたが、ちょっと影響範囲がわかりませんでした。
=end

[shyouhei (Shyouhei Urabe)]

=begin
_ wanabe さんは書きました:

ありがとうございます。
NG なケースを見つけてしまったので報告します。これで SEGV します。

それはrb_compile_fileのバグでは...

Signed-off-by: Urabe, Shyuohei [e-mail]

iseq.c | 2 ++
parse.y | 14 ++++++++------
2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/iseq.c b/iseq.c
index 77f1497..2cccfcc 100644
--- a/iseq.c
+++ b/iseq.c
@@ -551,6 +551,8 @@ rb_iseq_compile_with_option(VALUE src, VALUE file, VALUE filepath, VALUE line, V
node = rb_compile_file(fn, src, ln);
else
node = rb_compile_string(fn, StringValue(src), ln);

• if (!node)

•    rb_exc_raise(GET_THREAD()->errinfo);
  

  if (th->base_block && th->base_block->iseq) {
  return rb_iseq_new_with_opt(node, th->base_block->iseq->name,
  file, filepath, line, th->base_block->iseq->self,
  diff --git a/parse.y b/parse.y
  index 340a825..438b8f5 100644
  --- a/parse.y
  +++ b/parse.y
  @@ -5138,6 +5138,12 @@ lex_getline(struct parser_params *parser)

static const rb_data_type_t parser_data_type;

+static VALUE
+lex_get_generic(struct parser_params *parser, VALUE src)
+{

• return rb_funcall(src, rb_intern("gets"), 0);
  +}

#ifndef RIPPER
static NODE*
parser_compile_string(volatile VALUE vparser, const char *f, VALUE s, int line)
@@ -5209,7 +5215,7 @@ rb_parser_compile_file(volatile VALUE vparser, const char *f, VALUE file, int st
NODE *node;

  TypedData_Get_Struct(vparser, struct parser_params, &parser_data_type, parser);

• lex_gets = lex_io_gets;

• lex_gets = lex_get_generic;
  lex_input = file;
  lex_pbeg = lex_p = lex_pend = 0;
  compile_for_eval = rb_parse_in_eval();
  @@ -10319,11 +10325,7 @@ ripper_warningS(struct parser_params *parser, const char *fmt, const char *str)
  STR_NEW2(fmt), STR_NEW2(str));
  }

-static VALUE
-ripper_lex_get_generic(struct parser_params *parser, VALUE src)
-{

• return rb_funcall(src, ripper_id_gets, 0);
  -}
  +#define ripper_lex_get_generic lex_get_generic

static VALUE
ripper_s_allocate(VALUE klass)

1.6.0.4

=end

[mame (Yusuke Endoh)]

=begin
遠藤です。

2010年3月31日14:38 Urabe Shyouhei [e-mail]:

_ wanabe さんは書きました:

ありがとうございます。
NG なケースを見つけてしまったので報告します。これで SEGV します。

それはrb_compile_fileのバグでは...

rb_compile_file は T_FILE を受け取るという仕様だったのでしょう。

卜部さんのパッチをあてると、メソッド呼び出し分の速度劣化が……
はともかく、IO#gets を再定義することでパース中に任意のコードが
実行できるようになりますが、大丈夫でしょうか。

ripper がすでにやってるわけですが、ripper は動作実績なさそう

なので信用できません……

とりあえず continuation を使うと落ちました。

$ ./ruby -e '
require "continuation"
$code = "p :foo"
class IO
def gets
callcc {|c| $c = c }
code, $code = $code, nil
code
end
end
load "foo.rb"
$c.call
'
:foo
-e:11: [BUG] Segmentation fault
(略)

continuation は自己責任としても、何か妙な影響はないですかね。
load/require 命令を乗っ取れるようになるとか気になります。

$ cat foo.rb
p :innocent

$ ./ruby.new -e '
$code = "p :evil"
class IO
def gets
code, $code = $code, nil
code
end
end

 load "foo.rb"

'
:evil

IO#gets が再定義できる状況ならなんでもやり放題ではあるのですが。
wanabe さんと同じく、T_FILE かどうかで判断するのが無難だと思い
ました。

--
Yusuke ENDOH [e-mail]

=end

[shyouhei (Shyouhei Urabe)]

=begin
卜部です。

Yusuke ENDOH さんは書きました:

rb_compile_file は T_FILE を受け取るという仕様だったのでしょう。

getsじゃなきゃヤダという強い動機があるわけではないので、そういう仕様と決まれば
特に異論はありません。ただその場合はripperもT_FILEのほうに倒すべきでしょうね。

=end

[znz (Kazuhiro NISHIYAMA)]

• Start date set to 01/28/2010

=begin

=end

[mame (Yusuke Endoh)]

• Target version changed from 1.9.2 to 2.0.0

=begin
遠藤です。

残念ですが、4/1 の時点で「そういう仕様と決まれば特に異論はありません」
という状況で、その後議論が途絶えてしまったようなので、spec freeze まで
には合意ができなかったと考えます。
target を 1.9.x にさせていただきます。

--
Yusuke Endoh [e-mail]
=end

[shyouhei (Shyouhei Urabe)]

=begin
あっ、はい。

てか1.9.3っていうtarget作りません?
=end

[ko1 (Koichi Sasada)]

• Description updated (diff)

随分放置しております．すみません．
とくに異論はないんですが，今のにあたる，BUG らないパッチ下さい，って感じでしょうか．

[mame (Yusuke Endoh)]

• Status changed from Assigned to Feedback

パッチ募集中だそうです。

--
Yusuke Endoh <[e-mail]

[Glass_saga (Masaki Matsushita)]

• File iseq_io.diff iseq_io.diff added
• File ripper_with_T_FILE.diff ripper_with_T_FILE.diff added

IOとして受け付けるかをT_FILE かどうかで判断するパッチを作りました。
また、ripperについてもT_FILEかどうかで判断し、rb_funcall()経由でgetsを呼ぶのではなくrb_io_gets()を使うパッチを作りました。

2つのパッチを添付します。

[shyouhei (Shyouhei Urabe)]

あー、時間差でGlass_sagaと同じものを作ってしまった。残念

https://github.com/ruby/ruby/pull/136/files

中身見ても同じなのでどっちを採用しても大差ないです。

[shyouhei (Shyouhei Urabe)]

• Status changed from Feedback to Assigned

[ko1 (Koichi Sasada)]

前も書いたけど特に意見はないのでてきとうに入れといてもらってもいいでしょうか．

[ko1 (Koichi Sasada)]

• Assignee changed from ko1 (Koichi Sasada) to shyouhei (Shyouhei Urabe)

これ，どうでしょう．
卜部君にお願いすればいいでしょうか．

[shyouhei (Shyouhei Urabe)]

ko1 (Koichi Sasada) wrote:

これ，どうでしょう．
卜部君にお願いすればいいでしょうか．

はい。遅くならないうちに入れます。

それはそうとしてGlass_Sagaはにコミット権をあげた方が便利ではないかという気がしています。

[mame (Yusuke Endoh)]

• Status changed from Assigned to Closed

卜部さん、

これって r37339 で入ったということでいいですよね？
勝手に閉じますが間違ってたら適当にしてください。

--
Yusuke Endoh [e-mail]