close
Skip to content

Implementation of try_from_{nanos_u128,mins,hours,days,weeks}#153683

Open
Zorbatron wants to merge 3 commits intorust-lang:mainfrom
Zorbatron:zb/non_panicking_duration_conversion
Open

Implementation of try_from_{nanos_u128,mins,hours,days,weeks}#153683
Zorbatron wants to merge 3 commits intorust-lang:mainfrom
Zorbatron:zb/non_panicking_duration_conversion

Conversation

@Zorbatron
Copy link
Copy Markdown

@Zorbatron Zorbatron commented Mar 11, 2026

Implements the methods discussed in the related ACP.

impl Duration {
    pub const fn try_from_nanos_u128(nanos: u64) -> Result<Duration, DurationConversionError>;
    pub const fn try_from_weeks(weeks: u64) -> Result<Duration, DurationConversionError>;
    pub const fn try_from_days(days: u64) -> Result<Duration, DurationConversionError>;
    pub const fn try_from_hours(hours: u64) -> Result<Duration, DurationConversionError>;
    pub const fn try_from_mins(mins: u64) -> Result<Duration, DurationConversionError>;
}

ACP: rust-lang/libs-team#749
Tracking issue: #153678

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 11, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Mar 11, 2026

r? @scottmcm

rustbot has assigned @scottmcm.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: @scottmcm, libs
  • @scottmcm, libs expanded to 8 candidates
  • Random selection from Mark-Simulacrum, scottmcm

@rust-log-analyzer

This comment has been minimized.

@Zorbatron Zorbatron force-pushed the zb/non_panicking_duration_conversion branch from d8bc8ed to e0982d7 Compare April 12, 2026 16:06
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 12, 2026

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@Zorbatron
Copy link
Copy Markdown
Author

@scottmcm, sorry for the ping if I'm missing something for this PR to be reviewed, but it's been over a month. Are you available to look this over?

Copy link
Copy Markdown
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code here looks good, I have a question about the stability of the error and display impls. (Probably for the libs-api team to answer.)

View changes since this review

Comment thread library/core/src/error.rs
#[stable(feature = "duration_checked_float", since = "1.66.0")]
impl Error for crate::time::TryFromFloatSecsError {}
#[unstable(feature = "non_panicking_duration_conversion", issue = "153678")]
impl Error for crate::time::DurationConversionError {}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think removing the Error and Display impls from TryFromFloatSecsError is a breaking change?
https://doc.rust-lang.org/std/time/struct.TryFromFloatSecsError.html#impl-Error-for-TryFromFloatSecsError

Maybe it's worth asking if DurationConversionError and its trait impls should be stabilised instead?
rust-lang/libs-team#749 (comment)

Here's the context from the meeting notes approving the ACP:
https://hackmd.io/rGWrzOppS8-ehldMMjWwKQ#new-change-proposal-rusttflibs749-ACP-Add-non-panicking-versions-of-Durationfrom_nanos_u128minshoursdaysweeks

Suddenly not being able to print the return type of the stable try_from_secs_f64() method seems surprising for users. So does having to name it using a stable alias that doesn't appear in the method docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants