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

Discrepancy between as_nanos API and from_nanos API #103332

Closed
mlej8 opened this issue Oct 21, 2022 · 2 comments
Closed

Discrepancy between as_nanos API and from_nanos API #103332

mlej8 opened this issue Oct 21, 2022 · 2 comments
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@mlej8
Copy link

mlej8 commented Oct 21, 2022

The as_nanos API in https://doc.rust-lang.org/src/core/time.rs.html#462 returns u128, however the from_nanos API https://doc.rust-lang.org/src/core/time.rs.html#276 only accepts u64. Can we make it u128 so that it is consistent ?

@Noratrieb
Copy link
Member

Sadly not, as that would be a breaking change.

melekes added a commit to webrtc-rs/webrtc that referenced this issue Nov 2, 2022
Refs rust-lang/rust#103332
If we don't want to lose precision here, we need to use nanos, but
currently because of API disrepancy it's not possible.
@jyn514
Copy link
Member

jyn514 commented Apr 9, 2023

I'm going to close this since we can't change the API here.

@jyn514 jyn514 closed this as not planned Won't fix, can't repro, duplicate, stale Apr 9, 2023
@jyn514 jyn514 added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Apr 9, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 11, 2024
…shtriplett

Suggest less bug-prone construction of Duration in docs

std::time::Duration has a well-known quirk: Duration::as_nanos() returns u128 [1], but Duration::from_nanos() takes u64 [2]. So these methods cannot easily roundtrip [3]. It is not possible to simply accept u128 in from_nanos [4], because it requires breaking other API [5].

It seems to me that callers have basically only two options:
1. `Duration::from_nanos(d.as_nanos() as u64)`, which is the "obvious" and buggy approach.
2. `Duration::new(d.as_secs(), d.subsecs_nanos())`, which only becomes apparent after reading and digesting the entire Duration struct documentation.

I suggest that the documentation of `from_nanos` is changed to make option 2 more easily discoverable.

There are two major usecases for this:
- "Weird math" operations that should not be supported directly by `Duration`, like squaring.
- "Disconnected roundtrips", where the u128 value is passed through various other stack frames, and perhaps reconstructed into a Duration on a different machine.

In both cases, it seems like a good idea to not tempt people into thinking "Eh, u64 is good enough, what could possibly go wrong!". That's why I want to add a note that points out the similarly-easy and *safe* way to reconstruct a Duration.

[1] https://doc.rust-lang.org/stable/std/time/struct.Duration.html#method.as_nanos
[2] https://doc.rust-lang.org/stable/std/time/struct.Duration.html#method.from_nanos
[3] https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=fa6bab2b6b72f20c14b5243610ea1dde
[4] rust-lang#103332
[5] rust-lang#51107 (comment)
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Feb 11, 2024
Rollup merge of rust-lang#119242 - BenWiederhake:dev-from-nanos, r=joshtriplett

Suggest less bug-prone construction of Duration in docs

std::time::Duration has a well-known quirk: Duration::as_nanos() returns u128 [1], but Duration::from_nanos() takes u64 [2]. So these methods cannot easily roundtrip [3]. It is not possible to simply accept u128 in from_nanos [4], because it requires breaking other API [5].

It seems to me that callers have basically only two options:
1. `Duration::from_nanos(d.as_nanos() as u64)`, which is the "obvious" and buggy approach.
2. `Duration::new(d.as_secs(), d.subsecs_nanos())`, which only becomes apparent after reading and digesting the entire Duration struct documentation.

I suggest that the documentation of `from_nanos` is changed to make option 2 more easily discoverable.

There are two major usecases for this:
- "Weird math" operations that should not be supported directly by `Duration`, like squaring.
- "Disconnected roundtrips", where the u128 value is passed through various other stack frames, and perhaps reconstructed into a Duration on a different machine.

In both cases, it seems like a good idea to not tempt people into thinking "Eh, u64 is good enough, what could possibly go wrong!". That's why I want to add a note that points out the similarly-easy and *safe* way to reconstruct a Duration.

[1] https://doc.rust-lang.org/stable/std/time/struct.Duration.html#method.as_nanos
[2] https://doc.rust-lang.org/stable/std/time/struct.Duration.html#method.from_nanos
[3] https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=fa6bab2b6b72f20c14b5243610ea1dde
[4] rust-lang#103332
[5] rust-lang#51107 (comment)
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Apr 7, 2024
Suggest less bug-prone construction of Duration in docs

std::time::Duration has a well-known quirk: Duration::as_nanos() returns u128 [1], but Duration::from_nanos() takes u64 [2]. So these methods cannot easily roundtrip [3]. It is not possible to simply accept u128 in from_nanos [4], because it requires breaking other API [5].

It seems to me that callers have basically only two options:
1. `Duration::from_nanos(d.as_nanos() as u64)`, which is the "obvious" and buggy approach.
2. `Duration::new(d.as_secs(), d.subsecs_nanos())`, which only becomes apparent after reading and digesting the entire Duration struct documentation.

I suggest that the documentation of `from_nanos` is changed to make option 2 more easily discoverable.

There are two major usecases for this:
- "Weird math" operations that should not be supported directly by `Duration`, like squaring.
- "Disconnected roundtrips", where the u128 value is passed through various other stack frames, and perhaps reconstructed into a Duration on a different machine.

In both cases, it seems like a good idea to not tempt people into thinking "Eh, u64 is good enough, what could possibly go wrong!". That's why I want to add a note that points out the similarly-easy and *safe* way to reconstruct a Duration.

[1] https://doc.rust-lang.org/stable/std/time/struct.Duration.html#method.as_nanos
[2] https://doc.rust-lang.org/stable/std/time/struct.Duration.html#method.from_nanos
[3] https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=fa6bab2b6b72f20c14b5243610ea1dde
[4] rust-lang/rust#103332
[5] rust-lang/rust#51107 (comment)
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
Suggest less bug-prone construction of Duration in docs

std::time::Duration has a well-known quirk: Duration::as_nanos() returns u128 [1], but Duration::from_nanos() takes u64 [2]. So these methods cannot easily roundtrip [3]. It is not possible to simply accept u128 in from_nanos [4], because it requires breaking other API [5].

It seems to me that callers have basically only two options:
1. `Duration::from_nanos(d.as_nanos() as u64)`, which is the "obvious" and buggy approach.
2. `Duration::new(d.as_secs(), d.subsecs_nanos())`, which only becomes apparent after reading and digesting the entire Duration struct documentation.

I suggest that the documentation of `from_nanos` is changed to make option 2 more easily discoverable.

There are two major usecases for this:
- "Weird math" operations that should not be supported directly by `Duration`, like squaring.
- "Disconnected roundtrips", where the u128 value is passed through various other stack frames, and perhaps reconstructed into a Duration on a different machine.

In both cases, it seems like a good idea to not tempt people into thinking "Eh, u64 is good enough, what could possibly go wrong!". That's why I want to add a note that points out the similarly-easy and *safe* way to reconstruct a Duration.

[1] https://doc.rust-lang.org/stable/std/time/struct.Duration.html#method.as_nanos
[2] https://doc.rust-lang.org/stable/std/time/struct.Duration.html#method.from_nanos
[3] https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=fa6bab2b6b72f20c14b5243610ea1dde
[4] rust-lang/rust#103332
[5] rust-lang/rust#51107 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

3 participants