Add saturating methods for `Duration`#76114

marmeladema · 2020-08-30T17:48:24Z

In some project, I needed a saturating_add method for Duration. I implemented it myself but i thought it would be a nice addition to the standard library as it matches closely with the integers types.

3 new methods have been introduced and are gated by the new duration_saturating_ops unstable feature:

Duration::saturating_add
Duration::saturating_sub
Duration::saturating_mul

If have left the tracking issue to none for now as I want first to understand if those methods would be acceptable at all. If agreed, I'll update the PR with the tracking issue.

Further more, to match the behavior of integers types, I introduced 2 associated constants:

Duration::MIN: this one is somehow a duplicate from Duration::zero() method, but at the time this method was added, MIN was rejected as it was considered a different semantic (see core/time: Add Duration methods for zero #72790 (comment)).
Duration::MAX

Both have been gated by the already existing unstable feature duration_constants, I can introduce a new unstable feature if needed or just re-use the duration_saturating_ops.

We might have to decide whether:

MIN should be replaced by ZERO?
associated constants over methods?

rust-highfive · 2020-08-30T17:48:28Z

r? @shepmaster

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

library/core/src/time.rs

shepmaster

Minor nits.

library/core/src/time.rs

src/test/ui/consts/duration-consts-2.rs

library/core/src/time.rs

shepmaster · 2020-09-05T12:48:39Z

Why the PR close?

marmeladema · 2020-09-05T13:09:58Z

@shepmaster I really don't know what happened. I never meant to close this!

marmeladema · 2020-09-06T18:12:03Z

@shepmaster @pickfire I think i've addressed all the comments 👍

library/core/src/time.rs

pickfire

Looks good to me except the mentioned doc changes.

marmeladema · 2020-09-07T23:44:51Z

@pickfire I've fixed the docs according to your comments 👍

pickfire

Looks good to me.

shepmaster · 2020-09-11T16:46:28Z

@bors r+ rollup

bors · 2020-09-11T16:46:30Z

📌 Commit b869aa5 has been approved by shepmaster

…, r=shepmaster Add saturating methods for `Duration` In some project, I needed a `saturating_add` method for `Duration`. I implemented it myself but i thought it would be a nice addition to the standard library as it matches closely with the integers types. 3 new methods have been introduced and are gated by the new `duration_saturating_ops` unstable feature: * `Duration::saturating_add` * `Duration::saturating_sub` * `Duration::saturating_mul` If have left the tracking issue to `none` for now as I want first to understand if those methods would be acceptable at all. If agreed, I'll update the PR with the tracking issue. Further more, to match the behavior of integers types, I introduced 2 associated constants: * `Duration::MIN`: this one is somehow a duplicate from `Duration::zero()` method, but at the time this method was added, `MIN` was rejected as it was considered a different semantic (see rust-lang#72790 (comment)). * `Duration::MAX` Both have been gated by the already existing unstable feature `duration_constants`, I can introduce a new unstable feature if needed or just re-use the `duration_saturating_ops`. We might have to decide whether: * `MIN` should be replaced by `ZERO`? * associated constants over methods?

Rollup of 7 pull requests Successful merges: - rust-lang#76114 (Add saturating methods for `Duration`) - rust-lang#76297 (rustdoc: fix min_const_generics with ty::Param) - rust-lang#76484 (Add MaybeUninit::assume_init_drop.) - rust-lang#76530 (Eliminate mut reference UB in Drop impl for Rc<T>) - rust-lang#76583 (Update `std::os` module documentation.) - rust-lang#76599 (Finish off revisions for const generics UI tests.) - rust-lang#76615 (Add missing examples on binary core traits) Failed merges: r? `@ghost`

…ax, r=m-ou-se Stabilize Duration::MAX Following the suggested direction from rust-lang#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 rust-lang#57391 (comment) 2. introduced in rust-lang#76114 which added `duration_saturating_ops` and accordingly should be folded into `duration_saturating_ops` and therefore stabilized. r? `@m-ou-se`

rust-highfive assigned shepmaster Aug 30, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 30, 2020

jyn514 added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Aug 30, 2020

marmeladema force-pushed the duration-saturating-ops branch from 044ca51 to 904ec9f Compare August 30, 2020 19:20

pickfire reviewed Aug 31, 2020

View reviewed changes

library/core/src/time.rs Outdated Show resolved Hide resolved

shepmaster reviewed Sep 4, 2020

View reviewed changes

marmeladema closed this Sep 4, 2020

marmeladema reopened this Sep 5, 2020

marmeladema mentioned this pull request Sep 6, 2020

Tracking Issue for Duration saturating operations #76416

Closed

3 tasks

marmeladema force-pushed the duration-saturating-ops branch from 904ec9f to 30aa7dd Compare September 6, 2020 18:05

marmeladema requested a review from shepmaster September 6, 2020 18:11

pickfire reviewed Sep 7, 2020

View reviewed changes

library/core/src/time.rs Outdated Show resolved Hide resolved

pickfire reviewed Sep 7, 2020

View reviewed changes

library/core/src/time.rs Outdated Show resolved Hide resolved

pickfire reviewed Sep 7, 2020

View reviewed changes

library/core/src/time.rs Outdated Show resolved Hide resolved

pickfire reviewed Sep 7, 2020

View reviewed changes

library/core/src/time.rs Outdated Show resolved Hide resolved

pickfire approved these changes Sep 7, 2020

View reviewed changes

Add saturating methods for Duration

b869aa5

marmeladema force-pushed the duration-saturating-ops branch from 30aa7dd to b869aa5 Compare September 7, 2020 23:43

pickfire approved these changes Sep 8, 2020

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 11, 2020

Dylan-DPC-zz mentioned this pull request Sep 12, 2020

Rollup of 11 pull requests #76621

Closed

RalfJung mentioned this pull request Sep 12, 2020

Rollup of 7 pull requests #76637

Merged

bors merged commit 7344f93 into rust-lang:master Sep 12, 2020

rustbot added this to the 1.48.0 milestone Sep 12, 2020

marmeladema deleted the duration-saturating-ops branch September 15, 2020 17:46

faern mentioned this pull request Sep 21, 2020

Tracking issue for duration_constants #57391

Open

workingjubilee mentioned this pull request Apr 12, 2021

Stabilize Duration::MAX #84120

Merged

Uh oh!

Conversation

marmeladema commented Aug 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented Aug 30, 2020

Uh oh!

Uh oh!

shepmaster left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shepmaster commented Sep 5, 2020

Uh oh!

marmeladema commented Sep 5, 2020

Uh oh!

marmeladema commented Sep 6, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pickfire left a comment

Choose a reason for hiding this comment

Uh oh!

marmeladema commented Sep 7, 2020

Uh oh!

pickfire left a comment

Choose a reason for hiding this comment

Uh oh!

shepmaster commented Sep 11, 2020

Uh oh!

bors commented Sep 11, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

marmeladema commented Aug 30, 2020 •

edited

Loading