Out of the dozen or so lines of code listed, #asSeconds it looks like
the only one that's broken, it should be the number of seconds since
Time's epoch.  It must've been broken recently, it's still correct in
5.3.

The other accessors without "as", like #seconds, are supposed to be
the "clock readout" of those values, and _should_ never return a
fractional value.  Nothing in Chronology was ever intended to use or
return fractional (e.g., Float or Fraction) values, only integers.  If
we still have #exactSeconds, we should probably delete it and
introduce #fractionalSeconds if you feel you want that.  Subseconds
access was intended to be accessed via the explicit accessors
(#nanoSecond and #asNanoSeconds).  Introducing a new separate
mechanism for precision that utilizes Fractions sounds like it would
subvert the efficiency of your UTC upgrade, and be confusing to have
in conjunction with the API that already provides access to the
subsecond values.

Chronology needs to be fast and efficient.  To me, Tim's original
example of #addSeconds: dropping the nanos seems like the only other
bug, so far.

 - Chris

On Sun, Sep 6, 2020 at 5:08 PM David T. Lewis <lewis at mail.msen.com> wrote:
>
> On Mon, Jul 20, 2020 at 01:08:57PM -0700, tim Rowledge wrote:
> > Time >addSeconds: completely ignores the nanos ivar value and so
> > Time now -> (say)  12:51:47.418205 pm
> > send that #addSeconds: 2 -> 12:51:49 pm
> >
> > Which is incorrect. Actually *fixing* it seems to be a bit more complex
> > than just "add the nanoseconds stuff".
> >
> > Should #asSeconds be truncating the seconds and ignoring the nanoseconds?
> > That's also an issue for #addTime: and #subtractTime: (wait, no #subtractSeconds: ?)
> >
> > Chronology-afficonados assemble!
> >
>
> I'm a bit late following up on this, but I'm working on some unit tests
> and a fix to be submitted to inbox soon.
>
> I took a look at Squeak 3.6 (prior to the introduction of the Chronology
> package). I was expecting that Time instances had originally assumed the
> use of whole seconds, but to my pleasant surprise, it is not so.
>
> Here is what I see in Squeak 3.6, which looks entirely right to me (note
> that I am intentionally using a completely inappropriate Float value to
> illustrate the point):
>
>   t1 := Time fromSeconds: Float pi. "==> 12:00:03.141592653589793 am"
>   t2 := t1 addSeconds: Float pi. "==> 12:00:06.283185307179586 am"
>   t1 seconds. "==> 3.141592653589793"
>   t1 asSeconds. "==> 3.141592653589793"
>   t2 seconds. "==> 6.283185307179586"
>   t2 asSeconds. "==> .283185307179586"
>   t2 asSeconds - t1 asSeconds. "==> 3.141592653589793"
>
> And here is what I see in trunk today, which is clearly broken:
>
>   t1 := Time fromSeconds: Float pi. "==> 12:00:03.141592653 am"
>   t2 := t1 addSeconds: Float pi. "==> 12:00:06.141592654 am"
>   t1 seconds. "==> 3"
>   t1 nanoSecond. "==> 141592653"
>   t1 asSeconds. "==> 3"
>   t1 asNanoSeconds. "==> 3141592653"
>   t2 seconds. "==> 6"
>   t2 nanoSecond. "==> 141592654"
>   t2 asSeconds. "==> 6"
>   t2 asNanoSeconds. "==> 6141592654"
>   t2 asSeconds - t1 asSeconds. "==> 3"
>   t2 asNanoSeconds - t1 asNanoSeconds. "==> 3000000001"
>
> By fixing #addSeconds to respect the nanos ivar, we will have
> something more like this:
>
>   t1 := Time fromSeconds: Float pi. "==> 12:00:03.141592653 am"
>   t2 := t1 addSeconds: Float pi. "==> 12:00:06.283185307 am"
>   t1 seconds. "==> 3"
>   t1 nanoSecond. "==> 141592653"
>   t1 asSeconds. "==> 3"
>   t1 asNanoSeconds. "==> 3141592653"
>   t2 seconds. "==> 6"
>   t2 nanoSecond. "==> 283185307"
>   t2 asSeconds. "==> 6"
>   t2 asNanoSeconds. "==> 6283185307"
>   t2 asSeconds - t1 asSeconds. "==> 3"
>   t2 asNanoSeconds - t1 asNanoSeconds.  "==> 3141592654"
>
> That will still leave us with inconsistent behavior with respect to
> #asSeconds, but that is probably a separate topic, and best checked
> against the behavior of other Smalltalk dialects for compatibility.
>
> Dave
>
>