Add SystemTime::{MIN, MAX} #148825
Add SystemTime::{MIN, MAX} #148825
Conversation
rustbot
added
O-hermit
|
For what it is worth: Right now, the Rust ecosystem does weird stuff in order to figure out a limit.
|
cvengler
force-pushed
the
time_systemtime_limits
branch
from
143f4ce to
481fe49
Compare
This comment has been minimized.
This comment has been minimized.
|
I'm pretty sure, though I could be wrong that this will need an ACP first before any work |
cvengler
force-pushed
the
time_systemtime_limits
branch
from
481fe49 to
3ef3955
Compare
rustbot
added
the
S-waiting-on-review
rustbot
removed
the
S-waiting-on-author
|
r? @ChrisDenton rustbot has assigned @ChrisDenton. Use |
This comment has been minimized.
This comment has been minimized.
|
Marking this as ready for review, given that CI works now. 🎉 |
|
This will indeed require an ACP before this can be merged. |
cvengler
force-pushed
the
time_systemtime_limits
branch
from
3ef3955 to
e5745aa
Compare
Kivooeo
added
S-blocked
|
I block this until ACP will accepted |
|
Alright, we already have a draft of an ACP in our pipeline. |
|
@bors r+ |
bors
added
S-waiting-on-bors
…=ChrisDenton
Add SystemTime::{MIN, MAX}
Accepted ACP: <rust-lang/libs-team#692>
Tracking Issue: <rust-lang#149067>
---
This merge request introduces two new constants to `SystemTime`: `MIN` and `MAX`, whose values represent the maximum values for the respective data type, depending upon the platform.
Technically, this value is already obtainable during runtime with the following algorithm:
Use `SystemTime::UNIX_EPOCH` and call `checked_add` (or `checked_sub`) repeatedly with `Duration::new(0, 1)` on it, until it returns None.
Mathematically speaking, this algorithm will terminate after a finite amount of steps, yet it is impractical to run it, as it takes practically forever.
Besides, this commit also adds a unit test to verify those values represent the respective minimum and maximum, by letting a `checked_add` and `checked_sub` on it fail.
In the future, the hope of the authors lies within the creation of a `SystemTime::saturating_add` and `SystemTime::saturating_sub`, similar to the functions already present in `std::time::Duration`.
However, for those, these constants are crucially required, thereby this should be seen as the initial step towards this direction.
With this change, implementing these functions oneself outside the standard library becomes feasible in a portable manner for the first time.
This feature (and a related saturating version of `checked_{add, sub}` has been requested multiple times over the course of the past few years, most notably:
* rust-lang#100141
* rust-lang#133525
* rust-lang#105762
* rust-lang#71224
* rust-lang#45448
* rust-lang#52555
…=ChrisDenton
Add SystemTime::{MIN, MAX}
Accepted ACP: <rust-lang/libs-team#692>
Tracking Issue: <rust-lang#149067>
---
This merge request introduces two new constants to `SystemTime`: `MIN` and `MAX`, whose values represent the maximum values for the respective data type, depending upon the platform.
Technically, this value is already obtainable during runtime with the following algorithm:
Use `SystemTime::UNIX_EPOCH` and call `checked_add` (or `checked_sub`) repeatedly with `Duration::new(0, 1)` on it, until it returns None.
Mathematically speaking, this algorithm will terminate after a finite amount of steps, yet it is impractical to run it, as it takes practically forever.
Besides, this commit also adds a unit test to verify those values represent the respective minimum and maximum, by letting a `checked_add` and `checked_sub` on it fail.
In the future, the hope of the authors lies within the creation of a `SystemTime::saturating_add` and `SystemTime::saturating_sub`, similar to the functions already present in `std::time::Duration`.
However, for those, these constants are crucially required, thereby this should be seen as the initial step towards this direction.
With this change, implementing these functions oneself outside the standard library becomes feasible in a portable manner for the first time.
This feature (and a related saturating version of `checked_{add, sub}` has been requested multiple times over the course of the past few years, most notably:
* rust-lang#100141
* rust-lang#133525
* rust-lang#105762
* rust-lang#71224
* rust-lang#45448
* rust-lang#52555
Rollup of 11 pull requests Successful merges: - #145278 (Update `rustc_codegen_gcc` rotate operation document) - #148825 (Add SystemTime::{MIN, MAX}) - #148837 (Use `let...else` instead of `match foo { ... _ => return };` and `if let ... else return`) - #149177 (Add proper suggestion for associated function with unknown field) - #149843 (Inherit attributes in delegation) - #149860 (Fix: Prevent macro-expanded extern crates from shadowing extern arguments) - #149874 (Weak for Arc pointer is marked as DynSend/DynSync) - #149903 (Remove unused code in `cfg_old`) - #149911 (bootstrap: Don't pass an unused `--color` to compiletest) - #149916 (Add a sanity check in case of any duplicate nodes) - #149924 (`declare_lint_pass` for `INLINE_ALWAYS_MISMATCHING_TARGET_FEATURES`) r? `@ghost` `@rustbot` modify labels: rollup
bors
added
S-waiting-on-author
|
Hm, there are two issues here: Firstly there's a pre-existing bug in the Windows implementation of The second problem is with the new test. Windows |
Thanks, that was very fast!
Good catch, will fix it.
|
Ah, that does indeed sound good. |
I pushed two commits. I hope this PR is the correct place for this? Alternatively, I would hard reset this to ac5c70a and open a new PR with those changes, setting this PR to "stale" before the other one is merged, then rebasing onto it. |
This comment has been minimized.
This comment has been minimized.
cvengler
force-pushed
the
time_systemtime_limits
branch
2 times, most recently
from
168b257 to
b02ab4c
Compare
There is a slight edge case when adding and subtracting a `Duration` from a `SystemTime`, namely when the duration itself is finer/smaller than the time precision on the operating systems. On most (if not all non-Windows) operating systems, the precision of `Duration` aligns with the `SystemTime`, both being one nanosecond. However, on Windows, this time precision is 100ns, meaning that adding or subtracting a `Duration` whose value is `< Duration::new(0, 100)` will result in that method behaving like an addition/subtracting of `Duration::ZERO`, due to the `Duration` getting rounded-down to the zero value.
cvengler
force-pushed
the
time_systemtime_limits
branch
from
b02ab4c to
8decbdf
Compare
This comment has been minimized.
This comment has been minimized.
The Windows implementation of `SystemTime::checked_sub` contains a bug, namely that it does not return `None` on values below 1601. This bug stems from the fact that internally, the time gets converted to an i64, with zero representing the anchor in 1601. Of course, performing checked subtraction on a signed integer generally works fine. However, the resulting value delivers undefined behavior on Windows systems. To mitigate this issue, we try to convert the resulting i64 to an u64 because a negative value should obviously fail there.
cvengler
force-pushed
the
time_systemtime_limits
branch
from
8decbdf to
d80348b
Compare
|
This seems to be building again. 🎉 |
|
@bors r+ |
bors
added
S-waiting-on-bors
12 participants
Accepted ACP: rust-lang/libs-team#692
Tracking Issue: #149067
This merge request introduces two new constants to
SystemTime:MINandMAX, whose values represent the maximum values for the respective data type, depending upon the platform.Technically, this value is already obtainable during runtime with the following algorithm:
Use
SystemTime::UNIX_EPOCHand callchecked_add(orchecked_sub) repeatedly withDuration::new(0, 1)on it, until it returns None.Mathematically speaking, this algorithm will terminate after a finite amount of steps, yet it is impractical to run it, as it takes practically forever.
Besides, this commit also adds a unit test to verify those values represent the respective minimum and maximum, by letting a
checked_addandchecked_subon it fail.In the future, the hope of the authors lies within the creation of a
SystemTime::saturating_addandSystemTime::saturating_sub, similar to the functions already present instd::time::Duration.However, for those, these constants are crucially required, thereby this should be seen as the initial step towards this direction.
With this change, implementing these functions oneself outside the standard library becomes feasible in a portable manner for the first time.
This feature (and a related saturating version of
checked_{add, sub}has been requested multiple times over the course of the past few years, most notably:std::time::Instant::saturating_duration_since()? #133525