Project

General

Profile

Actions

Bug #11113

closed

Time ranges cannot be used in case statements in 1.9+ but they could in 1.8.7

Added by myronmarston (Myron Marston) about 7 years ago. Updated about 7 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
ruby -v:
1.9.2, 1.9.3, 2.0, 2.1, 2.2
[ruby-core:69052]

Description

Given this script:

range = (Time.now - 1000)...(Time.now + 1000)

case Time.now
when range then puts "in range"
else puts "not in range"
end

I'd expect it to print "in range". This worked on 1.8.7:

$ ruby -v time_range_problem.rb
ruby 1.8.7 (2013-12-22 patchlevel 375) [i686-darwin12.5.0]
in range

...but raises an exception in 1.9.2, 1.9.3, 2.0, 2.1 and 2.2:

time_range_problem.rb:4:in `each': can't iterate from Time (TypeError)
        from time_range_problem.rb:4:in `include?'
        from time_range_problem.rb:4:in `include?'
        from time_range_problem.rb:4:in `==='
        from time_range_problem.rb:4:in `<main>'

It looks like Range#=== is broken. Given that === is a standard ruby protocol, I would not expect a built-in type to raise an error like this. IMO, === should either return true or false but should not raise an error. We rely upon === in RSpec to do matching and this behavior means that if users pass us a time range, things blow up. See https://github.com/rspec/rspec-mocks/issues/938 for an example.

Updated by avit (Andrew Vit) about 7 years ago

The "can't iterate" error is simply because Time is float-based: there is no succ number.

Something like this could be a solution:

module Range::RubyIssue11113
  def ===(other)
    case first
    when Time, Float then self.cover?(other)
    else super
    end
  end
end

class Range
  prepend RubyIssue11113
end

Is the difference between a..b and a...b even relevant for float ranges?

Updated by myronmarston (Myron Marston) about 7 years ago

The "can't iterate" error is simply because Time is float-based: there is no succ number.

Floats don't have a succ method, and yet (2.3...3.2) === 3.0 works just fine. Actually, I'm surprised that succ is used for === at all...it implies that === is a O(N) operation over the size of the range. Seems like something like range.begin < other && range.end > other would perform better (constant time) and avoid this problem. I haven't looked into the implementation of cover? but is that basically what it does internally?

Updated by avit (Andrew Vit) about 7 years ago

Good point, it looks like Float is already "covered" (I didn't check) so it's only Time that is the issue here.

Seems like something like range.begin < other && range.end > other would perform better (constant time)

Yes, that's exactly what cover? does, instead of iterating with include?.

Actions #4

Updated by nobu (Nobuyoshi Nakada) about 7 years ago

  • Status changed from Open to Closed

Applied in changeset r50421.


range.c: covered for linear objects

Updated by myronmarston (Myron Marston) about 7 years ago

Thanks for the prompt fix, @nobu (Nobuyoshi Nakada)!!

On a side note, I wasn't notified by email of your fix even though I'm watching this issue so I get notified of updates. Do you know why that is?

Actions #6

Updated by usa (Usaku NAKAMURA) about 7 years ago

  • Backport changed from 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN to 2.0.0: WONTFIX, 2.1: REQUIRED, 2.2: REQUIRED
Actions #7

Updated by usa (Usaku NAKAMURA) about 7 years ago

  • Backport changed from 2.0.0: WONTFIX, 2.1: REQUIRED, 2.2: REQUIRED to 2.0.0: WONTFIX, 2.1: DONE, 2.2: REQUIRED

ruby_2_1 r50581 merged revision(s) 50421.

Updated by nagachika (Tomoyuki Chikanaga) about 7 years ago

  • Backport changed from 2.0.0: WONTFIX, 2.1: DONE, 2.2: REQUIRED to 2.0.0: WONTFIX, 2.1: DONE, 2.2: DONE

Backported into ruby_2_2 at r50630.

Actions

Also available in: Atom PDF