SE-0329 (Third review): Clock, Instant, and Duration

This has reminded me that I believe the initializer should either be failable or trap. In one interpretation, it would fail/trap if attoseconds exceeds 1e18. In your alternative, it would trap if attoseconds + (seconds * 1e18) overflows.

Int64.max * Int64(1e18) + Int64.max doesn't exceed Int128.max—overflow is not possible.

2 Likes

This is the source of my confusion. As written I would expect the attosecondsComponent argument to be strictly less than 1 second and I would expect the attoseconds argument to be unbounded.

But thats just how I interpret the API before reading the future API docs.

1 Like

IMO the getter already has one. eg. we’re using .components.seconds to access the seconds component, so it’s pretty fine.

I'm fine with the proposal as is, but I will add my 2-cents of bike-shedding: It seems like the obvious parallel to var components would be init(components:). That would support round-tripping the value, and would have consistent usage of 'component', 'second', and 'attosecond'.

public init(components: (seconds: Int64, attoseconds: Int64))
9 Likes

This gave me an idea: How about making the components an actual struct? It could look somewhat like this:

public struct Duration: Sendable {
    public struct Components {
        public let seconds: Int64
        public let attoseconds: Int64
    }
    
    public var components: Components { get }
    public init(components: Components)
}

This would keep the way to access the components the same (duration.components.attoseconds), but would also be extensible for the future. E.g. we could later decide to add convenience accessors:

extension Duration.Components {
    public var nanoseconds: Int64 { get }
    public var microseconds: Int64 { get }
    public var milliseconds: Int64 { get }
}
2 Likes

Duration is conceptually a count of attoseconds. When and if Swift gains an Int128 type, Components will seem redundant. As spelled, Components still encourages things like Duration(components: .init(attoseconds: 1e18 + 1)) despite .components returning Duration.Components(seconds: 1, attoseconds: 1). This isn’t the end of the world; being permissive seems better than requiring the client to do math. But the double-indirection doesn’t really solve any problems that couldn’t be solved by just elevating seconds and attoseconds to properties on Duration itself.

To recap some of the discussion here (and also some implementation review) for a few key points:

Per naming - some good arguments have been raised in favor and against the components name. I definitely feel that the name of components is distinctly better than "portion" since it leverages existing terminology. From a consistency standpoint (for as much as we can, shy of going the route of a full structural type for them) the initializer with second and attosecond components seems still like the best option. Anything beyond the tuple return seems like excess baggage to just address later when we get full fledged Int128.

Behaviorally speaking that initializer is additive arithmetic, so it does not need to guard against overflow cases (i.e. no precondition on beyond 1e18 for the attoseconds component). I do feel that because of the questions aptly raised about this; that it does merit documentation around that behavior for what is expected. One of my tasks for today is to update the PR for the implementation with prose to that effect.

From a forward looking perspective - it really feels that the end goal here will be when Int128 is available we will transition to using that via deprecation and replacement. Honestly that seems to me like the most forthright approach.

I will also make sure to update the proposal accordingly; adding a future plans/directions section for the evolution of the API when we get that type. However I believe that is a point at which I will need y'all's help to determine the exact shape of that API.

One issue that has slightly been talked about here but more concretely was raised as a consideration to account for on the implementation review is how this type encodes/decodes in a manner that can be serialized appropriately. JSON can't handle (at least uniformly among parsers) 128 bit numbers. Out of the options available to prevent potential loss there are two real approaches; either encode/decode as a string (numeric value of the attoseconds stored), encode/decode as two 64 bit portions (high/low), or encode/decode as some sort of component. Since the end goal seems pretty clear to have a representation via initializer of a 128 bit signed integer, the string encoding seems the best bet here.

I know this has been a pretty popular proposal to follow and a lot of folks have chimed in and given some passionate, detailed, and useful feedback; so thank you everyone for having patience with me sifting through all of this. I think we really have to come to a good solution that both feels natural and achieves the underlying goals we needed to address!

15 Likes

JSON has no concept of numeric width, all JSON numeric literals are bigints/bigfloats. so from the serialization/parsing perspective there is no problem here. the issue is that swift can’t handle 128 bit numbers right now, so it has no way of receiving that data from a JSON parser.

1 Like

yea, my comment was primarily directed at the existing parsers out there, not per se the specification (or lack there of w.r.t. numbers in JSON)

1 Like

Shorter durations would be more human-readable as a string encoding with separate components.

  • "123456789000000000" in attoseconds (can be parsed by Int128).
  • "0.123456789" in seconds (can be parsed by Decimal128 or Float64).

The actual encoding portion in my opinion is still an implementation detail that needs to be worked out. One consideration is that Foundation may offer encoding/decoding strategies for these as a solution; however I haven't fully determined how that might work.

The requirements that I have are that the default implementation must be able to be round tripped and must use the types available in the standard library.

If we wanted to just aim for round tripping (and lean on human readable encodings from the encoder/decoder strategies) Then the words may also be a decent approach to encode. For example I have to watch out for the fact that objc does not have a numeric type storage of 128 bit integers. Or for example Python would be forced to alter any JSON implementation to use numpy to parse them etc.

The proposal for encoding/decoding in my opinion is sound, but the detail of how that encoding is serialized needs some work still to land the actual implementation. Which is currently one of my top priorities to get done this week.

From a review standpoint it might be best for us to split off the encoding/decoding discussion out such that we can wrap the proposal side of this up.

2 Likes

We're past the scheduled end of this review (Feb 7th), but the thread is still open so I'm hoping it's still OK to provide feedback…

I had an additional thought about 'components' initializers like:

The signed attoseconds value might be confusing when constructing negative durations. For example:

Duration(secondsComponent: -100,
         attosecondsComponent: 700_000_000_000_000_000)

Assuming I got the 0s correct, does this create a duration of -100.7 or -99.3 seconds? (I believe it's the latter, but it's not obvious from just looking at the code.)

1 Like

I believe Philippe intends to document that the semantics are just addition, yes, so it would be -99.3. @Philippe_Hausler?

1 Like

-99.3 is the expected behavior. I almost have documentation finished and will be posting some examples including negatives soon.

3 Likes

SE-0329 has been accepted; I'd like to thank you all for your contributions.

7 Likes