[ruby-dev:34124] Re: Array#take,take_while,drop,drop_whlie

遠藤です。

08/03/25 に TOYOFUKU Chikanobu<[e-mail]> さんは書きました:
>  take の引数が負のときと大きな整数のときに enum と array で
> 動きが異なっています。

ありがとうございます。


> 負のとき
> $ ruby -e 'p (1..2).take(-1)'
> -e:1:in `take': negative array size (or size too big) (ArgumentError)
>        from -e:1:in `<main>'
> $ ruby -e 'p [1,2].take(-1)'
> nil

negative array size (or size too big) は生々しすぎると思うのですが、
どうしましょう。

- ちゃんとチェックして ArgumentError を投げる
- nil を返す
- 空配列を返す

のどれかでしょうか。とりあえず例外を投げるようにしてみます。


> 大きい整数のとき
> $ ruby -e 'p (1..2).take(10000000000)'
> -e:1:in `take': bignum too big to convert into `long' (RangeError)
>        from -e:1:in `<main>'
> $ ruby -e 'p [1,2].take(10000000000)'
> nil

これは Array#take (と drop) にバグがありました。直します。


> やや大きい整数のとき
> $ ruby -e 'p (1..2).take(1000000000)'
> $ ruby -e 'p [1,2].take(1000000000)'
> [1, 2]
>
>  ここで enum で何も表示されないのは ary_new の中の
>    RARRAY(ary)->ptr = ALLOC_N(VALUE, len);
> のところでどうかなっているようです。

ary_new のチェックがまずいですね。C のキャストは難しいなあ。

あと、(1..2).take(1000000000) などで 1000000000 要素分のバッファを
確保するのは無駄なので、ここは rb_ary_new() にしちゃえばいいと思う
のですが、どうでしょう。あと drop も ([ruby-dev:34092]) 。


Index: array.c
===================================================================
--- array.c	(revision 15837)
+++ array.c	(working copy)
@@ -114,7 +114,7 @@
     if (len < 0) {
 	rb_raise(rb_eArgError, "negative array size (or size too big)");
     }
-    if (len > 0 && len * sizeof(VALUE) <= len) {
+    if (len > 0 && len * (long)sizeof(VALUE) <= len) {
 	rb_raise(rb_eArgError, "array size too big");
     }
     ary = ary_alloc(klass);
@@ -3242,7 +3242,11 @@
 static VALUE
 rb_ary_take(VALUE obj, VALUE n)
 {
-    return rb_ary_subseq(obj, 0, FIX2LONG(n));
+    long len = NUM2LONG(n);
+    if (len < 0) {
+	rb_raise(rb_eArgError, "attempt to take negative size");
+    }
+    return rb_ary_subseq(obj, 0, len);
 }

 /*
@@ -3286,8 +3290,12 @@
 rb_ary_drop(VALUE ary, VALUE n)
 {
     VALUE result;
+    long pos = NUM2LONG(n);
+    if (pos < 0) {
+	rb_raise(rb_eArgError, "attempt to drop negative size");
+    }

-    result = rb_ary_subseq(ary, FIX2LONG(n), RARRAY_LEN(ary));
+    result = rb_ary_subseq(ary, pos, RARRAY_LEN(ary));
     if (result == Qnil) result = rb_ary_new();
     return result;
 }
Index: enum.c
===================================================================
--- enum.c	(revision 15837)
+++ enum.c	(working copy)
@@ -1499,9 +1499,14 @@
 enum_take(VALUE obj, VALUE n)
 {
     VALUE args[2];
+    long len = NUM2LONG(n);

-    args[1] = NUM2LONG(n);
-    args[0] = rb_ary_new2(args[1]);
+    if (len < 0) {
+	rb_raise(rb_eArgError, "attempt to take negative size");
+    }
+
+    args[1] = len;
+    args[0] = rb_ary_new();
     rb_block_call(obj, id_each, 0, 0, take_i, (VALUE)args);
     return args[0];
 }
@@ -1568,7 +1573,7 @@
     VALUE args[2];

     args[1] = NUM2ULONG(n);
-    args[0] = rb_ary_new2(args[1]);
+    args[0] = rb_ary_new();
     rb_block_call(obj, id_each, 0, 0, drop_i, (VALUE)args);
     return args[0];
 }
Index: test/ruby/test_array.rb
===================================================================
--- test/ruby/test_array.rb	(revision 15837)
+++ test/ruby/test_array.rb	(working copy)
@@ -1232,6 +1232,8 @@

   def test_take
     assert_equal([1,2,3], [1,2,3,4,5,0].take(3))
+    assert_raise(ArgumentError, '[ruby-dev:34123]') { [1,2].take(-1) }
+    assert_equal([1,2], [1,2].take(1000000000), '[ruby-dev:34123]')
   end

   def test_take_while
@@ -1240,6 +1242,8 @@

   def test_drop
     assert_equal([4,5,0], [1,2,3,4,5,0].drop(3))
+    assert_raise(ArgumentError, '[ruby-dev:34123]') { [1,2].drop(-1) }
+    assert_equal([], [1,2].drop(1000000000), '[ruby-dev:34123]')
   end

   def test_drop_while

-- 
Yusuke ENDOH <[e-mail]>