Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Request: saturating_add(Duration) for SystemTime / Instant #71224

Open
woody77 opened this issue Apr 16, 2020 · 2 comments
Open

Feature Request: saturating_add(Duration) for SystemTime / Instant #71224

woody77 opened this issue Apr 16, 2020 · 2 comments
Labels
A-time Area: Time C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@woody77
Copy link

woody77 commented Apr 16, 2020

Both SystemTime and Instant have checked_add(Duration), but no saturating_add(Duration).

This seems like an omission, as when doing math operations on time, a developer has to choose between:

  1. allowing for the (hopefully very remote) potential for overflows
  2. determining what to do with the None case of the checked_add()
  3. letting it checked_add(long_duration).unwrap() panic after an overflow

Given that the SystemTime and Instant both opaquely wrap a platform-dependent structure, it's not clear what "far future" values are valid on a given platform (e.g. issue #44394), and what values a developer could correctly use with checked_add(long_duration).unwrap_or(...?).

@jonas-schievink jonas-schievink added C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Apr 16, 2020
@woody77
Copy link
Author

woody77 commented Apr 16, 2020

However, I just discovered that impl Add<Duration> for SystemTime panics on overflow, as it internally is

self.checked_add(duration).expect("overflow when adding duration......

https://doc.rust-lang.org/src/std/time.rs.html#534

@workingjubilee workingjubilee added the A-time Area: Time label Apr 25, 2021
@tarcieri
Copy link
Contributor

Came across this issue and was also thinking that if there were a SystemTime::MAX, it could be implemented (even in end-user code) as .checked_add(duration).unwrap_or(SystemTime::MAX).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-time Area: Time C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants