Document that SystemTime does not count leap seconds

[ijackson]

Fixes #77994

This may not be entirely uncontroversial. I know that @Ekleog is going to disagree. However, in support of this docs change:

This documents the current behaviour. The alternative would be to plan to change the behaviour.

There are many programs which need to get a POSIX time (a time_t). Right now, duration_since(UNIX_EPOCH) is the only facility in std that does that. So, that is what programs use. Changing the behaviour would break[1] all of those programs. We would need to define a new API that can be used to get a POSIX time, and get everyone to use it. This seems highly unpalatable.

And, even if we wanted to do that, time with leap seconds is a lot less easy to work with. We would need to arrange to have a leap seconds table available to std somehow, and make sure that it was kept up to date. Currently we don't offer to do that for timezone data, which has similar needs. There are other complications. So it seems it would be awkwarrd to implement a facility that provides time including leap seconds, and the resulting value would be hard for applications to work with.

Therefore, I think it's clear that we don't want to plan to ever change SystemTime. We should plan to keep it the way it is. Providing TAI (for example) should be left to external crates, or additional APIs we may add in the future.

For more discussion see #77994 and in particular @fanf2's #77994 (comment)

[1] Of course, by "break" we really only mean future breakage in the case where there is, in fact, ever another leap second. There may well not be: they are in the process of being abolished (although this is of course being contested). But if we decide that SystemTime::now().duraton_since(UNIX_EPOCH) counts leap seconds, it would start to return Durationss that are 27s different to the current answers. That's clearly unacceptable. And we can hardly change UNIX_EPOCH by 27s.

[rustbot]

r? @thomcc

(rustbot has picked a reviewer for you, use r? to override)

[rustbot]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[ijackson]

@rustbot label +T-libs-api -T-libs

[Ekleog]

My opposition is more of a principled opposition: I think we should eventually deprecate duration_since in favor of some other name that makes it clear that the returned number is not an actual duration.

However, I'm still all an favor of fixing the documentation so that at least the current behavior is well-documented :)

I'm just thinking this PR is missing the place that is actually most subject to issues, which is the duration_since method; as the following code might have very surprising behavior if presented with a leap second even on an otherwise well-behaved system… and it's actually the code example that's currently being given to SystemTime::duration_since: (including, I'd guess, probably panicking on some OSes, though hopefully none of the main ones)

let start = SystemTime::now();
do_stuff();
println!("Time it took to run do_stuff: {:?}", SystemTime::now().duration_since(start).unwrap())

[thomcc]

Our behavior here is clearly an API decision, that requires some amount of discussion, so I'm nominating this.

The status quo is that we do not count leap seconds. Do we want to guarantee that, or leave it open to change? There is further discussion in #77994.

[m-ou-se]

We discussed this in a recent libs api meeting. We came to the same conclusion as above: the only way to convert a SystemTime to a date/time currently is by subtracting UNIX_EPOCH from it. If the resulting Duration would include leap seconds, an accurate conversion wouldn't be possible. (Even if we were to ship a database of leap seconds; future leap seconds aren't even known yet.)

So, SystemTime should not include leap seconds, and we should document that guarantee. Any other option would make SystemTime mostly unusable and incompatible with how it's already used in many places today.

[ijackson]

So, SystemTime should not include leap seconds, and we should document that guarantee. Any other option would make SystemTime mostly unusable and incompatible with how it's already used in many places today.

Cool, thanks. That's what I'm intending with the text in this MR. @thomcc, does that text look good to you?

[thomcc]

Sorry for the delay, I had some family issues that caused me to fall behind on reviews.

A bit bikesheddy, but I would prefer these have their wording tweaked.

[thomcc]

I think enumerating these probably is over the line for too much documentation, since it might lead people to file PRs adding other protocols. I also would prefer saying "This is $description, same as $POSIX_equivalent" rather than the other way around.

So my suggesion would be for this (and the other) comment to be more like:

duration_since(UNIX_EPOCH).unwrap().as_secs() returns the number of non-leap seconds since the start of 1970 UTC, as a u64. This is the same time representation as used in POSIX time_t, as well as a wide variety of internet protocols.

(Note: I'm not tied to precise wording except for the bits I mentioned above). Thoughts?

[ijackson]

I'm OK with these suggestions and have adopted them. Thanks.

[thomcc]

@rustbot author

[ijackson]

@rustbot reviewer

[thomcc]

@bors r+

[bors]

📌 Commit c4bc16c has been approved by thomcc

It is now in the queue for this repository.

[thomcc]

@bors rollup