From: hello@... Date: 2020-08-29T12:42:29+00:00 Subject: [ruby-core:99771] [Ruby master Bug#17131] Time.at(time) != time in certain cases Issue #17131 has been updated by phil_pirozhkov (Phil Pirozhkov). jeremyevans0 (Jeremy Evans) wrote in #note-3: > a spec that shows that defining to_r without to_int is expected to raise an error > I think that is sufficient to handle this issue All clear, thanks a lot. I'll handle the usages to adhere to this protocol. ---------------------------------------- Bug #17131: Time.at(time) != time in certain cases https://bugs.ruby-lang.org/issues/17131#change-87283 * 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 `
' ``` 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 `
' ``` 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 `
' 10: from org/jruby/RubyKernel.java:1117:in `eval' 9: from /Users/pirj/.rvm/rubies/jruby-head/bin/irb:23:in `
' 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 `
' 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: