Make remaining methods const

[pitdicker]

Depends on #1137.

This finishes the work to make all methods const where possible.
Where it is not possible is parsing and formatting, and anything that involves traits including DateTime<tz>.

[codecov]

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (e031ffb) 91.84% compared to head (addcf66) 91.84%.

Files	Patch %	Lines
src/naive/datetime/mod.rs	68.75%	5 Missing ⚠️
src/duration.rs	98.94%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1337   +/-   ##
=======================================
  Coverage   91.84%   91.84%           
=======================================
  Files          38       38           
  Lines       17491    17519   +28     
=======================================
+ Hits        16064    16090   +26     
- Misses       1427     1429    +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jtmoon79]

nit: typo wihtin

Same typo on other lines

[pitdicker]

Thank you, fixed.

[jtmoon79]

There appear to be significant logic changes to overflowing_add_signed. Currently, test_time_overflowing_add does not have any asserts involving fractional seconds.
https://github.com/chronotope/chrono/blob/v0.4.31/src/naive/time/tests.rs#L117

I strongly recommend adding more asserts to test_time_overflowing_add specifically targeting this change in logic.

[pitdicker]

There appear to be significant logic changes to overflowing_add_signed.

True, this method was pretty convoluted because it comes from a time before rem_euclid, like we had in more places in chrono.

This code is extremely well-tested, better than any other part of chrono. The tests just work on the higher-level methods that wrap test_time_overflowing_add.

[pitdicker]

I hoped to make this the last PR before suggesting a 0.4.32 release. And then 4 months evaporated 🙄.
Took care of the merge conflicts.

[pitdicker]

Updated. Now that our MSRV is 1.61 it is possible to also make Duration::to_std const.

[pitdicker]

I'll wait with merging until after #1385, to make this the one that has to rebase.

[danwilliams]

@pitdicker I'm concerned about some of the changes brought in here. I like the new Duration::new() function - that's a great addition! - but I'm seeing what I perceive as an increased brittleness in the code through direct checks on hard-coded values such as -i64::MAX instead of their representatives.

I can understand why this is, but I feel it is a wrong path.

Over in #1392 we have discussed reworking some of the constants - it seems to me that this would be a good opportunity to correct some of these patterns at the same time. What do you think? 🙂 Would you be open to me submitting a PR that not only streamlines the constants but also ensures they are used in the various checks that are currently diverse?

[pitdicker]

You are right. It would especially be nice to replace the -i64::MAX with a proper constant. Unfortunately I also did it in one place of your new code to make the function const (CI was instantly red after my naive rebase). I'll look forward to what you come up with 👍.

For now I'm glad to have the changes to make the Duration methods const land. It was based on #638 and #1137.

[Skgland]

The functions documentation says >= 1_000_000_000, but here > 1_000_000_000 is used in the check?

[pitdicker]

Thank you for catching that!

[pitdicker]

Would you be open to me submitting a PR that not only streamlines the constants but also ensures they are used in the various checks that are currently diverse?

@danwilliams Keep an eye on #1351 that is also touching constants in the duration module.