Nobuyoshi Nakada
12/5/2007 2:09:00 AM
Hi,
At Wed, 5 Dec 2007 08:40:11 +0900,
Just Another Victim of the Ambient Morality wrote in [ruby-talk:282100]:
> Why is there any "converting" to do? How can this not work?
> My best guess is that the Range class is not written in Ruby and, hence,
> has funny limitation on how it may work.
Yes, and performance issue.
Index: stable/range.c
===================================================================
--- stable/range.c (revision 14103)
+++ stable/range.c (working copy)
@@ -255,10 +255,18 @@ range_each_func(range, func, v, e, arg)
static VALUE
-step_i(i, iter)
+step_i(i, arg)
VALUE i;
- long *iter;
+ VALUE arg;
{
- iter[0]--;
- if (iter[0] == 0) {
+ VALUE *iter = (VALUE *)arg;
+
+ if (FIXNUM_P(iter[0])) {
+ iter[0] -= INT2FIX(1) & ~FIXNUM_FLAG;
+ }
+ else {
+ VALUE one = INT2FIX(1);
+ iter[0] = rb_funcall(iter[0], '-', 1, &one);
+ }
+ if (iter[0] == INT2FIX(0)) {
rb_yield(i);
iter[0] = iter[1];
@@ -308,11 +316,20 @@ range_step(argc, argv, range)
if (rb_scan_args(argc, argv, "01", &step) == 0) {
step = INT2FIX(1);
+ unit = 1;
+ }
+ else if (FIXNUM_P(step)) {
+ unit = NUM2LONG(step);
+ }
+ else {
+ VALUE tmp = rb_to_int(step);
+ unit = rb_cmpint(tmp, step, INT2FIX(0));
+ step = tmp;
}
-
- unit = NUM2LONG(step);
if (unit < 0) {
rb_raise(rb_eArgError, "step can't be negative");
- }
- if (FIXNUM_P(b) && FIXNUM_P(e)) { /* fixnums are special */
+ }
+ if (unit == 0)
+ rb_raise(rb_eArgError, "step can't be 0");
+ if (FIXNUM_P(b) && FIXNUM_P(e) && FIXNUM_P(step)) { /* fixnums are special */
long end = FIX2LONG(e);
long i;
@@ -331,11 +348,9 @@ range_step(argc, argv, range)
if (!NIL_P(tmp)) {
- VALUE args[5];
- long iter[2];
+ VALUE args[5], iter[2];
b = tmp;
- if (unit == 0) rb_raise(rb_eArgError, "step can't be 0");
args[0] = b; args[1] = e; args[2] = range;
- iter[0] = 1; iter[1] = unit;
+ iter[0] = INT2FIX(1); iter[1] = step;
rb_iterate((VALUE(*)_((VALUE)))str_step, (VALUE)args, step_i,
(VALUE)iter);
@@ -344,5 +359,4 @@ range_step(argc, argv, range)
ID c = rb_intern(EXCL(range) ? "<" : "<=");
- if (rb_equal(step, INT2FIX(0))) rb_raise(rb_eArgError, "step can't be 0");
while (RTEST(rb_funcall(b, c, 1, e))) {
rb_yield(b);
@@ -351,7 +365,6 @@ range_step(argc, argv, range)
}
else {
- long args[2];
+ VALUE args[2];
- if (unit == 0) rb_raise(rb_eArgError, "step can't be 0");
if (!rb_respond_to(b, id_succ)) {
rb_raise(rb_eTypeError, "can't iterate from %s",
@@ -359,6 +372,6 @@ range_step(argc, argv, range)
}
- args[0] = 1;
- args[1] = unit;
+ args[0] = INT2FIX(1);
+ args[1] = step;
range_each_func(range, step_i, b, e, args);
}
Index: trunk/range.c
===================================================================
--- trunk/range.c (revision 14103)
+++ trunk/range.c (working copy)
@@ -248,8 +248,14 @@ static VALUE
step_i(VALUE i, void *arg)
{
- long *iter = (long *)arg;
+ VALUE *iter = arg;
- iter[0]--;
- if (iter[0] == 0) {
+ if (FIXNUM_P(iter[0])) {
+ iter[0] -= INT2FIX(1) & ~FIXNUM_FLAG;
+ }
+ else {
+ VALUE one = INT2FIX(1);
+ iter[0] = rb_funcall(iter[0], '-', 1, &one);
+ }
+ if (iter[0] == INT2FIX(0)) {
rb_yield(i);
iter[0] = iter[1];
@@ -298,13 +304,20 @@ range_step(int argc, VALUE *argv, VALUE
if (rb_scan_args(argc, argv, "01", &step) == 0) {
step = INT2FIX(1);
+ unit = 1;
+ }
+ else if (FIXNUM_P(step)) {
+ unit = NUM2LONG(step);
+ }
+ else {
+ VALUE tmp = rb_to_int(step);
+ unit = rb_cmpint(tmp, step, INT2FIX(0));
+ step = tmp;
}
-
- unit = NUM2LONG(step);
if (unit < 0) {
rb_raise(rb_eArgError, "step can't be negative");
- }
+ }
if (unit == 0)
rb_raise(rb_eArgError, "step can't be 0");
- if (FIXNUM_P(b) && FIXNUM_P(e)) { /* fixnums are special */
+ if (FIXNUM_P(b) && FIXNUM_P(e) && FIXNUM_P(step)) { /* fixnums are special */
long end = FIX2LONG(e);
long i;
@@ -323,12 +336,11 @@ range_step(int argc, VALUE *argv, VALUE
if (!NIL_P(tmp)) {
- VALUE args[2];
- long iter[2];
+ VALUE args[2], iter[2];
b = tmp;
args[0] = e;
args[1] = EXCL(range) ? Qtrue : Qfalse;
- iter[0] = 1;
- iter[1] = unit;
+ iter[0] = INT2FIX(1);
+ iter[1] = step;
rb_block_call(b, rb_intern("upto"), 2, args, step_i, (VALUE)iter);
}
@@ -344,5 +356,5 @@ range_step(int argc, VALUE *argv, VALUE
}
else {
- long args[2];
+ VALUE args[2];
if (!rb_respond_to(b, id_succ)) {
@@ -350,6 +362,6 @@ range_step(int argc, VALUE *argv, VALUE
rb_obj_classname(b));
}
- args[0] = 1;
- args[1] = unit;
+ args[0] = INT2FIX(1);
+ args[1] = step;
range_each_func(range, step_i, b, e, args);
}
@@ -867,6 +879,4 @@ void
Init_Range(void)
{
- VALUE members;
-
id_cmp = rb_intern("<=>");
id_succ = rb_intern("succ");
--
Nobu Nakada