This comment has been hidden.

This comment has been hidden.

Are there plans for stabilization?

I had these two traits in my code for over a year, and they now show the "unstable_name_collisions" lint every build, everywhere they're used - #[allow(unstable_name_collisions)] does not work well with a workspace, I have to put it in all modules or rename the functions :/

/// Extensions for `Duration`
pub trait DurationExt : Copy {
    /// Returns true if self is equivalent to zero
    fn is_zero(self) -> bool;

    /// Get a zero duration
    fn zero() -> Duration {
        Duration::new(0, 0)
    }
}

impl DurationExt for Duration {
    #[inline]
    fn is_zero(self) -> bool {
        self.as_secs() == 0 && self.subsec_nanos() == 0
    }
}

jonhoo commented on Sep 11, 2020

My guess is that these can probably be stabilized as-is?

LukasKalbertodt commented on Sep 13, 2020

As the top comments says, the only open question is basically whether we want the associated const ZERO instead of the two methods.

// Do we want this?
let d = Duration::zero();
if d.is_zero() { ... }

// Or rather this?
let d = Duration::ZERO;
if d == Duration::ZERO { ... }

There are certainly some uses of associated consts in std already:

• Ip4Addr::{LOCALHOST, BROADCAST, UNSPECIFIED}. Ip4Addr also has plenty of is_* methods, including is_broadcast. It does not have localhost, .. constructors functions.
• Integers and floats have associated consts and no constructor functions.

I think both of those two solutions (or any mix of those) would be fine. I don't think anyone would consider either of those a "bad API". So if you think that either one of those should be stabilized, go ahead and either open a PR that adds the const ZERO or a PR that stabilizes the existing methods. In either case, provide some reasoning why the API you're suggesting is good/better.

I was thinking about the pros and cons, and I'm really divided, though leaning slightly towards ZERO

Duration::zero()

• Intuitive (demonstrated by my independently invented trait with the same methods)
• Might be more obvious to beginners who don't know that Duration is Copy - it looks like a constructor
• Can be used in places that accept Fn() -> Duration - I'm not sure how useful that really is
• Optimized to a constant = no overhead

Duration::ZERO

• Cheap and straightforward, it is a constant
• Name clearly indicates it's a constant
• A bit shorter
• Has a precedent in Ip4Addr

Regardless, is_zero() is a useful addition - although you can use d == Duration::ZERO, calling a method is much more succinct and makes for cleaner code

Rua commented on Sep 30, 2020

My preference is for the constant. Places that need a function can easily be given || Duration::ZERO, which doesn't seem like much of a bother.

workingjubilee commented on Oct 27, 2020

As of #78216, Nightly Rust moves to the constant for a variety of reasons stated here, in addition to the fact that a const does not require a const evaluation context in order to be used in places where consts but not dynamic values are accepted... this matters a lot for match contexts and the many sugared versions of match.

TehPers commented on Mar 11

Are there any open questions remaining, or does this just need a stabilization PR now?

m-ou-se commented 11 days ago

@rfcbot merge

rfcbot commented 11 days ago •

Team member @m-ou-se has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

rfcbot commented 11 days ago

This is now entering its final comment period, as per the review above.

workingjubilee commented 2 days ago •

Well, I suppose it is a final comment period:

I do not particularly feel that is_zero is necessary, because most numeric types do not have is_zero methods either, and broadly I would not want them to acquire such. In #78216 I omitted removal because there was at least one request in favor. Nonetheless, I don't particularly stand by it: even if one specifically wanted a method, it seems like unnecessary duplication next to

use std::time::Duration;
use std::cmp::PartialEq;
use fun_library::some_duration;

fn main() {
    let d = some_duration();
    if d.eq(&Duration::ZERO) {
        println!("what could be less than an instant?");
    }
}

But neither am I strongly against if it is felt that there is something different about Duration that justifies such. That said, every additional line in documentation makes it harder to find other lines in documentation, so I still feel it should justify itself.

BurntSushi commented yesterday

@workingjubilee I guess I don't really see Duration as a strictly numeric type. It has a bit of a higher level meaning than that. So I don't think the precedent with integer types is precisely relevant here.

rfcbot commented 21 hours ago

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

workingjubilee commented 17 hours ago

@workingjubilee I guess I don't really see Duration as a strictly numeric type. It has a bit of a higher level meaning than that. So I don't think the precedent with integer types is precisely relevant here.

That seems fair, then, if the precedent is not considered to hold in either direction.

m-ou-se commented 8 hours ago

Without .is_zero(), you'd have to import Duration or write == std::time::Duration::ZERO, which is a bit much for such a simple operation. And we already have things like is_empty() on containers and quite a few more is_..() functions like that.