Skip to content

Commit f56f3ea

Browse files
committed
[Bug #21030] Fix step for non-numeric range
When the end points of an inclusive range equal, `Range#step` should yields the element once.
1 parent d9e1a7c commit f56f3ea

File tree

2 files changed

+62
-41
lines changed

2 files changed

+62
-41
lines changed

range.c

Lines changed: 46 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,7 @@ range_step(int argc, VALUE *argv, VALUE range)
488488

489489
b = RANGE_BEG(range);
490490
e = RANGE_END(range);
491+
v = b;
491492

492493
const VALUE b_num_p = rb_obj_is_kind_of(b, rb_cNumeric);
493494
const VALUE e_num_p = rb_obj_is_kind_of(e, rb_cNumeric);
@@ -559,7 +560,8 @@ range_step(int argc, VALUE *argv, VALUE range)
559560
rb_yield(LONG2NUM(i));
560561
i += unit;
561562
}
562-
} else {
563+
}
564+
else {
563565
if (!EXCL(range))
564566
end += 1;
565567
i = FIX2LONG(b);
@@ -571,7 +573,8 @@ range_step(int argc, VALUE *argv, VALUE range)
571573
}
572574
else if (b_num_p && step_num_p && ruby_float_step(b, e, step, EXCL(range), TRUE)) {
573575
/* done */
574-
} else if (!NIL_P(str_b) && FIXNUM_P(step)) {
576+
}
577+
else if (!NIL_P(str_b) && FIXNUM_P(step)) {
575578
// backwards compatibility behavior for String only, when no step/Integer step is passed
576579
// See discussion in https://bugs.ruby-lang.org/issues/18368
577580

@@ -583,7 +586,8 @@ range_step(int argc, VALUE *argv, VALUE range)
583586
else {
584587
rb_str_upto_each(str_b, e, EXCL(range), step_i, (VALUE)iter);
585588
}
586-
} else if (!NIL_P(sym_b) && FIXNUM_P(step)) {
589+
}
590+
else if (!NIL_P(sym_b) && FIXNUM_P(step)) {
587591
// same as above: backward compatibility for symbols
588592

589593
VALUE iter[2] = {INT2FIX(1), step};
@@ -594,47 +598,48 @@ range_step(int argc, VALUE *argv, VALUE range)
594598
else {
595599
rb_str_upto_each(sym_b, rb_sym2str(e), EXCL(range), sym_step_i, (VALUE)iter);
596600
}
597-
} else {
598-
v = b;
599-
if (!NIL_P(e)) {
600-
if (b_num_p && step_num_p && r_less(step, INT2FIX(0)) < 0) {
601-
// iterate backwards, for consistency with ArithmeticSequence
602-
if (EXCL(range)) {
603-
for (; r_less(e, v) < 0; v = rb_funcall(v, id_plus, 1, step))
604-
rb_yield(v);
605-
}
606-
else {
607-
for (; (c = r_less(e, v)) <= 0; v = rb_funcall(v, id_plus, 1, step)) {
608-
rb_yield(v);
609-
if (!c) break;
610-
}
611-
}
612-
613-
} else {
614-
// Direction of the comparison. We use it as a comparison operator in cycle:
615-
// if begin < end, the cycle performs while value < end (iterating forward)
616-
// if begin > end, the cycle performs while value > end (iterating backward with
617-
// a negative step)
618-
dir = r_less(b, e);
619-
// One preliminary addition to check the step moves iteration in the same direction as
620-
// from begin to end; otherwise, the iteration should be empty.
621-
if (r_less(b, rb_funcall(b, id_plus, 1, step)) == dir) {
622-
if (EXCL(range)) {
623-
for (; r_less(v, e) == dir; v = rb_funcall(v, id_plus, 1, step))
624-
rb_yield(v);
625-
}
626-
else {
627-
for (; (c = r_less(v, e)) == dir || c == 0; v = rb_funcall(v, id_plus, 1, step)) {
628-
rb_yield(v);
629-
if (!c) break;
630-
}
631-
}
632-
}
601+
}
602+
else if (NIL_P(e)) {
603+
// endless range
604+
for (;; v = rb_funcall(v, id_plus, 1, step))
605+
rb_yield(v);
606+
}
607+
else if (b_num_p && step_num_p && r_less(step, INT2FIX(0)) < 0) {
608+
// iterate backwards, for consistency with ArithmeticSequence
609+
if (EXCL(range)) {
610+
for (; r_less(e, v) < 0; v = rb_funcall(v, id_plus, 1, step))
611+
rb_yield(v);
612+
}
613+
else {
614+
for (; (c = r_less(e, v)) <= 0; v = rb_funcall(v, id_plus, 1, step)) {
615+
rb_yield(v);
616+
if (!c) break;
633617
}
634618
}
635-
else
636-
for (;; v = rb_funcall(v, id_plus, 1, step))
619+
620+
}
621+
else if ((dir = r_less(b, e)) == 0) {
622+
if (!EXCL(range)) {
623+
rb_yield(v);
624+
}
625+
}
626+
else if (dir == r_less(b, rb_funcall(b, id_plus, 1, step))) {
627+
// Direction of the comparison. We use it as a comparison operator in cycle:
628+
// if begin < end, the cycle performs while value < end (iterating forward)
629+
// if begin > end, the cycle performs while value > end (iterating backward with
630+
// a negative step)
631+
// One preliminary addition to check the step moves iteration in the same direction as
632+
// from begin to end; otherwise, the iteration should be empty.
633+
if (EXCL(range)) {
634+
for (; r_less(v, e) == dir; v = rb_funcall(v, id_plus, 1, step))
635+
rb_yield(v);
636+
}
637+
else {
638+
for (; (c = r_less(v, e)) == dir || c == 0; v = rb_funcall(v, id_plus, 1, step)) {
637639
rb_yield(v);
640+
if (!c) break;
641+
}
642+
}
638643
}
639644
return range;
640645
}

test/ruby/test_range.rb

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -594,6 +594,22 @@ def test_step_ruby_core_35753
594594
assert_equal(4, (1.0...5.6).step(1.5).to_a.size)
595595
end
596596

597+
def test_step_with_nonnumeric_endpoint
598+
num = Data.define(:value) do
599+
def coerce(o); [o, 100]; end
600+
def <=>(o) value<=>o; end
601+
def +(o) with(value: value + o) end
602+
end
603+
i = num.new(100)
604+
605+
assert_equal([100], (100..100).step(10).to_a)
606+
assert_equal([], (100...100).step(10).to_a)
607+
assert_equal([100], (100..i).step(10).to_a)
608+
assert_equal([i], (i..100).step(10).to_a)
609+
assert_equal([], (100...i).step(10).to_a)
610+
assert_equal([], (i...100).step(10).to_a)
611+
end
612+
597613
def test_each
598614
a = []
599615
(0..10).each {|x| a << x }

0 commit comments

Comments
 (0)