From a6bcd5250285e170160eb3e6312c37bf9caa4932 Mon Sep 17 00:00:00 2001 From: InTVyWOC <56090912+InTVyWOC@users.noreply.github.com> Date: Tue, 12 Nov 2019 10:41:06 +0700 Subject: [PATCH 1/4] Create 0000-duration-is-zero.md --- text/0000-duration-is-zero.md | 121 ++++++++++++++++++++++++++++++++++ 1 file changed, 121 insertions(+) create mode 100644 text/0000-duration-is-zero.md diff --git a/text/0000-duration-is-zero.md b/text/0000-duration-is-zero.md new file mode 100644 index 00000000000..6e16d0a9766 --- /dev/null +++ b/text/0000-duration-is-zero.md @@ -0,0 +1,121 @@ +- Feature Name: `Duration::is_zero()` +- Start Date: 2019-11-12 +- RFC PR: N/A +- RFC Issue: [rust-lang/rfcs#2809](https://github.com/rust-lang/rfcs/issues/2809) + +# Summary +[summary]: #summary + +Adding new function `is_zero(&self) -> bool` to `core::time::Duration`. + +# Motivation +[motivation]: #motivation + +- Why are we doing this? + + Personally I found many times the need to compare a duration to zero. What I would do is: + + ```rust + if some_duration == Duration::from_secs(0) { + ... + } + ``` + +- What use cases does it support? + + It can be used to check if a duration is zero. + +- What is the expected outcome? + + It can help eliminate workaround such as above. + +# Guide-level explanation +[guide-level-explanation]: #guide-level-explanation + +- Function name: `is_zero()` + + A duration holds a point related to timestamp. When we work with durations, we often think of: + + + 2 seconds + + 30 seconds + + 3 hours + + So using `is_zero()` is appropriate. + +- Example usage: + + + `std::net::TcpStream::connect_timeout()` takes a duration as timeout. The documentation says: _It is an + error to pass a zero `Duration` to this function._ This is a good example of the proposal function + `is_zero()`. + + + `std::thread::sleep()` takes a duration. A zero duration does not help much in this case. If one program + allows the user to set some delay time between jobs via command line, it needs to verify that a duration + is not zero. + +# Reference-level explanation +[reference-level-explanation]: #reference-level-explanation + +Implementing this is quite simple. In Rust `1.39.0`, there are only 2 internal fields `secs` and `nanos`. So +an implementation might look like this: + +```rust +impl Duration { + + /// # Checks if this duration is zero + pub fn is_zero(&self) -> bool { + self.secs == 0 && self.nanos == 0 + } + +} +``` + +I think there are no corner cases. + +# Drawbacks +[drawbacks]: #drawbacks + +I could not think of any drawbacks. + +# Rationale and alternatives +[rationale-and-alternatives]: #rationale-and-alternatives + +- First alternative: + + In (issue #2809)(https://github.com/rust-lang/rfcs/issues/2809), one developer suggested new constant in + `core::time` module: + + ```rust + pub const ZERO: Duration = ... + ``` + + Some drawbacks I think of are: + + + Developers need to import the constant or the `time` module (for usage like + `if some_duration == time::ZERO`). + + Importing the constant might introduce new conflicting name to developer's existing code, which might not + be their desire. + + Performance: I have no knowledge about compiler. But I _guess_ comparing 2 instances of `Duration` + generates more code than having one instance checking its internal fields. If it's true, we should prefer + `is_zero()` instead of this constant. + +- Second alternative: + + Using `is_empty()` instead of `is_zero()`. + + I think `is_empty()` is related to collections (string, vector, set, map, slice...) However a duration is + not like a collection. So this name is not appropriate. + +# Prior art +[prior-art]: #prior-art + +I have no examples of prior arts. + +# Unresolved questions +[unresolved-questions]: #unresolved-questions + +I could not think of any unresolved questions. + +# Future possibilities +[future-possibilities]: #future-possibilities + +I could not think of any future possibilities. From 7a90686d1295dc5fd8c740b33b909483f91c34c7 Mon Sep 17 00:00:00 2001 From: InTVyWOC <56090912+InTVyWOC@users.noreply.github.com> Date: Tue, 12 Nov 2019 10:42:53 +0700 Subject: [PATCH 2/4] Update 0000-duration-is-zero.md --- text/0000-duration-is-zero.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-duration-is-zero.md b/text/0000-duration-is-zero.md index 6e16d0a9766..54258b0c26b 100644 --- a/text/0000-duration-is-zero.md +++ b/text/0000-duration-is-zero.md @@ -81,7 +81,7 @@ I could not think of any drawbacks. - First alternative: - In (issue #2809)(https://github.com/rust-lang/rfcs/issues/2809), one developer suggested new constant in + In [issue #2809](https://github.com/rust-lang/rfcs/issues/2809), one developer suggested new constant in `core::time` module: ```rust From 255553c952746c56561a449fb93e94a14103471d Mon Sep 17 00:00:00 2001 From: InTVyWOC <56090912+InTVyWOC@users.noreply.github.com> Date: Tue, 12 Nov 2019 18:47:43 +0700 Subject: [PATCH 3/4] Added prior-art and some drawbacks to ZERO constant. --- text/0000-duration-is-zero.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/text/0000-duration-is-zero.md b/text/0000-duration-is-zero.md index 54258b0c26b..e04a211e15a 100644 --- a/text/0000-duration-is-zero.md +++ b/text/0000-duration-is-zero.md @@ -96,7 +96,9 @@ I could not think of any drawbacks. be their desire. + Performance: I have no knowledge about compiler. But I _guess_ comparing 2 instances of `Duration` generates more code than having one instance checking its internal fields. If it's true, we should prefer - `is_zero()` instead of this constant. + `is_zero()` over this constant. + + A `Duration` offers other functions such as `as_secs()`, `as_nanos()`... So using the constant only for + zero-verifying might not be useful. - Second alternative: @@ -108,7 +110,7 @@ I could not think of any drawbacks. # Prior art [prior-art]: #prior-art -I have no examples of prior arts. +- # Unresolved questions [unresolved-questions]: #unresolved-questions From ebcbb132060876744fc6324b49e988b42391441a Mon Sep 17 00:00:00 2001 From: InTVyWOC <56090912+InTVyWOC@users.noreply.github.com> Date: Thu, 14 Nov 2019 09:31:33 +0700 Subject: [PATCH 4/4] Update 0000-duration-is-zero.md - Added `zero()` as one alternative and future possibility. - Removed some drawbacks of `ZERO` alternative. --- text/0000-duration-is-zero.md | 59 +++++++++++++++++++++++++++++------ 1 file changed, 49 insertions(+), 10 deletions(-) diff --git a/text/0000-duration-is-zero.md b/text/0000-duration-is-zero.md index e04a211e15a..f7bea62bbfb 100644 --- a/text/0000-duration-is-zero.md +++ b/text/0000-duration-is-zero.md @@ -1,6 +1,6 @@ - Feature Name: `Duration::is_zero()` - Start Date: 2019-11-12 -- RFC PR: N/A +- RFC PR: Not available - RFC Issue: [rust-lang/rfcs#2809](https://github.com/rust-lang/rfcs/issues/2809) # Summary @@ -45,7 +45,7 @@ Adding new function `is_zero(&self) -> bool` to `core::time::Duration`. - Example usage: + `std::net::TcpStream::connect_timeout()` takes a duration as timeout. The documentation says: _It is an - error to pass a zero `Duration` to this function._ This is a good example of the proposal function + error to pass a zero `Duration` to this function._ This is a good example of the proposed function `is_zero()`. + `std::thread::sleep()` takes a duration. A zero duration does not help much in this case. If one program @@ -90,15 +90,11 @@ I could not think of any drawbacks. Some drawbacks I think of are: - + Developers need to import the constant or the `time` module (for usage like - `if some_duration == time::ZERO`). + + Developers need to import one more component: either the constant or the `time` module (for usage like + `if some_duration == time::ZERO`). While with `is_zero()`, they can just call it directly on any + `Duration` instance. + Importing the constant might introduce new conflicting name to developer's existing code, which might not be their desire. - + Performance: I have no knowledge about compiler. But I _guess_ comparing 2 instances of `Duration` - generates more code than having one instance checking its internal fields. If it's true, we should prefer - `is_zero()` over this constant. - + A `Duration` offers other functions such as `as_secs()`, `as_nanos()`... So using the constant only for - zero-verifying might not be useful. - Second alternative: @@ -107,6 +103,35 @@ I could not think of any drawbacks. I think `is_empty()` is related to collections (string, vector, set, map, slice...) However a duration is not like a collection. So this name is not appropriate. +- Third alternative: + + ```rust + impl Duration { + + /// Creates new zero duration + pub const fn zero() -> Self { + ... + } + + } + ``` + + This one was suggested by a developer, originally I didn't have this in mind. So from my view, I might not + see proper use cases of it. However for this RFC, I think it's less convenient than `is_zero()`. For example: + + ```rust + // It's longer if we type this + if some_duration == Duration::zero() { ... } + + // Or, we have to declare new (internal) constant, and we might need + // to repeat this for each independent project. + const ZERO: Duration = Duration::zero(); + if some_duration == ZERO { ... } + ``` + + But I think `zero()` might be useful in other situations. So I'll add this one to + [Future possibilities][future-possibilities] section. + # Prior art [prior-art]: #prior-art @@ -120,4 +145,18 @@ I could not think of any unresolved questions. # Future possibilities [future-possibilities]: #future-possibilities -I could not think of any future possibilities. +One future possibility is this: + +```rust +impl Duration { + + /// Creates new zero duration + pub const fn zero() -> Self { + ... + } + +} +``` + +It's one of above alternatives. From my view, I explained that it's less convenient than `is_zero()`. But I +think it might be useful for other developers.