Add Duration from nanos u128 #139243

omanirudh · 2025-04-02T08:48:52Z

What does this PR do?

This draft PR adds the Duration::from_nanos_u128 function to handle durations that exceed the range of u64, allowing for time spans greater than ~584 years.

Motivation

The current Duration API does not support creating durations from nanoseconds represented as u128. This addition addresses that limitation.

Tracking Issue

Fixes #139201

Details

Introduced Duration::from_nanos_u128 as a const fn similar to other functions in the file.
Ensured safety by validating the nanoseconds before using unsafe code.
To do : complete the documentation and examples for the new function.

r? @RalfJung

rustbot · 2025-04-02T08:48:56Z

r? @tgross35

rustbot has assigned @tgross35.
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

library/core/src/time.rs

RalfJung · 2025-04-02T12:06:56Z

Thanks for taking this over!
However, this needs a libs reviewer. I just proposed the API. :)

r? libs

omanirudh · 2025-04-02T15:08:45Z

Thanks for taking this over! However, this needs a libs reviewer. I just proposed the API. :)

r? libs

Happy to take this up. noted.

library/core/src/time.rs

tgross35 · 2025-04-02T18:06:31Z

@rustbot author

rustbot · 2025-04-02T18:06:36Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

library/core/src/time.rs

omanirudh · 2025-04-11T12:42:25Z

@rustbot ready
r? @tgross35
apologies for the many commits. too many errors.

Skgland · 2025-04-11T13:03:20Z

library/core/src/time.rs

+    #[inline]
+    pub const fn from_nanos_u128(nanos: u128) -> Duration {
+        const NANOS_PER_SEC: u128 = self::NANOS_PER_SEC as u128;
+        let secs: u64 = (nanos / NANOS_PER_SEC) as u64;


The function documentation currently states that the function panics on overflow,
but this cast truncates the result of the division instead of panicking on overflow.

e.g. u128::MAX results in 13088917067439035463.768211455s instead of panicking.

playground

thanks for pointing it out @Skgland
will add the following check and ensure it panics on overflow

if expected_secs > u64::MAX.into() { panic!("overflow in duration in Duration::from_nanos_u128"); }

library/core/src/time.rs

madhav-madhusoodanan · 2025-04-11T17:04:55Z

library/core/src/time.rs

+            panic!("overflow in duration in Duration::from_nanos_u128");
+        }
+        let subsec_nanos = (nanos % NANOS_PER_SEC) as u32;
+        // SAFETY: x % 1_000_000_000 < 1_000_000_000


new_unchecked is unsafe since there is no bounds check being performed on the input value. Here is the reference.

(The comment saying that val == 0 is language UB doesn't exactly apply here since the Nanosecond type is defined to be between 0 and 999_999_999 both inclusive).

Just so that our safety assumption is clear here, do add the fact that subsec_nanos >= 0 because u128 >=0 and u32 >= 0

madhav-madhusoodanan · 2025-04-11T17:12:28Z

Rest of the code looks good to me, amazing work!

Only need to make small changes on the documentation side of things (along with @Skgland 's review about panicking on overflow instead of truncating on overflow).

Do await a review from someone from the libs team too.

madhav-madhusoodanan · 2025-04-11T17:21:52Z

About the commit history (this is completely upto you)

You can use git rebase -i HEAD~<number of commits>. This opens up an interactive window using which you can reorganize and meld related commits together.

This is a related thread on zulip on the concerns behind squashing vs merging commits.

tgross35 · 2025-04-15T00:07:23Z

@rustbot author , @madhav-madhusoodanan left some good review here.

Add unstable attribute Add unstable attribute for `from_nanos_u128` with tracking issue rust-lang#139201 Co-authored-by: Madhav Madhusoodanan <[email protected]> add feature gate for the new function - add feature gate - remove trailing whitespace Add feature gate at the right place Co-authored-by: Madhav Madhusoodanan <[email protected]> Update time.rs making sure seconds are in u64 Co-authored-by: Madhav Madhusoodanan <[email protected]> Update time.rs keeping seconds as u64 removing the use of non const function within const function Use a u128 example time.rs Earlier for the example I used 2.pow(64). correcting it to use u128 type which can contain the value Synched my fork Dummy commit dummy commit. sync local with fork style: format code with rustfmt Add panic test for Duration::from_nanos_u128 fix: remove non-const .into() call from const function A const function cannot call non-const functions, so we remove the trailing .into() call and use raw literals instead. remove paratheses Add comment on largest input and explain safety assumption Ran Rust fmt again

git checkout 095486e -- Cargo.lock

omanirudh · 2025-04-21T08:12:54Z

@rustbot ready

omanirudh · 2025-04-21T15:13:20Z

About the commit history (this is completely upto you)

You can use git rebase -i HEAD~<number of commits>. This opens up an interactive window using which you can reorganize and meld related commits together.

This is a related thread on zulip on the concerns behind squashing vs merging commits.

I looked into it. This PR now has just one commit. I preserved the commit messages of earlier commits though.

tgross35

Implementation and docs look good to me, left a few small comments about adding backticks around code types. However, this needs some tests. Could you update library/coretests/tests/time.rs to include from_nanos_u128 everywhere that from_nanos is tested? Also make sure some large values are tested, and one or two #[should_panic] tests for the overflow.

tgross35 · 2025-04-26T23:32:10Z

library/core/src/time.rs

@@ -308,6 +307,39 @@ impl Duration {
        Duration { secs, nanos: subsec_nanos }
    }

+    /// Creates a new Duration from the specified number of nanoseconds.


Suggested change

/// Creates a new Duration from the specified number of nanoseconds.

/// Creates a new `Duration` from the specified number of nanoseconds.

tgross35 · 2025-04-26T23:32:23Z

library/core/src/time.rs

+    ///
+    /// # Panics
+    ///
+    /// Panics if the given number of nanoseconds is greater than what Duration can handle,


Suggested change

/// Panics if the given number of nanoseconds is greater than what Duration can handle,

/// Panics if the given number of nanoseconds is greater than what `Duration` can handle,

tgross35 · 2025-04-26T23:32:32Z

library/core/src/time.rs

+    ///
+    /// Panics if the given number of nanoseconds is greater than what Duration can handle,
+    /// which is `(u64::MAX * NANOS_PER_SEC) + NANOS_PER_SEC - 1`
+    /// Use this function if you need to specify time greater than what can fit in u64


Suggested change

/// Use this function if you need to specify time greater than what can fit in u64

/// Use this function if you need to specify time greater than what can fit in `u64`

tgross35 · 2025-04-26T23:44:55Z

library/core/src/time.rs

@@ -1,5 +1,4 @@
 #![stable(feature = "duration_core", since = "1.25.0")]
-


Looks like a spurious removal

tgross35 · 2025-04-27T00:10:24Z

I looked into it. This PR now has just one commit. I preserved the commit messages of earlier commits though.

Also fyi - pedantically, there is no need to preserve the messages unless they are still meaningful. It's best if the message describes the contained change, it's not really useful to rust's forever history that you had "dummy commit" and "remove parentheses" at some point :)

omanirudh · 2025-04-27T08:04:02Z

I looked into it. This PR now has just one commit. I preserved the commit messages of earlier commits though.

Also fyi - pedantically, there is no need to preserve the messages unless they are still meaningful. It's best if the message describes the contained change, it's not really useful to rust's forever history that you had "dummy commit" and "remove parentheses" at some point :)

noted.

rustbot assigned tgross35 Apr 2, 2025

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 Apr 2, 2025

rustbot assigned RalfJung and unassigned tgross35 Apr 2, 2025