Skip to content

Add functions Duration::try_from_secs_{f32, f64} - #82179

Merged
bors merged 2 commits into
rust-lang:masterfrom
mbartlett21:patch-5
Jun 15, 2021
Merged

Add functions Duration::try_from_secs_{f32, f64}#82179
bors merged 2 commits into
rust-lang:masterfrom
mbartlett21:patch-5

Conversation

@mbartlett21

@mbartlett21mbartlett21 commented Feb 16, 2021•
edited
Loading

Copy link
Copy Markdown
Contributor

These functions allow constructing a Duration from a floating point value that could be out of range without panicking.

Tracking issue: #83400

@rust-highfive

rust-highfive commented Feb 16, 2021

Copy link
Copy Markdown
Contributor

r? @sfackler

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 16, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@marmeladema

marmeladema commented Feb 16, 2021

Copy link
Copy Markdown
Contributor

Just a random contributor opinon here^^.
If the try_ prefix is usually associated with a Result return type then maybe checked_from_secs would work with an Option return type? I feel like if there are different reasons for the failure (or if we suspect their will be in the future), then a Result return type and a proper error being returned is nicer but if it's just a blind failure then a Option is fine.

@jieyouxu

jieyouxu commented Feb 16, 2021

Copy link
Copy Markdown
Member

Maybe it would make sense then to have bothchecked_from_secs_{f32,f64} -> Option<Duration> and try_from_secs_{f32,f64} -> Result<Duration, DurationConversionError> (the checked variant for wishing to handle fallible conversion but don't care for the cause, the try variant for wishing to handle fallible conversion and to know the cause of the conversion failure)? Where DurationConversionError is something like

enumDurationConversionError{NotFinite,// note: maybe distinguish between Inf/-Inf and NaN floating point values?Negative,Overflow}

@marmeladema

marmeladema commented Feb 16, 2021

Copy link
Copy Markdown
Contributor

Well, if we do want a get the actual error and if the error is not especially costly to compute, which is most likely going to be the case, I don't really see the benefit of having another version that returns an Option.
A user of that API can choose to ignore the error, call .ok() to turn into an Option or whatever.

@mbartlett21

mbartlett21 commented Feb 17, 2021

Copy link
Copy Markdown
ContributorAuthor

Should I open a tracking issue where this can be discussed?

I would rather have an opaque error type that just displays different messages for different errors, only using an enum internally. (See ParseIntError for an example)

@crlf0710crlf0710 added the T-libs-api [DEPRECATED; DO NOT USE] label Mar 5, 2021
@JohnCSimonJohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 22, 2021
@Dylan-DPC-zz

Dylan-DPC-zz commented Mar 23, 2021

Copy link
Copy Markdown

@mbartlett21

Should I open a tracking issue where this can be discussed?

yes it is better to do that.

@mbartlett21mbartlett21 mentioned this pull request Mar 23, 2021
Closed
3 tasks
@mbartlett21

mbartlett21 commented Mar 23, 2021

Copy link
Copy Markdown
ContributorAuthor

The tracking issue is #83400

@mbartlett21mbartlett21 changed the title Add functions Duration::try_from_secs_{f32, f64}Add functions Duration::checked_from_secs_{f32, f64}Mar 23, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@mbartlett21mbartlett21 changed the title Add functions Duration::checked_from_secs_{f32, f64}Add functions Duration::try_from_secs_{f32, f64}Mar 23, 2021
@crlf0710crlf0710 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 9, 2021
joshtriplett
joshtriplett reviewed Apr 21, 2021
View reviewed changes
Comment threadlibrary/core/src/time.rs Outdated
@joshtriplettjoshtriplett added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 21, 2021
@mbartlett21

mbartlett21 commented Apr 22, 2021

Copy link
Copy Markdown
ContributorAuthor

@rustbot modify labels +S-waiting-on-review -S-waiting-on-author

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 22, 2021
@JohnTitor

JohnTitor commented Jun 14, 2021

Copy link
Copy Markdown
Member

r? @joshtriplett
and @mbartlett21 it'd be great if you could squash commits, the current is a bit long to read.

@mbartlett21
Add functions Duration::try_from_secs_{f32, f64}
c2c1ca0
This also adds the error type used, `FromSecsError` and its `impl`s.
@mbartlett21
Use try_from_secs_* in Duration::from_secs_* functions.
7803955
`Duration::from_secs_{f32, f64}` now use the results from the
non-panicking functions and unwrap it.
@mbartlett21

mbartlett21 commented Jun 14, 2021

Copy link
Copy Markdown
ContributorAuthor

@JohnTitor I have made it into two commits now, one adding the new functions, and the other changing from_secs_{f32, f64} to use the new functions.

joshtriplett
joshtriplett reviewed Jun 14, 2021
View reviewed changes
Comment threadlibrary/core/src/time.rs
@joshtriplett

joshtriplett commented Jun 14, 2021

Copy link
Copy Markdown
Member

Posted one comment; LGTM otherwise. r=me with that fixed.

@joshtriplett

joshtriplett commented Jun 15, 2021

Copy link
Copy Markdown
Member

@bors r+

@JohnTitorJohnTitor reopened this Jun 15, 2021
@JohnTitor

JohnTitor commented Jun 15, 2021

Copy link
Copy Markdown
Member

@bors r=joshtriplett

@bors

bors commented Jun 15, 2021

Copy link
Copy Markdown
Collaborator

📌 Commit 7803955 has been approved by joshtriplett

@borsbors 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 Jun 15, 2021
@JohnTitorJohnTitor mentioned this pull request Jun 15, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 15, 2021
@bors
Auto merge ofrust-lang#86321- JohnTitor:rollup-q61c8q4, r=JohnTitor
607d6b0
Rollup of 10 pull requests
Successful merges:
- rust-lang#80269 (Explain non-dropped sender recv in docs)
- rust-lang#82179 (Add functions `Duration::try_from_secs_{f32, f64}`)
- rust-lang#85608 (Stabilize `ops::ControlFlow` (just the type))
- rust-lang#85792 (Refactor windows sockets impl methods)
- rust-lang#86220 (Improve maybe_uninit_extra docs)
- rust-lang#86277 (Remove must_use from ALLOWED_ATTRIBUTES)
- rust-lang#86285 (:arrow_up: rust-analyzer)
- rust-lang#86294 (Stabilize {std, core}::prelude::rust_*.)
- rust-lang#86306 (Add mailmap entries for myself)
- rust-lang#86314 (Remove trailing triple backticks in `mut_keyword` docs)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
@bors

bors commented Jun 15, 2021

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 7803955 with merge 607d6b0...

@bors
bors merged commit 1e14d39 into rust-lang:masterJun 15, 2021
@rustbotrustbot added this to the 1.55.0 milestone Jun 15, 2021
@mbartlett21
mbartlett21 deleted the patch-5 branch May 30, 2022 06:19
@lopopololopopolo mentioned this pull request Sep 25, 2022
Merged
JohnTitor pushed a commit to JohnTitor/rust that referenced this pull request Oct 24, 2022
Yuki Okushi
Rollup merge ofrust-lang#102271- lopopolo:lopopolo/stabilize-durati…
c1f9d98
…on-try-from-secs-float, r=dtolnay
Stabilize `duration_checked_float`
## Stabilization Report
This stabilization report is for a stabilization of `duration_checked_float`, tracking issue: rust-lang#83400.
### Implementation History
- rust-lang#82179
- rust-lang#90247
- rust-lang#96051
- Changed error type to `FromFloatSecsError` in rust-lang#90247
- rust-lang#96051 changes the rounding mode to round-to-nearest instead of truncate.
## API Summary
This stabilization report proposes the following API to be stabilized in `core`, along with their re-exports in `std`:
```rust
// core::time
impl Duration {
pub const fn try_from_secs_f32(secs: f32) -> Result<Duration, TryFromFloatSecsError>;
pub const fn try_from_secs_f64(secs: f64) -> Result<Duration, TryFromFloatSecsError>;
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct TryFromFloatSecsError { ... }
impl core::fmt::Display for TryFromFloatSecsError { ... }
impl core::error::Error for TryFromFloatSecsError { ... }
```
These functions are made const unstable under `duration_consts_float`, tracking issue rust-lang#72440.
There is an open question in the tracking issue around what the error type should be called which I was hoping to resolve in the context of an FCP.
In this stabilization PR, I have altered the name of the error type to `TryFromFloatSecsError`. In my opinion, the error type shares the name of the method (adjusted to accommodate both types of floats), which is consistent with other error types in `core`, `alloc` and `std` like `TryReserveError` and `TryFromIntError`.
## Experience Report
Code such as this is ready to be converted to a checked API to ensure it is panic free:
```rust
impl Time {
pub fn checked_add_f64(&self, seconds: f64) -> Result<Self, TimeError> {
// Fail safely during `f64` conversion to duration
if seconds.is_nan() || seconds.is_infinite() {
return Err(TzOutOfRangeError::new().into());
}
if seconds.is_sign_positive() {
self.checked_add(Duration::from_secs_f64(seconds))
} else {
self.checked_sub(Duration::from_secs_f64(-seconds))
}
}
}
```
See: artichoke/artichoke#2194.
`@rustbot` label +T-libs-api -T-libs
cc `@mbartlett21`
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Oct 24, 2022
@Dylan-DPC
Rollup merge ofrust-lang#102271- lopopolo:lopopolo/stabilize-durati…
89d2387
…on-try-from-secs-float, r=dtolnay
Stabilize `duration_checked_float`
## Stabilization Report
This stabilization report is for a stabilization of `duration_checked_float`, tracking issue: rust-lang#83400.
### Implementation History
- rust-lang#82179
- rust-lang#90247
- rust-lang#96051
- Changed error type to `FromFloatSecsError` in rust-lang#90247
- rust-lang#96051 changes the rounding mode to round-to-nearest instead of truncate.
## API Summary
This stabilization report proposes the following API to be stabilized in `core`, along with their re-exports in `std`:
```rust
// core::time
impl Duration {
pub const fn try_from_secs_f32(secs: f32) -> Result<Duration, TryFromFloatSecsError>;
pub const fn try_from_secs_f64(secs: f64) -> Result<Duration, TryFromFloatSecsError>;
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct TryFromFloatSecsError { ... }
impl core::fmt::Display for TryFromFloatSecsError { ... }
impl core::error::Error for TryFromFloatSecsError { ... }
```
These functions are made const unstable under `duration_consts_float`, tracking issue rust-lang#72440.
There is an open question in the tracking issue around what the error type should be called which I was hoping to resolve in the context of an FCP.
In this stabilization PR, I have altered the name of the error type to `TryFromFloatSecsError`. In my opinion, the error type shares the name of the method (adjusted to accommodate both types of floats), which is consistent with other error types in `core`, `alloc` and `std` like `TryReserveError` and `TryFromIntError`.
## Experience Report
Code such as this is ready to be converted to a checked API to ensure it is panic free:
```rust
impl Time {
pub fn checked_add_f64(&self, seconds: f64) -> Result<Self, TimeError> {
// Fail safely during `f64` conversion to duration
if seconds.is_nan() || seconds.is_infinite() {
return Err(TzOutOfRangeError::new().into());
}
if seconds.is_sign_positive() {
self.checked_add(Duration::from_secs_f64(seconds))
} else {
self.checked_sub(Duration::from_secs_f64(-seconds))
}
}
}
```
See: artichoke/artichoke#2194.
``@rustbot`` label +T-libs-api -T-libs
cc ``@mbartlett21``
thomcc pushed a commit to pgcentralfoundation/postgrestd that referenced this pull request Feb 10, 2023
Yuki Okushi
Rollup merge of #102271 - lopopolo:lopopolo/stabilize-duration-try-fr…
bc9b441
…om-secs-float, r=dtolnay
Stabilize `duration_checked_float`
## Stabilization Report
This stabilization report is for a stabilization of `duration_checked_float`, tracking issue: rust-lang/rust#83400.
### Implementation History
- rust-lang/rust#82179
- rust-lang/rust#90247
- rust-lang/rust#96051
- Changed error type to `FromFloatSecsError` in rust-lang/rust#90247
- rust-lang/rust#96051 changes the rounding mode to round-to-nearest instead of truncate.
## API Summary
This stabilization report proposes the following API to be stabilized in `core`, along with their re-exports in `std`:
```rust
// core::time
impl Duration {
pub const fn try_from_secs_f32(secs: f32) -> Result<Duration, TryFromFloatSecsError>;
pub const fn try_from_secs_f64(secs: f64) -> Result<Duration, TryFromFloatSecsError>;
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct TryFromFloatSecsError { ... }
impl core::fmt::Display for TryFromFloatSecsError { ... }
impl core::error::Error for TryFromFloatSecsError { ... }
```
These functions are made const unstable under `duration_consts_float`, tracking issue #72440.
There is an open question in the tracking issue around what the error type should be called which I was hoping to resolve in the context of an FCP.
In this stabilization PR, I have altered the name of the error type to `TryFromFloatSecsError`. In my opinion, the error type shares the name of the method (adjusted to accommodate both types of floats), which is consistent with other error types in `core`, `alloc` and `std` like `TryReserveError` and `TryFromIntError`.
## Experience Report
Code such as this is ready to be converted to a checked API to ensure it is panic free:
```rust
impl Time {
pub fn checked_add_f64(&self, seconds: f64) -> Result<Self, TimeError> {
// Fail safely during `f64` conversion to duration
if seconds.is_nan() || seconds.is_infinite() {
return Err(TzOutOfRangeError::new().into());
}
if seconds.is_sign_positive() {
self.checked_add(Duration::from_secs_f64(seconds))
} else {
self.checked_sub(Duration::from_secs_f64(-seconds))
}
}
}
```
See: artichoke/artichoke#2194.
`@rustbot` label +T-libs-api -T-libs
cc `@mbartlett21`
github-actionsBot pushed a commit to tautschnig/verify-rust-std that referenced this pull request Mar 11, 2025
@JohnTitorgitbot
Rollup merge ofrust-lang#82179- mbartlett21:patch-5, r=joshtriplett
d54382f
Add functions `Duration::try_from_secs_{f32, f64}`
These functions allow constructing a Duration from a floating point value that could be out of range without panicking.
Tracking issue: rust-lang#83400
@BurntSushiBurntSushi mentioned this pull request Dec 30, 2025
Closed
@coderabbitaicoderabbitaiBot mentioned this pull request Jun 22, 2026
Merged
4 tasks
@coderabbitaicoderabbitaiBot mentioned this pull request Aug 12, 2026
Open
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joshtriplettjoshtriplettjoshtriplett left review comments
@oli-obkoli-obkoli-obk left review comments
@yaahcyaahcyaahc left review comments

Assignees

@joshtriplettjoshtriplett

Labels

disposition-mergeThis issue / PR is in PFCP or FCP with a disposition to merge it.F-rust_2018_preview`#![feature(rust_2018_preview)]`finished-final-comment-periodThe final comment period is finished for this PR / Issue.S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-libs-api[DEPRECATED; DO NOT USE]

Projects

None yet

Milestone

1.55.0

Development

Successfully merging this pull request may close these issues.

19 participants

@mbartlett21@rust-highfive@rust-log-analyzer@marmeladema@jieyouxu@Dylan-DPC-zz@joshtriplett@rfcbot@m-ou-se@BurntSushi@JohnTitor@bors@oli-obk@yaahc@crlf0710@sfackler@JohnCSimon@apiraino@rustbot