Feature #2674

RubyVM::InstructionSequence to accept IOs

Added by Shyouhei Urabe over 5 years ago. Updated over 2 years ago.

[ruby-dev:40202]
Status:Closed
Priority:Normal
Assignee:Shyouhei Urabe

Description

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

Signed-off-by: Urabe, Shyuohei shyouhei@ruby-lang.org


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

iseq_io.diff Magnifier - IOを受け付けるIseq (748 Bytes) Masaki Matsushita, 07/01/2012 10:57 PM

ripper_with_T_FILE.diff Magnifier - T_FILEの方に倒したripper (1.26 KB) Masaki Matsushita, 07/01/2012 10:57 PM

History

#1 Updated by Shyouhei Urabe over 5 years ago

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

=begin

=end

#2 Updated by Yui NARUSE over 5 years ago

  • Priority changed from Low to Normal

=begin

=end

#3 Updated by _ wanabe about 5 years ago

=begin
ワナベと申します。

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

#4 Updated by Koichi Sasada about 5 years ago

=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

#5 Updated by _ wanabe about 5 years ago

=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

#6 Updated by Shyouhei Urabe about 5 years ago

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

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

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

Signed-off-by: Urabe, Shyuohei shyouhei@ruby-lang.org


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

#7 Updated by Yusuke Endoh about 5 years ago

=begin
遠藤です。

2010年3月31日14:38 Urabe Shyouhei shyouhei@ruby-lang.org:

_ 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 mame@tsg.ne.jp

=end

#8 Updated by Shyouhei Urabe about 5 years ago

=begin
卜部です。

Yusuke ENDOH さんは書きました:

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

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

=end

#9 Updated by Kazuhiro NISHIYAMA about 5 years ago

  • Start date set to 01/28/2010

=begin

=end

#10 Updated by Yusuke Endoh about 5 years ago

  • Target version changed from 1.9.2 to 2.0.0

=begin
遠藤です。

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

--
Yusuke Endoh mame@tsg.ne.jp
=end

#11 Updated by Shyouhei Urabe about 5 years ago

=begin
あっ、はい。

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

#12 Updated by Koichi Sasada almost 3 years ago

  • Description updated (diff)

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

#13 Updated by Yusuke Endoh almost 3 years ago

  • Status changed from Assigned to Feedback

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

Yusuke Endoh <mame@tsg.ne.jp

#14 Updated by Masaki Matsushita almost 3 years ago

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

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

#15 Updated by Shyouhei Urabe almost 3 years ago

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

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

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

#16 Updated by Shyouhei Urabe almost 3 years ago

  • Status changed from Feedback to Assigned

#17 Updated by Koichi Sasada over 2 years ago

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

#18 Updated by Koichi Sasada over 2 years ago

  • Assignee changed from Koichi Sasada to Shyouhei Urabe

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

#19 Updated by Shyouhei Urabe over 2 years ago

ko1 (Koichi Sasada) wrote:

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

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

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

#20 Updated by Yusuke Endoh over 2 years ago

  • Status changed from Assigned to Closed

卜部さん、

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

Yusuke Endoh mame@tsg.ne.jp

Also available in: Atom PDF