Stabilize Duration::MAX

[workingjubilee]

Following the suggested direction from #76416 (comment), this PR proposes that Duration::MAX should have been part of the duration_saturating_ops feature flag all along, having been

0. heavily referenced by that feature flag
1. an odd duck next to most of duration_constants, as I expressed in Tracking issue for duration_constants #57391 (comment)
2. introduced in Add saturating methods for Duration #76114 which added duration_saturating_ops

and accordingly should be folded into duration_saturating_ops and therefore stabilized.

r? @m-ou-se

[m-ou-se]

@rfcbot merge

(@workingjubilee can you rebase this? The first commit should now be included in the main branch.)

[rfcbot]

Team member @m-ou-se has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @m-ou-se
• @sfackler

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[workingjubilee]

Rebased!

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[workingjubilee]

I have added a small remark clarifying that Duration::MAX may vary by platform, because std uses it for creating differences between time::Instants (and so should fit such, and so the repr may vary slightly), so that this stabilization does not have to unnecessarily constrain future adjustments to Duration's representation.

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

[m-ou-se]

I have added a small remark clarifying that Duration::MAX may vary by platform, because std uses it for creating differences between time::Instants (and so should fit such, and so the repr may vary slightly), so that this stabilization does not have to unnecessarily constrain future adjustments to Duration's representation.

Considering we already have a stable Duration::new() accepting a u64 for the seconds, I think we can safely guarantee it's at least 584,942,417,355 years. Otherwise it might seem like on 32 bit platforms MAX could be much lower.

Edit: And with .as_secs() and .subsec_nanos() there's not really any space in the other direction either.

[workingjubilee]

I removed the "64-bit" implication part of the remark, because you're completely right that it's not actually dependent on the target's hardware bitness.

Currently you can input values into Duration::new that cause it to panic already, and the nanoseconds may not actually matter on a given OS once you interact with it in any way. Duration is not unique in this: Vec::with_capacity is supposed to be abstract but you can't reserve more memory than you actually have, and File-oriented APIs use u64 or i64 even though 16777216 TiB (u64::MAX bytes) exceeds the 2 TiB capacity of the filesystems of most 32-bit targets.

But people expect that. Duration, however, is supposed to be platform-agnostic, but it is actually Not, because std implicitly relies on the assumption that the range of Duration > range of Instant, and 128-bit numbers and their use are not a hypothetical anymore. I think if that assumption breaks in one direction, it is also worth reviewing whether it should still be valid in the other direction, as chrono doesn't think it's necessary... or at least waiting and seeing for a little bit while the Year 2038 problem and migrations because of it remains an issue for current tier 2 targets and the nature of other types in std::time remains slightly underdocumented. But if there was a firm consensus from T-libs about how arithmetic on Instants, and implicitly, Duration, is supposed to work, I would agree.

[m-ou-se]

Yeah the situation is a bit weird with Duration sitting in core trying to be platform-independent, while std provides functions such as SystemTime::duration_since that can't report an overflow without panicking.

I don't think this description is very useful though:

At least equal to the number of seconds difference between the minimum and maximum std::time::Instant.

Because we don't provide any way to construct arbitrary big Instants other than by sleeping for millions of years or adding a Duration to an Instant. The concept of a maximum and minimum Instant doesn't really have much meaning right now.

How about we just say that it might vary by platform, but right now it's 584,942,417,355 years on all platforms?

[workingjubilee]

I'm good with that! It sounds like the appropriate mix of Ominous and Reassuring.
You're right that "minimum" and "maximum" are erroneous concepts for Instant, I was thinking of it too much like an integer. I tried to reflect the actual details that might impact the value of MAX :

    /// May vary by platform as necessary. Must be able to contain the difference between
    /// two instances of [`Instant`] or two instances of [`SystemTime`].
    /// This constraint gives it a value of about 584,942,417,355 years in practice,
    /// which is currently used on all platforms.

[m-ou-se]

@bors r+ rollup

[bors]

📌 Commit 8278380 has been approved by m-ou-se

[bors]

⌛ Testing commit 8278380 with merge 8d859f836b759db255119449a8cc222b140e63dd...

[bors]

💔 Test failed - checks-actions

[rust-log-analyzer]

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

 58  516M   58  301M    0     0  24.3M      0  0:00:21  0:00:12  0:00:09 24.5M
 60  516M   60  313M    0     0  24.4M      0  0:00:21  0:00:12  0:00:09 24.6M
curl: (56) OpenSSL SSL_read: Connection was reset, errno 10054

gzip: stdin: unexpected end of file
tar: Unexpected EOF in archive
tar: Unexpected EOF in archive
tar: Error is not recoverable: exiting now
##[error]Process completed with exit code 2.
[command]"C:\Program Files\Git\bin\git.exe" version
git version 2.31.1.windows.1
[command]"C:\Program Files\Git\bin\git.exe" config --local --name-only --get-regexp core\.sshCommand
[command]"C:\Program Files\Git\bin\git.exe" submodule foreach --recursive "git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :"

[JohnTitor]

2021-04-26T11:14:16.8477608Z curl: (56) OpenSSL SSL_read: Connection was reset, errno 10054
2021-04-26T11:14:16.8525720Z 
2021-04-26T11:14:16.8526973Z gzip: stdin: unexpected end of file
2021-04-26T11:14:16.8528023Z tar: Unexpected EOF in archive
2021-04-26T11:14:16.8529006Z tar: Unexpected EOF in archive
2021-04-26T11:14:16.8530077Z tar: Error is not recoverable: exiting now

@bors retry