[ruby-core:99754] [Ruby master Bug#17131] Time.at(time) != time in certain cases
From:
merch-redmine@...
Date:
2020-08-28 18:01:44 UTC
List:
ruby-core #99754
Issue #17131 has been updated by jeremyevans0 (Jeremy Evans).
Status changed from Open to Closed
@nobu committed a spec that shows that defining to_r without to_int is expected to raise an error (commit:7e1fddba4a609cb7bf4a696eccd892e68753bb21). I think that is sufficient to handle this issue. If you would like additional documentation added, please reply. Currently, the documentation states a single argument to `Time.at`, if it isn't a time, should be an `Integer, Float, Rational, or other Numeric`.
----------------------------------------
Bug #17131: Time.at(time) != time in certain cases
https://bugs.ruby-lang.org/issues/17131#change-87257
* Author: phil_pirozhkov (Phil Pirozhkov)
* Status: Closed
* Priority: Normal
* ruby -v: 2.0.0 - 2.7.0
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
## Problem
According to the [spec](https://github.com/ruby/ruby/blob/445e5548c9da906a2d7a490e660328b2893d07d1/spec/ruby/core/time/at_spec.rb#L89):
```ruby
describe "with an argument that responds to #to_r" do
it "coerces using #to_r" do
o = mock_numeric('rational')
o.should_receive(:to_r).and_return(Rational(5, 2))
Time.at(o).should == Time.at(Rational(5, 2))
end
end
```
`Time.at` applies `to_r` to its argument if it responds to `to_r`. It doesn't specify though what the result should be.
It preserves the value with `Time` class:
```ruby
time = Time.now
time.nsec # => 110716000
time.nsec == Time.at(time).nsec # => true
```
```ruby
time = Time.at(946684800, 123456789, :nsec)
time.nsec # => 123456789
Time.at(time).nsec # => 123456789
```
and stdlib's `DateTime` class:
```ruby
require 'time'
dt = DateTime.now
dt.to_time.nsec # => 111439000
dt.to_time.nsec == Time.at(dt.to_time).nsec # => true
```
However, `ActiveSupport::TimeWithZone`, which [mimics `Time`](https://github.com/rails/rails/blob/3ddf6b66bca0fd7f79a18864a1d260a3ab323404/activesupport/lib/active_support/time_with_zone.rb#L493) but doesn't inherit from `Time` and rather [delegates to it](https://github.com/rails/rails/blob/3ddf6b66bca0fd7f79a18864a1d260a3ab323404/activesupport/lib/active_support/time_with_zone.rb#L188), does not preserve the value:
```ruby
t=Time.current
t.nsec # => 3882000
Time.at(t).nsec # => 3881931
```
This may look like rounding issue:
> The lowest digits of to_f and nsec are different because IEEE 754 double is not accurate enough to represent the exact number of nanoseconds since the Epoch.
But that doesn't explain why `Time` doesn't suffer from this rounding issue.
## Why?
The ecosystems have provided workarounds:
* https://github.com/travisjeffery/timecop/pull/70
* https://github.com/rails/rails/pull/9403
But they do not work because `to_r` is not called. And it spirals out to downright crutches
* https://github.com/rails/rails/pull/35713
and duct tape
* https://github.com/rspec/rspec-rails/pull/2304
This results in end user specs that do not handle nano-, micro-, or even milliseconds precision:
```ruby
it 'is scheduled in 5 seconds' do
expect { SayHiJob.perform_in_five }
.to have_enqueued_job(SayHiJob).at(a_value_within(1.second).of(Time.current))
end
```
A related Rails issue is
* https://github.com/rails/rails/issues/38831
## Investigation
With a simpler example of a class that pretends to be coercible to `Time`:
```ruby
RationalTime = Class.new do
def respond_to?(method_name, *)
puts method_name
super
end
def to_r
@r
end
def initialize(r)
@r = r
end
end
r = Rational(858710000, 3)
rt = RationalTime.new(r)
puts Time.at(rt)
```
the result is rather frustrating:
```
to_r # <= `Time.at` checked if the class responds to `.to_r`
to_int # but also checked if it responds to `to_int`!
1.rb:19:in `at': can't convert RationalTime into an exact number (TypeError)
from 1.rb:19:in `<main>'
```
It seems to fail at the following test:
```ruby
describe "with an argument that responds to #to_r" do
it "coerces using #to_r" do
```
We see that `respond_to?` was called for `to_int`. Let's implement it:
```ruby
RationalTime = Class.new do
def to_r
Rational(1111, 3)
end
def to_int
100
end
end
puts Time.at(RationalTime.new).nsec # => 333333333
puts Time.at(RationalTime.new).to_f # => 370.3333333333333
```
The returned value is far away from `100`, but it raises no errors, and our rational part is finally used.
## Nanoseconds rounding
`Time.now` returns time with no nanosecond precision, e.g.:
```ruby
Time.now.nsec # => 225_852_000
```
I do not know whether defining `is_a?` and `kind_of?` makes any difference:
```ruby
RationalTime = Class.new do
def is_a?(klass)
klass == ::Time || super
end
def kind_of?(klass)
klass == ::Time || super
end
def to_r
Rational(1111, 3)
end
def to_int
100
end
end
```
## Digging deeper
The following mostly guessing.
According to [Ruby core spec that covers Time.at with a Rational](https://github.com/ruby/ruby/blob/e5db3da9d34f0a7595208863301c044b612adbed/spec/ruby/core/time/at_spec.rb#L26):
```ruby
it "roundtrips a Rational produced by #to_r" do
t = Time.now()
t2 = Time.at(t.to_r)
t2.should == t
t2.usec.should == t.usec
t2.nsec.should == t.nsec
end
```
it seems that `Time.at` copies the underlying time data without modification if the argument is detected to be `Time` ([Ruby source](https://github.com/ruby/ruby/blob/e5db3da9d34f0a7595208863301c044b612adbed/time.c#L2853):
```C
else if (IsTimeval(time)) {
struct time_object *tobj, *tobj2;
GetTimeval(time, tobj);
t = time_new_timew(klass, tobj->timew);
GetTimeval(t, tobj2);
TZMODE_COPY(tobj2, tobj);
}
```
where `IsTimeval` is defined by:
```C
#define IsTimeval(obj) rb_typeddata_is_kind_of((obj), &time_data_type)
```
where `rb_typeddata_is_kind_of` is [defined as](https://github.com/ruby/ruby/blob/aefb13eb631cc5cd784fe2fc10f1f333a2c5e68c/error.c#L888) (error.c?):
```C
rb_typeddata_is_kind_of(VALUE obj, const rb_data_type_t *data_type)
{
if (!RB_TYPE_P(obj, T_DATA) ||
!RTYPEDDATA_P(obj) || !rb_typeddata_inherited_p(RTYPEDDATA_TYPE(obj), data_type)) {
return 0;
}
return 1;
}
```
The hope that `IsTimeval` evaluates to `true` for an impersonating class is debunked by an experimental result:
```ruby
RTInteger = Class.new do
def is_a?(klass)
klass == ::Time || super
end
def kind_of?(klass)
klass == ::Time || super
end
def to_int
100
end
end
Time.at(RTInteger.new) # => 1970-01-01 02:01:40 +0200
```
Our time passes through a series of lossy transformations ([1](https://github.com/ruby/ruby/blob/e5db3da9d34f0a7595208863301c044b612adbed/time.c#L2861), [2](https://github.com/ruby/ruby/blob/e5db3da9d34f0a7595208863301c044b612adbed/time.c#L314)):
```C
timew = rb_time_magnify(v2w(num_exact(time)));
t = time_new_timew(klass, timew);
```
```C
v2w(VALUE v)
{
if (RB_TYPE_P(v, T_RATIONAL)) {
if (RRATIONAL(v)->den != LONG2FIX(1))
return WIDEVAL_WRAP(v);
v = RRATIONAL(v)->num;
}
```
There is a check for it to be a `Rational`, but I can't find where `respond_to?(:to_r)` call comes from. In any case, it ends up deciding not to call `.to_r` for some reason.
## Regression or undefined behavior?
Let's run the snippet from the beginning of this ticket with `RationalTime` on different Ruby versions/implementations.
```ruby
RationalTime = Class.new do
def to_r
Rational(1111, 3)
end
def to_int
100
end
end
puts Time.at(RationalTime.new).nsec
puts Time.at(RationalTime.new).to_f
```
With `to_int` defined:
```ruby
Time.at(RationalTime.new).nsec # => 333333333
Time.at(RationalTime.new).to_f # => 370.3333333333333
```
Without `to_int` defined:
```ruby
TypeError: can't convert RationalTime into an exact number
from (irb):7:in `at'
from (irb):7
from /Users/pirj/.rvm/rubies/ruby-2.0.0-p648/bin/irb:12:in `<main>'
```
It's consistent from 2.0.0 throughout 2.7.0.
JRuby (jruby 9.3.0.0-SNAPSHOT (2.6.5) 2020-08-25 2f0c49000a OpenJDK 64-Bit Server VM 14.0.1+14 on 14.0.1+14 +jit [darwin-x86_64]):
with `to_int`:
```ruby
Time.at(RationalTime.new).nsec # => 333333333
Time.at(RationalTime.new).to_f # => 370.33333333300004
```
Without `to_int`:
```
Traceback (most recent call last):
11: from /Users/pirj/.rvm/rubies/jruby-head/bin/jruby_executable_hooks:24:in `<main>'
10: from org/jruby/RubyKernel.java:1117:in `eval'
9: from /Users/pirj/.rvm/rubies/jruby-head/bin/irb:23:in `<main>'
8: from org/jruby/RubyKernel.java:1078:in `load'
7: from /Users/pirj/.rvm/rubies/jruby-head/lib/ruby/gems/shared/gems/irb-1.0.0/exe/irb:11:in `<main>'
6: from org/jruby/RubyKernel.java:1263:in `catch'
5: from org/jruby/RubyKernel.java:1263:in `catch'
4: from org/jruby/RubyKernel.java:1524:in `loop'
3: from org/jruby/RubyKernel.java:1117:in `eval'
2: from (irb):7:in `evaluate'
1: from org/jruby/RubyTime.java:1329:in `at'
TypeError (can't convert RationalTime into an exact number)
```
## Additional note: precision
The spec:
```ruby
describe "with an argument that responds to #to_r" do
it "coerces using #to_r" do
o = mock_numeric('rational')
o.should_receive(:to_r).and_return(Rational(5, 2))
Time.at(o).should == Time.at(Rational(5, 2))
end
end
```
doesn't provide much precision, as `Rational(5, 2)` is 2.5. Obviously, microseconds and nanoseconds are all zero, and this time it has 500 milliseconds. `Rational(22, 7)` would yield more floating digits to make assertions on nanosecond precision.
## How to fix
The point is to let `Time.at` adhere to duck typing. If the only argument to `Time.at` responds to `to_r`, it should be used, and `to_int` shouldn't be required, since it's not used down the lines anyway.
In addition to that, it would be nice to adjust the specs so that if a `Rational` or an object that responds to `to_r` is passed, then all of its precision is kept, i.e:
```ruby
describe "with an argument that responds to #to_r" do
it "coerces using #to_r" do
o = mock_numeric('rational')
o.should_receive(:to_r).and_return(Rational(22, 7))
Time.at(o).should == Time.at(Rational(22, 7))
Time.at(o).nsec.should == 142857142
Time.at(o).to_f.round(8).should == 3.14285714
Time.at(o).to_f.round(9).should == 3.142857143 # Not sure if JRuby is capable of this precision though
end
end
```
--
https://bugs.ruby-lang.org/
Unsubscribe: <mailto:ruby-core-request@ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>