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

Make Duration::seconds (and siblings) const fn #309

Closed
axos88 opened this issue Mar 17, 2019 · 13 comments
Closed

Make Duration::seconds (and siblings) const fn #309

axos88 opened this issue Mar 17, 2019 · 13 comments

Comments

@axos88
Copy link

axos88 commented Mar 17, 2019

This would allow to specify things like:

const THRESHOLD: Duration = Duration::seconds(3);

@axos88 axos88 changed the title Make Duration::seconds (and siblings) const Make Duration::seconds (and siblings) const fn Mar 17, 2019
@bspeice
Copy link

bspeice commented Mar 18, 2019

I don't think this is possible in current Rust - const fn isn't allowed to use branches as of Rust 1.33.

@quodlibetor
Copy link
Contributor

Yes, that's what I'm currently holding off on. Even once const fn is stable enough for chrono we'll need to think about how best to support them in a way that doesn't break backcompat too terribly. Chrono currently supports rust 1.13, but const fn is the strongest reason to upgrade to a new version that is upcoming.

Obviously we'll update, but I'd like to keep things as compatible as possible, if that means adding some features or something else so that we can still work on old rustcs, I'm not sure.

@hjmallon
Copy link

hjmallon commented Sep 3, 2019

const fn on NaiveDate and friends would also be useful.

@quodlibetor
Copy link
Contributor

If anyone wants to create a PR for this behind something like an unstable-nightly-const feature gate I would be happy to take it. It should just be a matter of duplicating some functions and adding some cfg and cfg(not) attributes.

@chrismooredev
Copy link

For the googlers that came here like me, it looks like the current blocker on this issue are const-panics (RFC) (issue) that are triggered when doing the .expect/panic-based bounds checking on the various Duration constructors.

Perhaps we can introduce variants that follow stdlib functions like checked_{mul,div,...}, and return an Option<Duration>, with None meaning out-of-bounds, just like checked_mul.

Since Duration is re-exported from the time crate, shouldn't this really be an issue there?

@zertyz
Copy link

zertyz commented Feb 18, 2021

Still not good to go on with that change?
I'd like to vote for it, because Duration constants make much sense, imho.
I actually had to quit using Duration for that and resort to ints (with a particular unit attached to their names), having to create a Duration variable every time...
Do you, authors, tought of a better way to address this scenario than to use constants?
Found it nowhere in the docs...

@robjtede
Copy link

robjtede commented Mar 17, 2021

With https://github.com/dtolnay/rustversion, there's a specific feature for making functions const conditionally on compiler versions since X. I'd argue this is a big win for a very cheap (no syn) extra dep.

Edit: also found https://github.com/taiki-e/const_fn which is more specific macro and used by time 0.2 crate.

@msdrigg
Copy link
Contributor

msdrigg commented Dec 22, 2021

We now have const panics in rust 1.57 would it be possible to move this issue forward?

@hoon-prestolabs
Copy link

I'm waiting for this. It seems this can be reviewed relatively easily. Can we merge the @msdrigg 's PR? Or are there other concerns?

@bestouff
Copy link

@quodlibetor does your offer still stand ?

@djc
Copy link
Member

djc commented Mar 28, 2022

I'm open to merging something like this, provided it is made conditional on an opt-in feature flag (so that people who don't need this can still depend on the currently 1.32 MSRV).

@pitdicker
Copy link
Collaborator

Fixed in #1337.

@bestouff
Copy link

bestouff commented Feb 2, 2024

Thanks !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.