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

Deprecate std::time::Instant::saturating_duration_since()? #133525

Open
cher-nov opened this issue Nov 27, 2024 · 2 comments
Open

Deprecate std::time::Instant::saturating_duration_since()? #133525

cher-nov opened this issue Nov 27, 2024 · 2 comments
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@cher-nov
Copy link

cher-nov commented Nov 27, 2024

As previously suggested here: #84448 (comment)

Starting from #89926, std::time::Instant::duration_since() effectively does the same thing as the later introduced std::time::Instant::saturating_duration_since() (they are now literally identical). Given that this was a possibly breaking change that has already passed, I propose the latter function for deprecation and subsequent removal, as it currently creates the false ambiguity.

The only objection I see is that the current state of affairs allows to bring back panic! in the future, as mentioned in a comment here:
https://doc.rust-lang.org/1.82.0/src/std/time.rs.html#143-144

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 27, 2024
@jieyouxu jieyouxu added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. C-discussion Category: Discussion or questions that doesn't represent real issues. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Nov 27, 2024
@CodesInChaos
Copy link

I think the semantic difference between these is worth preserving:

When you call duration_since, you say "if later comes before earlier, that's a monotonicity bug". On the other hand it's fine to call saturating_duration_since() if you're not certain which timestamp is supposed to be earlier.

Besides re-introducing the panic, it could be possible to introduce some kind of warning mechanism in the future. Either via specific callback that can be registed, or via a generic warning mechanism.

@cher-nov
Copy link
Author

cher-nov commented Nov 30, 2024

I think the semantic difference between these is worth preserving:

When you call duration_since, you say "if later comes before earlier, that's a monotonicity bug". On the other hand it's fine to call saturating_duration_since() if you're not certain which timestamp is supposed to be earlier.

I disagree. The problem here is that this semantic difference is not backed up or guaranteed by anything, and therefore can only be implied. Which in itself is not necessary, and therefore ephemeral - using saturating_duration_since() with an explanatory comment thus becomes indistinguishable from duration_since().

Besides re-introducing the panic, it could be possible to introduce some kind of warning mechanism in the future. Either via specific callback that can be registed, or via a generic warning mechanism.

This will require a different signature anyway, and therefore a different function - say, checked_duration_since().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. T-libs Relevant to the library team, which will review and decide on the PR/issue. 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