Feature #2674

RubyVM::InstructionSequence to accept IOs

Added by Shyouhei Urabe about 4 years ago. Updated over 1 year ago.

[ruby-dev:40202]
Status:Closed
Priority:Normal
Assignee:Shyouhei Urabe
Category:YARV
Target version:2.0.0

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 @@ rbiseqcompilewithoption(VALUE src, VALUE file, VALUE line, VALUE opt)
rbcompileoptiont option;
const char *fn = StringValueCStr(file);
int ln = NUM2INT(line);
- NODE *node = parse
string(StringValue(src), fn, ln);
+ NODE *node;
rbthreadt *th = GETTHREAD();
make
compile_option(&option, opt);

  • if (rbobjrespondto(src, rbintern("gets"), 0))
  • node = rbcompilefile(fn, src, ln);
  • else
  • node = rbcompilestring(fn, StringValue(src), ln); if (th->baseblock && th->baseblock->iseq) { return rbiseqnewwithopt(node, th->baseblock->iseq->name, file, line, th->baseblock->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 about 4 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 about 4 years ago

  • Priority changed from Low to Normal

=begin

=end

#3 Updated by _ wanabe about 4 years ago

=begin
ワナベと申します。

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

#4 Updated by Koichi Sasada about 4 years ago

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

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

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

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

--
// SASADA Koichi at atdot dot net

=end

#5 Updated by _ wanabe about 4 years ago

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

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

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

あるいは rbparsercompilefile 内部で、file が TFILE かどうかで
lexgets を変える(rbiogets を呼ぶか rbfuncall 経由で gets するか)
というのも考えましたが、ちょっと影響範囲がわかりませんでした。
=end

#6 Updated by Shyouhei Urabe about 4 years ago

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

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

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

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 @@ rbiseqcompilewithoption(VALUE src, VALUE file, VALUE filepath, VALUE line, V
node = rbcompilefile(fn, src, ln);
else
node = rbcompilestring(fn, StringValue(src), ln);
+ if (!node)
+ rbexcraise(GETTHREAD()->errinfo);
if (th->base
block && th->baseblock->iseq) {
return rb
iseqnewwithopt(node, th->baseblock->iseq->name,
file, filepath, line, th->baseblock->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 rbdatatypet parserdata_type;

+static VALUE
+lexgetgeneric(struct parserparams *parser, VALUE src)
+{
+ return rb
funcall(src, rbintern("gets"), 0);
+}
+
#ifndef RIPPER
static NODE*
parser
compilestring(volatile VALUE vparser, const char *f, VALUE s, int line)
@@ -5209,7 +5215,7 @@ rb
parsercompilefile(volatile VALUE vparser, const char *f, VALUE file, int st
NODE *node;

  TypedData_Get_Struct(vparser, struct parser_params, &parser_data_type, parser);
  • lexgets = lexio_gets;
  • lexgets = lexgetgeneric;
    lex
    input = file;
    lexpbeg = lexp = lexpend = 0;
    compile
    foreval = rbparseineval();
    @@ -10319,11 +10325,7 @@ ripperwarningS(struct parserparams *parser, const char *fmt, const char *str)
    STRNEW2(fmt), STRNEW2(str));
    }

    -static VALUE
    -ripperlexgetgeneric(struct parserparams *parser, VALUE src)
    -{

  • return rbfuncall(src, ripperidgets, 0);
    -}
    +#define ripper
    lexgetgeneric lexgetgeneric

    static VALUE

    rippersallocate(VALUE klass)

    1.6.0.4

=end

#7 Updated by Yusuke Endoh about 4 years ago

=begin
遠藤です。

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

_ wanabe さんは書きました:

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

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

rbcompilefile は 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 4 years ago

=begin
卜部です。

Yusuke ENDOH さんは書きました:

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

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

=end

#9 Updated by Kazuhiro NISHIYAMA about 4 years ago

  • Start date set to 01/28/2010

=begin

=end

#10 Updated by Yusuke Endoh about 4 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 4 years ago

=begin
あっ、はい。

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

#12 Updated by Koichi Sasada almost 2 years ago

  • Description updated (diff)

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

#13 Updated by Yusuke Endoh almost 2 years ago

  • Status changed from Assigned to Feedback

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

Yusuke Endoh <mame@tsg.ne.jp

#14 Updated by Masaki Matsushita almost 2 years ago

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

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

#15 Updated by Shyouhei Urabe almost 2 years ago

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

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

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

#16 Updated by Shyouhei Urabe almost 2 years ago

  • Status changed from Feedback to Assigned

#17 Updated by Koichi Sasada over 1 year ago

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

#18 Updated by Koichi Sasada over 1 year ago

  • Assignee changed from Koichi Sasada to Shyouhei Urabe

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

#19 Updated by Shyouhei Urabe over 1 year ago

ko1 (Koichi Sasada) wrote:

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

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

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

#20 Updated by Yusuke Endoh over 1 year ago

  • Status changed from Assigned to Closed

卜部さん、

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

Yusuke Endoh mame@tsg.ne.jp

Also available in: Atom PDF