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