Stream: general

Topic: Maximum duration


marmeladema (May 21 2020 at 11:23, on Zulip):

Hello! Whats the best way to construct the biggest value a Duration can hold? Using Duration::new(u64::MAX, u32::MAX) will panic because of seconds overflow.

marmeladema (May 21 2020 at 11:24, on Zulip):

I understand the panic, but it seems there is no easy way to build such Duration::MAX value

Laurențiu Nicola (May 21 2020 at 11:25, on Zulip):

https://doc.rust-lang.org/src/core/time.rs.html#562-579 hmm

Laurențiu Nicola (May 21 2020 at 11:31, on Zulip):

Duration::new(u64::MAX, 1_000_000_000 - 1);

marmeladema (May 21 2020 at 11:32, on Zulip):

Duration::new(u64::MAX, NANOS_PER_SEC-1) seems to work but unfortunately i had to redefine NANOS_PER_SEC myself

marmeladema (May 21 2020 at 11:32, on Zulip):

yeah right we came up with the same answer

marmeladema (May 21 2020 at 11:33, on Zulip):

Would it be acceptable to have NANOS_PER_SEC (and probably the other related constants) as associated constants for Duration?

marmeladema (May 21 2020 at 11:34, on Zulip):

or just exported in the time module

Laurențiu Nicola (May 21 2020 at 11:37, on Zulip):

Also https://github.com/rust-lang/rust/issues/32070

marmeladema (May 21 2020 at 11:40, on Zulip):

Apparently i am not the only trying to build such maximum duration

Laurențiu Nicola (May 21 2020 at 11:50, on Zulip):

Duration is a bit of a mess :-(

marmeladema (May 21 2020 at 11:52, on Zulip):

I'll do a PR that adds Duration:::MAX and we will see what happens :D

Laurențiu Nicola (May 21 2020 at 11:54, on Zulip):

Unfortunately, Duration::new is not const

RalfJ (May 21 2020 at 11:57, on Zulip):

Laurențiu Nicola said:

Unfortunately, Duration::new is not const

hm, as a start someone could experiment with making the checked_* arithmetic methods const fn

RalfJ (May 21 2020 at 11:57, on Zulip):

that requires const_if_match but that's okay for unstable methods (and that feature will hopefully become stable Soon(TM) )

marmeladema (May 21 2020 at 13:34, on Zulip):

@RalfJ i'd happy to help! What is required exactly to make checked_* function const? Only adding the feature gate + adding the const keyword?

RalfJ (May 21 2020 at 13:38, on Zulip):

well... I don't know for sure, I am only following constification at a distance

RalfJ (May 21 2020 at 13:38, on Zulip):

so I'd start by adding const fn and #[rustc_const_unstable]

RalfJ (May 21 2020 at 13:38, on Zulip):

and then let the compiler errors guide me

RalfJ (May 21 2020 at 13:40, on Zulip):

I dont know what our current story is for "internal instability" but that only really matters once we try to stabilize them -- so, not for now.

RalfJ (May 21 2020 at 13:41, on Zulip):

@marmeladema ^

marmeladema (May 21 2020 at 13:42, on Zulip):

Ok, i'll try to do something! Worst case scenario, reviewer screams at me :D

LeSeulArtichaut (May 21 2020 at 13:43, on Zulip):

Worst case scenario, reviewer screams at me

Isn't that also the best case scenario? :big_smile:

RalfJ (May 21 2020 at 13:46, on Zulip):

our reviewers should be more kind than that ;)

RalfJ (May 21 2020 at 13:46, on Zulip):

oh and make sure to also add a testcase, otherwise the reviewer will definitely demand that :D

RalfJ (May 21 2020 at 13:47, on Zulip):

we already have a bunch of const-int-arith test file in src/test/ui/consts, see if there's a good file to add these two or if it seems more sensible to add a new file

LeSeulArtichaut (May 21 2020 at 13:52, on Zulip):

RalfJ said:

our reviewers should be more kind than that ;)

Just kidding :D

marmeladema (May 21 2020 at 18:38, on Zulip):

So checked_add is already const apparently

marmeladema (May 21 2020 at 18:40, on Zulip):

Duration::new cannot be made const because of the call to .expect(..)

marmeladema (May 21 2020 at 18:53, on Zulip):

Matching and calling panic manually seems to do the trick BUT new method is already tagged with #[rustc_const_stable(feature = "duration_consts", since = "1.32.0")] which means i cannot just add #[rustc_const_unstable(feature = "const_checked_int_methods", issue = "53718")]

RalfJ (May 21 2020 at 19:12, on Zulip):

marmeladema said:

So checked_add is already const apparently

doesn't look like it: https://doc.rust-lang.org/nightly/std/primitive.i64.html#method.checked_add

RalfJ (May 21 2020 at 19:12, on Zulip):

at last I think that would say const explicitly?

RalfJ (May 21 2020 at 19:12, on Zulip):

yes it would

RalfJ (May 21 2020 at 19:13, on Zulip):

new method is already tagged with #[rustc_const_stable(feature = "duration_consts", since = "1.32.0")]

only const methods should be tagged with that...? which new method do you mean?

Laurențiu Nicola (May 21 2020 at 19:13, on Zulip):

Duration::new

Laurențiu Nicola (May 21 2020 at 19:14, on Zulip):

https://doc.rust-lang.org/src/core/time.rs.html#134

lcnr (May 21 2020 at 19:15, on Zulip):

Afaik these attributes don't actually do anything if the given function is not const

lcnr (May 21 2020 at 19:15, on Zulip):

There was some issue on github for that

RalfJ (May 21 2020 at 19:15, on Zulip):

that makes no sense... @oli any idea why it should have that attribute?

lcnr (May 21 2020 at 19:17, on Zulip):

It's https://github.com/rust-lang/rust/issues/69630

RalfJ (May 21 2020 at 19:18, on Zulip):

well for now let's clean this one up: https://github.com/rust-lang/rust/pull/72434

RalfJ (May 21 2020 at 19:19, on Zulip):

@marmeladema I'll close that PR if you have one that makes it actually const. (unstably).. feel free to just remove the existing attribute if it gets in your way.

RalfJ (May 21 2020 at 19:19, on Zulip):

and yes you'll have to make Option::except unstably const as well

marmeladema (May 21 2020 at 19:21, on Zulip):

https://github.com/rust-lang/rust/blob/master/src/libcore/num/mod.rs#L746
this is the checked_add implem that I looked at

RalfJ (May 21 2020 at 19:22, on Zulip):

hm... maybe rustdoc does not render the const when it is unstable

RalfJ (May 21 2020 at 19:22, on Zulip):

well that means less work for you then, nice :D

marmeladema (May 21 2020 at 19:23, on Zulip):

https://github.com/rust-lang/rust/compare/master...marmeladema:duration-new-const?expand=1
this is what i tried and the compiler does not seem to complain

RalfJ (May 21 2020 at 19:34, on Zulip):

sure that also works

RalfJ (May 21 2020 at 19:35, on Zulip):

though the constant MAX you added seems orthogonal, and should be a separate feature IMO

RalfJ (May 21 2020 at 19:35, on Zulip):

oh you made them separate :)

RalfJ (May 21 2020 at 19:35, on Zulip):

hm re-using "const_checked_int_methods" for Duration::new is a bit fishy though

marmeladema (May 21 2020 at 19:40, on Zulip):

I open-ed https://github.com/rust-lang/rust/pull/72436 for the sake of it

marmeladema (May 21 2020 at 19:41, on Zulip):

i've never modified any user visible code from standard library so it might be totally none sense^^

marmeladema (May 21 2020 at 19:52, on Zulip):

I'd also like to add Duration::saturating_add and Duration::saturating_sub

RalfJ (May 21 2020 at 19:59, on Zulip):

@marmeladema as mentioned above, usually new const fn come with a UI test ensuring their correct functionality

marmeladema (May 21 2020 at 20:00, on Zulip):

Oh yeah right sorry, i'll add one later tonight

LeSeulArtichaut (May 21 2020 at 20:32, on Zulip):

"tonight" :eyes:

marmeladema (May 21 2020 at 21:57, on Zulip):

So i ran a bit more experiments, and most of non-trait Duration methods can be made const

marmeladema (May 21 2020 at 22:04, on Zulip):

I just have a question about feature and unstability. Why would it be wrong it use an already existing feature like #[rustc_const_stable(feature = "duration_consts", since = "1.45.0")] (using the version its going to be released of course) to get instantaneous stable const methods?

marmeladema (May 21 2020 at 22:31, on Zulip):

Its a bit stressful to create a 'tracking issue', it feels like a lot of responsibility now but here we go: https://github.com/rust-lang/rust/issues/72440
I hope i did not make such a bad job at it

simulacrum (May 21 2020 at 22:55, on Zulip):

that looks pretty good!

RalfJ (May 22 2020 at 20:00, on Zulip):

marmeladema said:

Its a bit stressful to create a 'tracking issue', it feels like a lot of responsibility now but here we go: https://github.com/rust-lang/rust/issues/72440
I hope i did not make such a bad job at it

yeah I know it can feel intimidating. alternatively you can always ask a team member to do it for you. :)

RalfJ (May 22 2020 at 20:00, on Zulip):

but to tell you the truth, I am also somewhat intimidated by it. :D you did a great job!

marmeladema (May 22 2020 at 21:43, on Zulip):

Thanks :) i am writing tests right now and will open a PR soon

Last update: May 29 2020 at 18:00UTC