Backport #9575

Step with 0 step is buggy

Added by Marc-Andre Lafortune 12 months ago. Updated 9 months ago.

[ruby-core:61125]
Status:Closed
Priority:Normal
Assignee:Yui NARUSE

Description

I didn't realize that we now allow stepping with a '0' step. It should probably have been mentioned in the NEWS of 2.1.0?

Anyways, couple of bugs with that new feature:

bn = 1 << 100
bn.step(by: 0, to: bn).first(2) # => [bn, bn] ok
bn.step(by: 0).first(2)         # => [bn.to_f, bn.to_f] not ok
bn.step(by: 0, to: 0).first(2)  # => [] not ok

The corresponding size don't all work either:

bn.step(by: 0) # => Float::INFINITY, ok
bn.step(by: 0, to: bn).size # => ZeroDivisionError: divided by 0, should be infinity
bn.step(by: 0, to: 0).size  # => same
1.step(by:0, to: 42).size   # => same

My patch is almost finished.

Associated revisions

Revision 45209
Added by Marc-Andre Lafortune 12 months ago

  • numeric.c: Fix Numeric#step with 0 unit [#9575]

Revision 45209
Added by Marc-Andre Lafortune 12 months ago

  • numeric.c: Fix Numeric#step with 0 unit [#9575]

Revision 45234
Added by Marc-Andre Lafortune 12 months ago

  • NEWS-2.1.0: Mention that step can accept a 0 unit. See #9575

Revision 46401
Added by Tomoyuki Chikanaga 9 months ago

merge revision(s) r45207,r45208,r45209,r45210: [Backport #9575]

* numeric.c: Create var for rb_intern("<=>")
* numeric.c: Fix Numeric#step with 0 unit [Bug #9575]

History

#1 Updated by Marc-Andre Lafortune 12 months ago

Found many other problems when some arguments are floats:

4.2.step(by: 0.0).first(2)           # => [], should be [4.2, 4.2]
4.2.step(by: -0.0).first(2)          # => [-Infinity, -Infinity], should be [4.2, 4.2]
42.step(by: 0, to: -Float::INFINITY).first(2) # => [], should be [42, 42]
42.step(by: 0, to: 42.5).first(2)    # => [42.0, 42.0], should really be [42, 42]

I believe I've fixed all cases.

#2 Updated by Marc-Andre Lafortune 12 months ago

  • Tracker changed from Bug to Backport
  • Project changed from Ruby trunk to Backport21
  • Category deleted (core)
  • Assignee changed from Marc-Andre Lafortune to Yui NARUSE
  • Target version deleted (current: 2.2.0)

You'll have to backport r45206 -> r45208 in addition to r45209.

#3 Updated by Marc-Andre Lafortune 12 months ago

Also, it might be good to modify the 2.1 NEWS file to mention possibility of 0 step.
Thanks

#4 Updated by Yui NARUSE 12 months ago

  • Status changed from Open to Feedback

r45209 conflicts with current ruby_2_1 branch.

Marc-Andre Lafortune wrote:

Also, it might be good to modify the 2.1 NEWS file to mention possibility of 0 step.
Thanks

yeah, could you add NEWS and additional previous commits and NEWS commit?

thanks

#5 Updated by Marc-Andre Lafortune 12 months ago

  • Status changed from Feedback to Open

Yui NARUSE wrote:

r45209 conflicts with current ruby_2_1 branch.

Right, #9570 must be backported first.

More explicitly, the following will backport both without conflicts:

git cherry-pick c1fc20124c
git cherry-pick e184e31c0956..1636c60fe16

yeah, could you add NEWS and additional previous commits and NEWS commit?

I pushed r45234 (ec8de033e72935)

I hope that's what you wanted?

#6 Updated by Tomoyuki Chikanaga 9 months ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100

Applied in changeset r46401.


merge revision(s) r45207,r45208,r45209,r45210: [Backport #9575]

* numeric.c: Create var for rb_intern("<=>")
* numeric.c: Fix Numeric#step with 0 unit [Bug #9575]

Also available in: Atom PDF