api: add new top-level SignedDuration type #86
Conversation
BurntSushi
force-pushed
the
ag/signed-duration
branch
7 times, most recently
from
d76885a
to
05852e7
Compare
BurntSushi
force-pushed
the
ag/signed-duration
branch
4 times, most recently
from
02ee5ff
to
d49ba7e
Compare
Previously we framed our tiny little libm port as specific to f64. But in order to support SignedDuration::try_from_secs_f32, I think we'll want f32 libm routines too. So rename and tweak the trait to be less specific to f64.
This is basically just vendoring the 'f' variants (e.g., 'truncf') from the `libm` crate.
BurntSushi
force-pushed
the
ag/signed-duration
branch
from
d49ba7e
to
26122b7
Compare
It is meant to be as close of a mimic of `std::time::Duration` as possible, but signed instead of unsigned. In most cases, it's a drop-in replacement. This type fits more naturally with Jiff's signed `Span` type. We call methods using this type "jiff_duration," which is a bit of a mouthful. In `jiff 0.2`, we'll plan to drop the `jiff_` prefix and remove the corresponding `std::time::Duration` methods. Instead, callers can use `SignedDuration` and then convert to-and-from `std::time::Duration` from there.
These methods permit completely circumventing the `Span` type. They aren't exported yet. We'll do that by adding new methods on the various datetime types in a subsequent commit.
I find 'until' easier to reason about since it reads from left to right. That is, the usual case is that the "before" date comes first and the "after" date comes second. This is to prep for significantly reducing the documentation of the 'since' methods. Namely, they are virtually identical to the 'until' methods. This makes it more difficult for humans to scan for differences *other* than the order of the arguments. So instead, we'll write the docs in terms of 'until' which should make them more digestible. In addition to likely being a good idea on its own, this is also motivated by wanting to add new 'duration_since' and 'duration_until' methods to each of the datetime types that return a `SignedDuration` instead of a `Span`. So in order to avoid repeating the duration documentation AGAIN, we set the stage to reduce things a bit. (We'll do the same for `checked_add` and `checked_sub` as well.)
This trims of the docs of routines like 'checked_sub', 'saturating_sub' and 'since'. Instead, we just say, very briefly, how they differ from 'checked_add', 'saturating_add' and 'until', respectively. This commit also has some other documentation clean-ups, like replacing most uses of `jiff::civil::Date::constant` with `jiff::civil::date`.
This makes it match how 'add' and 'until' methods are ordered on the datetime types.
This rejiggers the checked_{add,sub} methods on all of the datetime
types to be polymorphic. Now, instead of just accepting a concrete
`Span`, they accept one of `Span`, `SignedDuration` or
`std::time::Duration`. This makes their type signatures unfortunately
more complex, but I think it's better than vomitting more methods with
awkward names into the API.
In contrast, we do add `duration_until` and `duration_since` APIs to
each datetime type as well. These APIs are like their corresponding
`until` and `since` APIs, but instead of returning a `Span`, they return
a `SignedDuration`. Since they don't support rounding or balancing, they
are infallible, which is a nice benefit. We stop short of adding methods
for returning a `std::time::Duration`, since 1) a `SignedDuration` can
be converted to a `std::time::Duration` if needed and 2) these APIs can
return negative durations, which would be awkward to express with an
unsigned duration. Moreover, there isn't really a clear name to give
those methods.
We add methods in the case of routines *returning* a `SignedDuration`
because overloading based on return type is more inconvenient and would
require relying on type inference to choose the correct type. So not
only would it be an in-practice breaking change, but it would make
caller code more annoying. Plus, by adding new methods, we express the
fact that they are infallible.
This makes it possible to use `Span::checked_add` with `SignedDuration` and `std::time::Duration`. We don't need to add any new "target" types because we already had a `SpanArithmetic`. So mostly this was just about adding new trait impls and support for adding an absolute duration directly to a `Span`.
I think this was an oversight. I generally don't like using 'impl Trait' in public APIs unless there is no avoiding it. In particular, when using 'impl Trait', it inhibits the use of turbofish, which can sometimes be useful.
BurntSushi
force-pushed
the
ag/signed-duration
branch
from
389bbc8
to
e5196a3
Compare
| /// # Panics | ||
| /// | ||
| /// Panics if the number of hours, after being converted to nanoseconds, | ||
| /// overflows the minimum or maximum `SignedDuration` values. |
There was a problem hiding this comment.
Shall we then provide a fallible constructor like try_from_hours? So does from_mins.
There was a problem hiding this comment.
There aren't fallible constructors for any units. That matches std. The design philosophy is for SignedDuration to match std as closely as possible. I'm open to adding fallible constructors, but I want to see compelling use cases.
There was a problem hiding this comment.
Thanks for your explanation. I noticed that the upstream APIs are unstable also, so let's see what's the consensus - rust-lang/rust#120301
Discussions for both fallible constructor and ambiguous semantic are thrown. I noticed an argument that hours and minutes are not supported for leap seconds and more (ref), but in jiff we typically consider days and above are ambiguous? Is there certain different consideration?
There was a problem hiding this comment.
Sorry, I don't understand your question. Jiff specifically and intentionally doesn't support leap seconds. And whether leap seconds, even if supported, should impact the definition of hours or minutes is unclear.
There was a problem hiding this comment.
doesn't support leap seconds
OK. Then it looks some concerns from the Rust RFC 1040 proposer, not for jiff.
even if supported, should impact the definition of hours or minutes is unclear
Yeah .. So the reason jiff don't support days and above to SignedDuration, is that:
- How many days in a year is indeterminate (leap year)
- How many days in a month is indeterminate (of course)
- How many hours in a day is indeterminate (I'm not sure but perhaps daylight savings things?)
- How many hours in a week is indeterminate (because of day -> hours is indeterminate)
Is this understanding properly?
There was a problem hiding this comment.
I would definitely encourage you to read more of Jiff's docs. And especially the examples. They'll demonstrate a lot of these weird edge cases.
There was a problem hiding this comment.
I would definitely encourage you to read more of Jiff's docs. And especially the examples. They'll demonstrate a lot of these weird edge cases.
Sure .. Thanks for providing such detailed docs.
This PR adds a new top-level
SignedDurationtype to Jiff. This PR alsodoes the work to integrate it into all Jiff datetime APIs. For example,
Zoned::checked_addnow accepts ajiff::Span,jiff::SignedDurationor a
std::time::Duration. To achieve this, we add a new target type,ZonedArithmetic, withFromtrait implementations for each of thecorresponding duration types. We then changed the
span: Spanparameter type to
duration: impl Into<ZonedArithmetic>. We also addnew
Arithmetictarget types for each datetime type, for example,TimestampArithmetic.This design was chosen over using one target type shared across all
datetime types to allow for future extensibility. For example, maybe in
the future we want to add extra options to
Zoned::checked_add. We cannow do that via
ZonedArithmeticin a way that is distinct from otherdatetime types. Moreover, this also avoids defining a new type that
looks a lot like a duration. Instead, the target types are purpose
built and constrained to arithmetic operations involving datetimes and
durations.
The main motivation for this was to facilitate faster arithmetic
operations in cases where folks need them and tighter integration with
the standard library
std::time::Durationtype. TheSpantype isstill the primary duration type in Jiff that almost everyone should be
using most of the time.
Closes #21