Make functions in internals const #1043

pitdicker · 2023-04-29T15:32:32Z

@esheppa My intention was to understand why you would need to overhaul the internals of chono in #882 to make functions const. So I started sprinkling const around to see what breaks.

Everything in internals.rs can easily be made const, even on the 0.4.x branch. I made this PR because the work was done anyway, no use in letting someone else do it again.

@esheppa Would you be willing to review?
I also did a good number of the functions from NaiveDate just to prove for myself there are no major obstacles, but kept them out of this PR to keep it reviewable. There are a lot more choices that can be made there, so they deserve an own PR in the future.

I'll add some comments on the required changes.

pitdicker · 2023-04-29T15:34:11Z

src/naive/internals.rs

@@ -53,7  51,7 @@ pub(super) const FE: YearFlags = YearFlags(0o07);
 pub(super) const G: YearFlags = YearFlags(0o16);
 pub(super) const GF: YearFlags = YearFlags(0o06);

-static YEAR_TO_FLAGS: [YearFlags; 400] = [
+const YEAR_TO_FLAGS: &[YearFlags; 400] = &[


A static is only included once in the binary. To prevent duplication of this array on every use as const, It has to be put behind a reference (see https://users.rust-lang.org/t/const-vs-static/52951/12). It can still be duplicated with multiple codegen units, but the statics are not that big that it should be a problem.

src/naive/internals.rs

pitdicker · 2023-04-29T15:40:54Z

src/naive/internals.rs

        let mdl = mdf >> 3;
-        match MDL_TO_OL.get(mdl as usize) {
-            Some(&v) => Of(mdf.wrapping_sub((i32::from(v) as u32 & 0x3ff) << 3)),
-            None => Of(0),


These three lines where using array.get() and returning a safe value instead of panicking on out-of-bounds. Yet the index value is created in this module, and always in bounds.

I suspect the reason here is not just to avoid a potential panic, but to avoid the creation of landings pads if the compiler can't elide the bounds check.

In my opinion it would be fine to simplify this code to direct array indexing, but I kept the functionality. Note that array.get() is unavailable in const, so this needs a manual bounds check.

Ai. I was wrong, this code is just ancient. In various places it is returning 0 instead of Option, and you are supposed to call valid() afterwards 😞. Let me see if I can clean that up a little.

we may even have some correctness gains by separating these values out to separate modules, with the modules only exposing APIs that allow valid values (in cases where a const API still requires things like .valid())

pitdicker · 2023-04-29T15:41:58Z

src/naive/internals.rs

@@ -368,33  370,37 @@ impl fmt::Debug for Of {
 /// (month, day of month and leap flag),
 /// which is an index to the `MDL_TO_OL` lookup table.
 #[derive(PartialEq, PartialOrd, Copy, Clone)]
-pub(super) struct Mdf(pub(super) u32);


The value didn't need to be public. Making it private makes it clearer the value is always correct, and thus in bounds for array indexing.

pitdicker · 2023-04-29T15:43:09Z

src/naive/internals.rs

        let Of(of) = *self;
-        Weekday::from_u32(((of >> 4)   (of & 0b111)) % 7).unwrap()
+        Weekday::from_u32_mod7((of >> 4)   (of & 0b111))


Can't use Weekday::from_u32 because that is part of num_traits::FromPrimitive, and trait methods can't yet be const.

pitdicker · 2023-04-29T15:45:34Z

src/weekday.rs

+    /// Create a `Weekday` from an `u32`, with Monday = 0.
+    /// Infallible, takes `n & 7`.
+    #[inline]
+    pub(crate) const fn from_u32_mod7(n: u32) -> Weekday {


For use in this crate it would be nice if there was a const, infallible way to create a Weekday.

Because the two methods constructing a Weekday in internals.rs passed n % 7, making from_u32_mod7 seemed like a good way to have an infallible function and avoid a little code duplication.

Given the mod7 requirement which is not enforced in the type system and the fact that this is used only in the internals module, I think this should be a free function in the internals module rather than a method here.

pitdicker · 2023-04-29T16:45:19Z

@RagibHasin You expressed an interest to help with making the API const. Would you also be willing to review?

And maybe we can work together?

It seems hard to avoid one massive 'make everything const'-PR, because a lot depends on each other. DateTime depends on NaiveDateTime, NaiveDateTime depends on NaiveDate and NaiveTime...

IsoWeek, Month, NaiveTime, Duration and FixedOffset should be pretty self contained. Do you want to make PRs for any or all of those?

RagibHasin · 2023-04-29T17:10:19Z

Thanks, @pitdicker. I expect to have some free time in around 12 hours. I would try to review it then.

Very excited about this progress.

pitdicker · 2023-04-29T17:13:27Z

#638 Already worked on Duration about 2 years ago, but hit the problem Option::unwrap and Option::expect are unavailable in const.

Maybe we can use some ugly macro that imitates them but works in const by doing an implicit panic, like https://users.rust-lang.org/t/compile-time-const-unwrapping/51619/6?

Or chrono would have to update to Rust 2021 and raise the MSRV to 1.57, which supports panic in const context. See #995.

jtmoon79

LGTM

This is a safe and useful improvement!

Just one request for a new test case.

src/weekday.rs

pitdicker

While making parts of NaiveDate const, I noticed there where Options created in places where I would not expect them, and not created where they should.

Of was written to allow the creation of invalid values, and you were supposed to call valid() afterwards and handle the error. That didn't seem very Rusty, so I changed Of is to always be valid and return Options earlier.

There is not any extra validation happening. It is just happening in the functions that would create the invalid state, not in the functions calling them.

src/naive/date.rs

pitdicker · 2023-04-30T07:49:40Z

src/naive/date.rs

+    /// Does not check whether the flags are correct for the provided year.
+    const fn from_of(year: i32, ordinal: u32, flags: YearFlags) -> Option<NaiveDate> {
+        if year < MIN_YEAR || year > MAX_YEAR {
+            return None; // Out-of-range


This already splits out the failure causes for when this method is switched to return a Result.

pitdicker · 2023-04-30T07:51:03Z

src/naive/date.rs

+
+    /// Makes a new `NaiveDate` from year, ordinal and flags.
+    /// Does not check whether the flags are correct for the provided year.
+    const fn from_of(year: i32, ordinal: u32, flags: YearFlags) -> Option<NaiveDate> {


This function was always used in a pattern of NaiveDate::from_of(year, Of::new(ordinal, flags)?). Clearer to combine them, which helps to see the failure cause.

This combination should be a separate commit with no other changes.

Should we also rename this method? The current name doesn't really fit anymore.

pitdicker · 2023-04-30T07:53:07Z

src/naive/date.rs

+        }
+        match mdf.to_of() {
+            Some(of) => Some(NaiveDate { ymdf: (year << 13) | (of.inner() as DateImpl) }),
+            None => None, // Non-existing date


This should return another error than in from_of if this methods is converted to return a Result.

pitdicker · 2023-04-30T07:55:10Z

src/naive/date.rs

-        } else {
-            None
-        }
+    const fn with_of(&self, of: Of) -> NaiveDate {


Because an Of is now always valid, this function no longer has to return Option.

pitdicker · 2023-04-30T07:58:21Z

src/naive/internals.rs

@@ -172,8  172,9 @@ impl fmt::Debug for YearFlags {
    }
 }

+// OL: (ordinal << 1) | leap year flag


This is the important trick of the validation logic.

pitdicker · 2023-04-30T08:16:17Z

src/naive/internals.rs

@@ -266,56  267,66 @@ const OL_TO_MDL: &[u8; MAX_OL as usize   1] = &[
 /// The whole bits except for the least 3 bits are referred as `Ol` (ordinal and leap flag),
 /// which is an index to the `OL_TO_MDL` lookup table.
 #[derive(PartialEq, PartialOrd, Copy, Clone)]
-pub(super) struct Of(pub(crate) u32);
+pub(super) struct Of(u32);


The value of Of is now private and always valid.

pitdicker · 2023-04-30T08:18:47Z

src/naive/internals.rs

+    }
+
+    pub(super) const fn from_date_impl(date_impl: DateImpl) -> Of {
+        // We assume the value in the `DateImpl` is valid.


Had to add this to make Of private, and prevent unnecessary validation when converting from DateImpl. But DateImpl is not private... Had to stop somewhere 😄

esheppa · 2023-04-30T13:18:48Z

Thanks @pitdicker - this looks great. Looks like there were a few critical parts I missed that led me down the path of an internals rewrite (that said, some of it was to try to simplify the code and avoid some conversions, but in the end the benchmarks were much worse). The good news is this looks like a great solution, and I can potentially look into doing a new PR with some of the potentially useful parts of #882 once this is merged

esheppa · 2023-04-30T13:24:25Z

With regards to:

Or chrono would have to update to Rust 2021 and raise the MSRV to 1.57, which supports panic in const context. See #995.

What we can do here is have a non-default feature that allows this (eg const-validation), that said, for Duration another potentially better option is moving to using the Duration type from the stdlib. I've got some PRs (which need a little updating) which I'll update shortly that could provide a good solution here

RagibHasin · 2023-04-30T13:34:05Z

With regards to:

Or chrono would have to update to Rust 2021 and raise the MSRV to 1.57, which supports panic in const context. See #995.

What we can do here is have a non-default feature that allows this (eg const-validation), that said, for Duration another potentially better option is moving to using the Duration type from the stdlib. I've got some PRs (which need a little updating) which I'll update shortly that could provide a good solution here

std::time::Duration cannot represent negative durations. But negative durations feel awkward anyway IMO. If the decision is to move towards stdlib then would there be some new type (eg. Stride) which would be a signed Duration to be returned by subtraction operations?

esheppa · 2023-04-30T13:55:01Z

std::time::Duration cannot represent negative durations. But negative durations feel awkward anyway IMO.

Yes I think the best solution is to avoid negative durations - however I'm also keen to receive any feedback on where they would be useful.

The current design is implemented in this PR: #895 (a continuation of a long series of designs, but this one is able to be used in the 0.4.x series which is a large benefit)

Originally we had thought to use a new enum, eg:

enum TimeDelta { Forwards(Duration), Backwards(Duration) }

However in the current iteration I'm just reusing the Result type, with Ok containing a forwards duration, and Err containing the absolute value of the negative duration if applicable, as there didn't seem to be enough justification to add a new type. Importantly, because Duration doesn't implement Error, this is unlikely to be accidentally handled with ?.

@RagibHasin as this is off topic for this PR, we should continue any further discussions on this at #895

jtmoon79

At risk of sounding like a broken record...

Could some 1:1 tests be added, e.g. each function gets it's own test_function?

On top of that, preferably added in two or more commits:

A bunch of tests in one (or more) a before changes commit
and then one (or more) commit with changes test adjustments.

The additional tests provide clarity to developers about function expectations (esp. code reviewers 😊) and give more confidence about the changes.

Just my request. I'm requesting this now and not previously because a new commit has added a few behavior changes (459defd).

Also, just having more 1:1 function testing would be good for chrono.

pitdicker · 2023-05-01T05:10:23Z

At risk of sounding like a broken record...

Please do! I'll learn 😄

For these changes though: this code is already tested really, really thoroughly.

In this module there are tests for the year flags, ordinal flag-format, and month-dag-flag formats. These test that one form can be converted to another and back correctly. And there are tests that create all possible year flags and ordinals, or year flags and dates, and check some methods.
Although this is another module than NaiveDate, they are very closely integrated. Of::succ and Of::pred are not tested here, but NaiveDate::succ and NaiveDate::pred have tests for the interesting cases.

I don't see much to improve here w.r.t the existing tests.

Or did you mean: add tests to verify the functions keep working in const context?

pitdicker · 2023-05-01T05:14:54Z

I'm requesting this now and not previously because a new commit has added a few behavior changes

Oops, replied to soon. I'll add tests that verify the changed functions return None on invalid cases. We can check those now directly in this module 👍.

pitdicker · 2023-05-04T09:33:36Z

Thanks @pitdicker - this looks great. Looks like there were a few critical parts I missed that led me down the path of an internals rewrite (that said, some of it was to try to simplify the code and avoid some conversions, but in the end the benchmarks were much worse). The good news is this looks like a great solution, and I can potentially look into doing a new PR with some of the potentially useful parts of #882 once this is merged

Nice! Seems like most of the code necessary to make functions in chrono const already exists, it just has to come together 👍.

pitdicker · 2023-05-04T09:49:13Z

What we can do here is have a non-default feature that allows this (eg const-validation), that said, for Duration another potentially better option is moving to using the Duration type from the stdlib. I've got some PRs (which need a little updating) which I'll update shortly that could provide a good solution here

Is that something that can be done on the 0.4.x branch, or would it require breaking changes? I'll reply on your #895 when I get to know that area a little better.

Maybe for the somewhat simple work of making methods const we can make something work for the current implementation.

What we can do here is have a non-default feature that allows this (eg const-validation)

I remember that removing a feature can only be done when going to a new major version. I.e. if we introduce it in 0.4.x, it can't be removed until 0.5. That doesn't seem too bad... Maybe a more general msrv_workarounds that potentially stays relevant?

This feels like a ok-ish solution to me. A macro that maps to regular unwrap without the feature, and a hack with implicit panic on older rust versions. But if we can avoid the mess and raise the MSRV that would be so much nicer.

Maybe just work on that assumption for now, and make a PR with the workarounds if the MSRV has to stay below 1.57?

jtmoon79

LGTM

pitdicker · 2023-05-05T16:41:33Z

Sorry for the slow follow-up. I let myself get distracted by some very interesting panics (in another issue)...

Added a test to check Of::new, Mdf::new, Of::from_mdf, Of::succ and Of::pred return None when they should.

Thank you for the reminders to add tests @jtmoon79.

djc · 2023-05-15T08:58:38Z

src/naive/internals.rs

-        match MDL_TO_OL.get(mdl as usize) {
-            Some(&v) => Of(mdf.wrapping_sub((i32::from(v) as u32 & 0x3ff) << 3)),
-            None => Of(0),
+        if mdl <= MAX_MDL {


Pretty sure <= is incorrect when preparing to index into something? Also for cases like these I prefer to invert the condition so we can do an early return for the simple case (here, Of(0)) and resume unindented for the straight-line path.

The arrays in this file are a bit strange: index 0 has an invalid value, and the length is MAX 1.
It makes some sense, as we encode ordinals and day of month starting from 1. This allows indexing without the need to subtract -1.

Okay, can you add a comment to each comparison where this is relevant?

Would like the comment on the same line as the if, can be shorter like // 1-based indexing.

djc · 2023-05-15T09:00:12Z

src/naive/internals.rs

-        match OL_TO_MDL.get(ol as usize) {
-            Some(&v) => Mdf(of   (u32::from(v) << 3)),
-            None => Mdf(0),
+        if ol <= MAX_OL {


Should be <? Please add some tests for the edge cases here...

Same array with an unused zero index. There is a test with the edge cases.

djc · 2023-05-15T09:00:40Z

src/naive/internals.rs

-        match MDL_TO_OL.get(mdl as usize) {
-            Some(&v) => v >= 0,
-            None => false,
+        if mdl <= MAX_MDL {


<= -> <?

djc · 2023-05-15T09:03:46Z

src/weekday.rs

+    /// Create a `Weekday` from an `u32`, with Monday = 0.
+    /// Infallible, takes `n & 7`.
+    #[inline]
+    pub(crate) const fn from_u32_mod7(n: u32) -> Weekday {


Given the mod7 requirement which is not enforced in the type system and the fact that this is used only in the internals module, I think this should be a free function in the internals module rather than a method here.

src/naive/date.rs

djc · 2023-05-15T09:05:01Z

src/naive/date.rs

+
+    /// Makes a new `NaiveDate` from year, ordinal and flags.
+    /// Does not check whether the flags are correct for the provided year.
+    const fn from_of(year: i32, ordinal: u32, flags: YearFlags) -> Option<NaiveDate> {


This combination should be a separate commit with no other changes.

djc · 2023-05-15T09:24:38Z

src/weekday.rs

@@ -260,6  260,14 @@ mod tests {
            );
        }
    }
+
+    #[test]
+    fn test_from_u32_mod7() {


Please squash this in the commit where the function is added.

djc · 2023-05-15T09:26:00Z

src/naive/internals.rs

@@ -266,6  266,8 @@ const OL_TO_MDL: &[u8; MAX_OL as usize   1] = &[
 ///
 /// The whole bits except for the least 3 bits are referred as `Ol` (ordinal and leap flag),
 /// which is an index to the `OL_TO_MDL` lookup table.
+///
+/// The methods implemented on `Of` always return a valid value.


Please squash this change into the commit that originated the described changes.

src/naive/date.rs

djc · 2023-05-15T15:08:47Z

src/naive/internals.rs

-        match MDL_TO_OL.get(mdl as usize) {
-            Some(&v) => Of(mdf.wrapping_sub((i32::from(v) as u32 & 0x3ff) << 3)),
-            None => Of(0),
+        if mdl <= MAX_MDL {


Would like the comment on the same line as the if, can be shorter like // 1-based indexing.

djc · 2023-05-15T15:11:50Z

src/naive/internals.rs

@@ -467,11  466,28 @@ impl fmt::Debug for Mdf {
    }
 }

+/// Create a `Weekday` from an `u32`, with Monday = 0.
+/// Infallible, takes `n % 7`.


The takes n % 7 comment is no longer correct, right?

The intention was to say it is infallible, because it takes any n and does modulus 7. I'll see if I can word it better.

djc · 2023-05-15T15:12:55Z

src/naive/internals.rs

+        for i in 0..=1000 {
+            assert_eq!(weekday_from_u32_mod7(i), Weekday::from_u32(i % 7).unwrap());
+        }
+        assert_eq!(weekday_from_u32_mod7(u32::MAX), Weekday::Thu);


I don't think there's any point to testing with values larger than 7.

Not really, but testing up to 1000 and a single u32:MAX is cheap.

djc · 2023-05-15T15:16:59Z

src/naive/internals.rs

+        if self.ordinal() == 1 {
+            return None;
+        }
+        Some(Of(self.0 - (1 << 4)))


Style: I prefer writing this as

match self.ordinal() { 1 => None, _ => Some(Of(self.0 - (1 << 4))) }

That looks very clean, thank you 👍

djc · 2023-05-15T15:17:14Z

src/naive/internals.rs

@@ -370,13  387,17 @@ impl fmt::Debug for Of {
 /// The whole bits except for the least 3 bits are referred as `Mdl`
 /// (month, day of month and leap flag),
 /// which is an index to the `MDL_TO_OL` lookup table.
+///
+/// The methods implemented on `Mdf` do not always return a valid value.
+/// Dates than can't exist, like February 30, can still be represented.


"than" -> "that"

Why do we need to allow Mdf instances with an invalid value?

Testing whether an ordinal is valid is easy. Testing a month, day, leap year flag combination is valid, and doing it fast, is more involved.

The table lookup to convert Mdf to Of is pretty much the ideal place to put the validity check.

djc · 2023-05-15T15:19:05Z

src/naive/internals.rs

@@ -662,8  683,7 @@ mod tests {
    fn test_of_fields() {
        for &flags in FLAGS.iter() {
            for ordinal in range_inclusive(1u32, 366) {
-                let of = Of::new(ordinal, flags).unwrap();
-                if of.valid() {
+                if let Some(of) = Of::new(ordinal, flags) {


For tests, let's just unwrap() cases like these?

We can't. The test is creating all possible combinations of year flags and ordinals. But some of these combinations are just invalid. It tests if those that are let through, have an ordinal that round-trips.

djc · 2023-05-15T15:19:50Z

src/naive/date.rs

+
+    /// Makes a new `NaiveDate` from year, ordinal and flags.
+    /// Does not check whether the flags are correct for the provided year.
+    const fn from_of(year: i32, ordinal: u32, flags: YearFlags) -> Option<NaiveDate> {


Should we also rename this method? The current name doesn't really fit anymore.

pitdicker · 2023-05-15T16:07:06Z

Would like the comment on the same line as the if, can be shorter like // 1-based indexing.

I would also like to put it on the same line, but rustfmt disagrees with me.

pitdicker · 2023-05-15T16:32:32Z

Should we also rename this method? The current name doesn't really fit anymore.

(Some of your comments are missing a reply field for some reason.)

I suppose it can be called from_ordinal_and_flags. I quite liked the short name, but agree it can be confusing.

pitdicker · 2023-05-17T04:38:30Z

@djc Just to make sure, you are not waiting on me?

src/naive/date.rs

djc · 2023-05-17T13:02:25Z

@djc Just to make sure, you are not waiting on me?

Nope.

djc

Thanks again for working on this!

pitdicker · 2023-05-23T10:10:34Z

🎉

Time to get my branch that makes most of NaiveDate and NaiveTime const in shape.

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [chrono](https://github.com/chronotope/chrono) | dependencies | patch | `0.4.24` -> `0.4.26` | --- ### Release Notes <details> <summary>chronotope/chrono</summary> ### [`v0.4.26`](https://github.com/chronotope/chrono/releases/tag/v0.4.26): 0.4.26 [Compare Source](chronotope/chrono@v0.4.25...v0.4.26) The changes from [#807](chronotope/chrono#807) we merged for 0.4.25 unfortunately restricted parsing in a way that was incompatible with earlier 0.4.x releases. We reverted this in [#1113](chronotope/chrono#1113). A small amount of other changes were merged since. - Update README ([#1111](chronotope/chrono#1111), thanks to [@pitdicker](https://github.com/pitdicker)) - Revert backport of [#807](chronotope/chrono#807) ([#1113](chronotope/chrono#1113), thanks to [@pitdicker](https://github.com/pitdicker)) - Update to 2021 edition ([#1109](chronotope/chrono#1109), thanks to [@tottoto](https://github.com/tottoto)) - Fix `DurationRound` panics from issue [#1010](chronotope/chrono#1010) ([#1093](chronotope/chrono#1093), thanks to [@pitdicker](https://github.com/pitdicker)) - tests: date path consolidate (branch 0.4.x) ([#1090](chronotope/chrono#1090), thanks to [@jtmoon79](https://github.com/jtmoon79)) - Parse tests nanosecond bare dot (branch 0.4.x) ([#1098](chronotope/chrono#1098), thanks to [@jtmoon79](https://github.com/jtmoon79)) - yamllint cleanup lint.yml test.yml ([#1102](chronotope/chrono#1102), thanks to [@jtmoon79](https://github.com/jtmoon79)) - Remove num-iter dependency ([#1107](chronotope/chrono#1107), thanks to [@tottoto](https://github.com/tottoto)) Thanks on behalf of the chrono team ([@djc](https://github.com/djc) and [@esheppa](https://github.com/esheppa)) to all contributors! ### [`v0.4.25`](https://github.com/chronotope/chrono/releases/tag/v0.4.25): 0.4.25 [Compare Source](chronotope/chrono@v0.4.24...v0.4.25) Time for another maintenance release. This release bumps the MSRV to 1.56; given MSRV bumps in chrono's dependencies (notably for syn 2), we felt that it no longer made sense to support any older versions. Feedback welcome in our issue tracker! ##### Additions - Bump the MSRV to 1.56 ([#1053](chronotope/chrono#1053)) - Apply comments from MSRV bump ([#1026](chronotope/chrono#1026), thanks to [@pitdicker](https://github.com/pitdicker)) - Remove num-integer dependency ([#1037](chronotope/chrono#1037), thanks to [@pitdicker](https://github.com/pitdicker)) - Add `NaiveDateTime::and_utc()` method ([#952](chronotope/chrono#952), thanks to [@klnusbaum](https://github.com/klnusbaum)) - derive `Hash` for most pub types that also derive `PartialEq` ([#938](chronotope/chrono#938), thanks to [@bruceg](https://github.com/bruceg)) - Add `parse_and_remainder()` methods ([#1011](chronotope/chrono#1011), thanks to [@pitdicker](https://github.com/pitdicker)) - Add `DateTime::fix_offset()` ([#1030](chronotope/chrono#1030), thanks to [@pitdicker](https://github.com/pitdicker)) - Add `#[track_caller]` to `LocalResult::unwrap` ([#1046](chronotope/chrono#1046), thanks to [@pitdicker](https://github.com/pitdicker)) - Add `#[must_use]` to some methods ([#1007](chronotope/chrono#1007), thanks to [@aceArt-GmbH](https://github.com/aceArt-GmbH)) - Implement `PartialOrd` for `Month` ([#999](chronotope/chrono#999), thanks to [@Munksgaard](https://github.com/Munksgaard)) - Add `impl From<NaiveDateTime> for NaiveDate` ([#1012](chronotope/chrono#1012), thanks to [@pezcore](https://github.com/pezcore)) - Extract timezone info from tzdata file on Android ([#978](chronotope/chrono#978), thanks to [@RumovZ](https://github.com/RumovZ)) ##### Fixes - Prevent string slicing inside char boundaries ([#1024](chronotope/chrono#1024), thanks to [@pitdicker](https://github.com/pitdicker)) - fix IsoWeek so that its flags are always correct ([#991](chronotope/chrono#991), thanks to [@moshevds](https://github.com/moshevds)) - Fix out-of-range panic in `NaiveWeek::last_day` ([#1070](chronotope/chrono#1070), thanks to [@pitdicker](https://github.com/pitdicker)) - Use correct offset in conversion from `Local` to `FixedOffset` ([#1041](chronotope/chrono#1041), thanks to [@pitdicker](https://github.com/pitdicker)) - Fix military timezones in RFC 2822 parsing ([#1013](chronotope/chrono#1013), thanks to [@pitdicker](https://github.com/pitdicker)) - Guard against overflow in NaiveDate::with_\*0 methods ([#1023](chronotope/chrono#1023), thanks to [@pitdicker](https://github.com/pitdicker)) - Fix panic in from_num_days_from_ce_opt ([#1052](chronotope/chrono#1052), thanks to [@pitdicker](https://github.com/pitdicker)) ##### Refactoring - Rely on std for getting local time offset ([#1072](chronotope/chrono#1072), thanks to [@pitdicker](https://github.com/pitdicker)) - Make functions in internals const ([#1043](chronotope/chrono#1043), thanks to [@pitdicker](https://github.com/pitdicker)) - Refactor windows module in `Local` ([#992](chronotope/chrono#992), thanks to [@nekevss](https://github.com/nekevss)) - Simplify from_timestamp_millis, from_timestamp_micros ([#1032](chronotope/chrono#1032), thanks to [@pitdicker](https://github.com/pitdicker)) - Backport [#983](chronotope/chrono#983) and [#1000](chronotope/chrono#1000) ([#1063](chronotope/chrono#1063), thanks to [@pitdicker](https://github.com/pitdicker)) ##### Documentation - Backport documentation improvements ([#1066](chronotope/chrono#1066), thanks to [@pitdicker](https://github.com/pitdicker)) - Add documentation for %Z quirk ([#1051](chronotope/chrono#1051), thanks to [@campbellcole](https://github.com/campbellcole)) - Add an example to Weekday ([#1019](chronotope/chrono#1019), thanks to [@pitdicker](https://github.com/pitdicker)) ##### Internal improvements - Gate test on `clock` feature ([#1061](chronotope/chrono#1061), thanks to [@pitdicker](https://github.com/pitdicker)) - CI: Also run tests with `--no-default-features` ([#1059](chronotope/chrono#1059), thanks to [@pitdicker](https://github.com/pitdicker)) - Prevent `bench_year_flags_from_year` from being optimized out ([#1034](chronotope/chrono#1034), thanks to [@pitdicker](https://github.com/pitdicker)) - Fix test_leap_second during DST transition ([#1064](chronotope/chrono#1064), thanks to [@pitdicker](https://github.com/pitdicker)) - Fix warnings when running tests on Windows ([#1038](chronotope/chrono#1038), thanks to [@pitdicker](https://github.com/pitdicker)) - Fix tests on AIX ([#1028](chronotope/chrono#1028), thanks to [@ecnelises](https://github.com/ecnelises)) - Fix doctest warnings, remove mention of deprecated methods from main doc ([#1081](chronotope/chrono#1081), thanks to [@pitdicker](https://github.com/pitdicker)) - Reformat `test_datetime_parse_from_str` ([#1078](chronotope/chrono#1078), thanks to [@pitdicker](https://github.com/pitdicker)) - GitHub yml shell `set -eux`, use bash ([#1103](chronotope/chrono#1103), thanks to [@jtmoon79](https://github.com/jtmoon79)) - test: explicitly set `LANG` to `c` in gnu `date` ([#1089](chronotope/chrono#1089), thanks to [@scarf005](https://github.com/scarf005)) - Switch test to `TryFrom` ([#1086](chronotope/chrono#1086), thanks to [@pitdicker](https://github.com/pitdicker)) - Add test for issue 551 ([#1020](chronotope/chrono#1020), thanks to [@pitdicker](https://github.com/pitdicker)) - RFC 2822 single-letter obsolete tests ([#1014](chronotope/chrono#1014), thanks to [@jtmoon79](https://github.com/jtmoon79)) - \[CI] Lint Windows target and documentation links ([#1062](chronotope/chrono#1062), thanks to [@pitdicker](https://github.com/pitdicker)) - add test_issue\_866 ([#1077](chronotope/chrono#1077), thanks to [@jtmoon79](https://github.com/jtmoon79)) - Remove AUTHORS metadata ([#1074](chronotope/chrono#1074)) On behalf of [@djc](https://github.com/djc) and [@esheppa](https://github.com/esheppa), thanks to all contributors! </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Co-authored-by: cabr2-bot <[email protected]> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1909 Reviewed-by: crapStone <[email protected]> Co-authored-by: Calciumdibromid Bot <[email protected]> Co-committed-by: Calciumdibromid Bot <[email protected]>

pitdicker force-pushed the const_internals branch from da835ae to 9167548 Compare April 29, 2023 15:50

pitdicker commented Apr 29, 2023

View reviewed changes

jtmoon79 approved these changes Apr 29, 2023

View reviewed changes

src/weekday.rs Outdated Show resolved Hide resolved

pitdicker force-pushed the const_internals branch from 6540cbf to 459defd Compare April 30, 2023 08:14

pitdicker commented Apr 30, 2023

View reviewed changes

jtmoon79 reviewed Apr 30, 2023

View reviewed changes

jtmoon79 approved these changes May 5, 2023

View reviewed changes

pitdicker force-pushed the const_internals branch from f7f4db1 to f92ac6b Compare May 5, 2023 16:37

pitdicker force-pushed the const_internals branch 2 times, most recently from bfc85fc to 600c953 Compare May 15, 2023 09:09

djc reviewed May 15, 2023

View reviewed changes

pitdicker force-pushed the const_internals branch 3 times, most recently from 59122a3 to b354578 Compare May 15, 2023 12:08

pitdicker commented May 15, 2023

View reviewed changes

src/naive/date.rs Outdated Show resolved Hide resolved

pitdicker force-pushed the const_internals branch from b354578 to 013427e Compare May 15, 2023 13:32

djc reviewed May 15, 2023

View reviewed changes

pitdicker force-pushed the const_internals branch from 013427e to 2e71e97 Compare May 15, 2023 16:31

esheppa reviewed May 17, 2023

View reviewed changes

src/naive/date.rs Show resolved Hide resolved

esheppa reviewed May 17, 2023

View reviewed changes

src/naive/date.rs Show resolved Hide resolved

pitdicker and others added 4 commits May 17, 2023 15:49

Make fns in internals const

4ef6875

Make internal functions returning Weekday const

314d3d6

Ensure Of is always valid

cc22196

Change NaiveDate::from_of to take ordinal and year flags

d2e88b2

pitdicker force-pushed the const_internals branch from 2e71e97 to d2e88b2 Compare May 17, 2023 13:56

pitdicker mentioned this pull request May 18, 2023

Fix DateTime out-of-range panics #1048

Closed

djc approved these changes May 23, 2023

View reviewed changes

djc merged commit 403b247 into chronotope:0.4.x May 23, 2023

pitdicker deleted the const_internals branch May 23, 2023 10:06

pitdicker mentioned this pull request May 26, 2023

Switch test to TryFrom #1086

Merged

pitdicker mentioned this pull request Jul 1, 2023

Changes to naive::internals to enable more const functions #882

Closed

pitdicker mentioned this pull request Jul 28, 2023

Use NonZeroI32 inside NaiveDate #1207

Merged

This was referenced Feb 2, 2024

Consider refactoring chrono to avoid systematic misuse of panics #815

Closed

Recognize our review capacity is limited for non-trivial work #1417

Open

pitdicker mentioned this pull request Feb 13, 2024

Mdf performance and docs #1438

Merged

Make functions in internals const #1043

Make functions in internals const #1043

Conversation

pitdicker commented Apr 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pitdicker Apr 29, 2023 • edited Loading

Choose a reason for hiding this comment

pitdicker Apr 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pitdicker commented Apr 29, 2023 • edited Loading

RagibHasin commented Apr 29, 2023

pitdicker commented Apr 29, 2023

jtmoon79 left a comment

Choose a reason for hiding this comment

pitdicker left a comment

Choose a reason for hiding this comment

pitdicker Apr 30, 2023 • edited Loading

Choose a reason for hiding this comment

pitdicker Apr 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

esheppa commented Apr 30, 2023

esheppa commented Apr 30, 2023

RagibHasin commented Apr 30, 2023

esheppa commented Apr 30, 2023 • edited Loading

jtmoon79 left a comment • edited Loading

Choose a reason for hiding this comment

pitdicker commented May 1, 2023

pitdicker commented May 1, 2023

pitdicker commented May 4, 2023

pitdicker commented May 4, 2023

jtmoon79 left a comment

Choose a reason for hiding this comment

pitdicker commented May 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pitdicker commented May 15, 2023

pitdicker commented May 15, 2023

pitdicker commented May 17, 2023

djc commented May 17, 2023

djc left a comment

Choose a reason for hiding this comment

pitdicker commented May 23, 2023 • edited Loading

🎉

pitdicker commented Apr 29, 2023 •

edited

Loading

pitdicker Apr 29, 2023 •

edited

Loading

pitdicker Apr 29, 2023 •

edited

Loading

pitdicker commented Apr 29, 2023 •

edited

Loading

pitdicker Apr 30, 2023 •

edited

Loading

pitdicker Apr 30, 2023 •

edited

Loading

esheppa commented Apr 30, 2023 •

edited

Loading

jtmoon79 left a comment •

edited

Loading

pitdicker commented May 23, 2023 •

edited

Loading