From e4123531ce30ab621e65c5e9b81882e83b9fa5f0 Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Fri, 11 Aug 2023 14:36:25 +0200 Subject: [PATCH 1/5] Move `iso_week_from_yof` to `IsoWeek::from_yof` --- src/naive/date.rs | 3 +-- src/naive/isoweek.rs | 48 ++++++++++++++++++++++---------------------- 2 files changed, 25 insertions(+), 26 deletions(-) diff --git a/src/naive/date.rs b/src/naive/date.rs index 6b5c9f559f..c4daae1370 100644 --- a/src/naive/date.rs +++ b/src/naive/date.rs @@ -38,7 +38,6 @@ use crate::{expect, try_opt}; use crate::{Datelike, TimeDelta, Weekday}; use super::internals::{self, Mdf, Of, YearFlags}; -use super::isoweek; /// A week represented by a [`NaiveDate`] and a [`Weekday`] which is the first /// day of the week. @@ -1689,7 +1688,7 @@ impl Datelike for NaiveDate { #[inline] fn iso_week(&self) -> IsoWeek { - isoweek::iso_week_from_yof(self.year(), self.of()) + IsoWeek::from_yof(self.year(), self.of()) } /// Makes a new `NaiveDate` with the year number changed, while keeping the same month and day. diff --git a/src/naive/isoweek.rs b/src/naive/isoweek.rs index c4649db557..45a142a0dc 100644 --- a/src/naive/isoweek.rs +++ b/src/naive/isoweek.rs @@ -31,32 +31,32 @@ pub struct IsoWeek { ywf: i32, // (year << 10) | (week << 4) | flag } -/// Returns the corresponding `IsoWeek` from the year and the `Of` internal value. -// -// internal use only. we don't expose the public constructor for `IsoWeek` for now, -// because the year range for the week date and the calendar date do not match and -// it is confusing to have a date that is out of range in one and not in another. -// currently we sidestep this issue by making `IsoWeek` fully dependent of `Datelike`. -pub(super) fn iso_week_from_yof(year: i32, of: Of) -> IsoWeek { - let (rawweek, _) = of.isoweekdate_raw(); - let (year, week) = if rawweek < 1 { - // previous year - let prevlastweek = YearFlags::from_year(year - 1).nisoweeks(); - (year - 1, prevlastweek) - } else { - let lastweek = of.flags().nisoweeks(); - if rawweek > lastweek { - // next year - (year + 1, 1) +impl IsoWeek { + /// Returns the corresponding `IsoWeek` from the year and the `Of` internal value. + // + // internal use only. we don't expose the public constructor for `IsoWeek` for now, + // because the year range for the week date and the calendar date do not match and + // it is confusing to have a date that is out of range in one and not in another. + // currently we sidestep this issue by making `IsoWeek` fully dependent of `Datelike`. + pub(super) fn from_yof(year: i32, of: Of) -> Self { + let (rawweek, _) = of.isoweekdate_raw(); + let (year, week) = if rawweek < 1 { + // previous year + let prevlastweek = YearFlags::from_year(year - 1).nisoweeks(); + (year - 1, prevlastweek) } else { - (year, rawweek) - } - }; - let flags = YearFlags::from_year(year); - IsoWeek { ywf: (year << 10) | (week << 4) as i32 | i32::from(flags.0) } -} + let lastweek = of.flags().nisoweeks(); + if rawweek > lastweek { + // next year + (year + 1, 1) + } else { + (year, rawweek) + } + }; + let flags = YearFlags::from_year(year); + IsoWeek { ywf: (year << 10) | (week << 4) as i32 | i32::from(flags.0) } + } -impl IsoWeek { /// Returns the year number for this ISO week. /// /// # Example From 45f750e202e680c692bdcba4bb9f680e4e2b0f09 Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Sun, 30 Jul 2023 13:14:45 +0200 Subject: [PATCH 2/5] Change `IsoWeek::from_yof` to consuming `ordinal` and `year_flags` remove `Of::isoweekdate_raw` --- src/naive/date.rs | 34 ++++++++++++++++++++++++++++++++-- src/naive/internals.rs | 17 ----------------- src/naive/isoweek.rs | 8 ++++---- 3 files changed, 36 insertions(+), 23 deletions(-) diff --git a/src/naive/date.rs b/src/naive/date.rs index c4daae1370..4b965d2711 100644 --- a/src/naive/date.rs +++ b/src/naive/date.rs @@ -1688,7 +1688,7 @@ impl Datelike for NaiveDate { #[inline] fn iso_week(&self) -> IsoWeek { - IsoWeek::from_yof(self.year(), self.of()) + IsoWeek::from_yof(self.year(), self.ordinal(), self.of().flags()) } /// Makes a new `NaiveDate` with the year number changed, while keeping the same month and day. @@ -2516,7 +2516,7 @@ mod serde { #[cfg(test)] mod tests { use super::{Days, Months, NaiveDate, MAX_YEAR, MIN_YEAR}; - use crate::naive::internals::YearFlags; + use crate::naive::internals::{YearFlags, A, AG, B, BA, C, CB, D, DC, E, ED, F, FE, G, GF}; use crate::{Datelike, TimeDelta, Weekday}; // as it is hard to verify year flags in `NaiveDate::MIN` and `NaiveDate::MAX`, @@ -3375,6 +3375,36 @@ mod tests { } } + #[test] + fn test_isoweekdate_with_yearflags() { + for (year, year_flags, _) in YEAR_FLAGS { + // January 4 should be in the first week + let jan4 = NaiveDate::from_ymd_opt(year, 1, 4).unwrap(); + let iso_week = jan4.iso_week(); + assert_eq!(jan4.of().flags(), year_flags); + assert_eq!(iso_week.week(), 1); + } + } + + // Used for testing some methods with all combinations of `YearFlags`. + // (year, flags, first weekday of year) + const YEAR_FLAGS: [(i32, YearFlags, Weekday); 14] = [ + (2006, A, Weekday::Sun), + (2005, B, Weekday::Sat), + (2010, C, Weekday::Fri), + (2009, D, Weekday::Thu), + (2003, E, Weekday::Wed), + (2002, F, Weekday::Tue), + (2001, G, Weekday::Mon), + (2012, AG, Weekday::Sun), + (2000, BA, Weekday::Sat), + (2016, CB, Weekday::Fri), + (2004, DC, Weekday::Thu), + (2020, ED, Weekday::Wed), + (2008, FE, Weekday::Tue), + (2024, GF, Weekday::Mon), + ]; + #[test] #[cfg(feature = "rkyv-validation")] fn test_rkyv_validation() { diff --git a/src/naive/internals.rs b/src/naive/internals.rs index 0af7d44923..451d937235 100644 --- a/src/naive/internals.rs +++ b/src/naive/internals.rs @@ -322,14 +322,6 @@ impl Of { weekday_from_u32_mod7((of >> 4) + (of & 0b111)) } - #[inline] - pub(super) fn isoweekdate_raw(&self) -> (u32, Weekday) { - // week ordinal = ordinal + delta - let Of(of) = *self; - let weekord = (of >> 4).wrapping_add(self.flags().isoweek_delta()); - (weekord / 7, weekday_from_u32_mod7(weekord)) - } - #[cfg_attr(feature = "cargo-clippy", allow(clippy::wrong_self_convention))] #[inline] pub(super) const fn to_mdf(&self) -> Mdf { @@ -740,15 +732,6 @@ mod tests { } } - #[test] - fn test_of_isoweekdate_raw() { - for &flags in FLAGS.iter() { - // January 4 should be in the first week - let (week, _) = Of::new(4 /* January 4 */, flags).unwrap().isoweekdate_raw(); - assert_eq!(week, 1); - } - } - #[test] fn test_of_to_mdf() { for i in 0u32..=8192 { diff --git a/src/naive/isoweek.rs b/src/naive/isoweek.rs index 45a142a0dc..1d0ddfba36 100644 --- a/src/naive/isoweek.rs +++ b/src/naive/isoweek.rs @@ -5,7 +5,7 @@ use core::fmt; -use super::internals::{Of, YearFlags}; +use super::internals::YearFlags; #[cfg(any(feature = "rkyv", feature = "rkyv-16", feature = "rkyv-32", feature = "rkyv-64"))] use rkyv::{Archive, Deserialize, Serialize}; @@ -38,14 +38,14 @@ impl IsoWeek { // because the year range for the week date and the calendar date do not match and // it is confusing to have a date that is out of range in one and not in another. // currently we sidestep this issue by making `IsoWeek` fully dependent of `Datelike`. - pub(super) fn from_yof(year: i32, of: Of) -> Self { - let (rawweek, _) = of.isoweekdate_raw(); + pub(super) fn from_yof(year: i32, ordinal: u32, year_flags: YearFlags) -> Self { + let rawweek = (ordinal + year_flags.isoweek_delta()) / 7; let (year, week) = if rawweek < 1 { // previous year let prevlastweek = YearFlags::from_year(year - 1).nisoweeks(); (year - 1, prevlastweek) } else { - let lastweek = of.flags().nisoweeks(); + let lastweek = year_flags.nisoweeks(); if rawweek > lastweek { // next year (year + 1, 1) From 4adb854c23b6d38f0de748eb802e77928a96dfc5 Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Sun, 30 Jul 2023 13:08:04 +0200 Subject: [PATCH 3/5] Remove `Of::weekday` --- src/naive/date.rs | 33 ++++++++++++++++++++++- src/naive/internals.rs | 60 ------------------------------------------ 2 files changed, 32 insertions(+), 61 deletions(-) diff --git a/src/naive/date.rs b/src/naive/date.rs index 4b965d2711..22a486b7e3 100644 --- a/src/naive/date.rs +++ b/src/naive/date.rs @@ -1473,10 +1473,19 @@ impl NaiveDate { self.mdf().day() } + /// Returns the day of week. // This duplicates `Datelike::weekday()`, because trait methods can't be const yet. #[inline] const fn weekday(&self) -> Weekday { - self.of().weekday() + match (((self.yof & ORDINAL_MASK) >> 4) + (self.yof & WEEKDAY_FLAGS_MASK)) % 7 { + 0 => Weekday::Mon, + 1 => Weekday::Tue, + 2 => Weekday::Wed, + 3 => Weekday::Thu, + 4 => Weekday::Fri, + 5 => Weekday::Sat, + _ => Weekday::Sun, + } } /// Counts the days in the proleptic Gregorian calendar, with January 1, Year 1 (CE) as day 1. @@ -2332,6 +2341,10 @@ const LEAP_YEAR_MASK: i32 = 0b1000; const OL_MASK: i32 = ORDINAL_MASK | LEAP_YEAR_MASK; const MAX_OL: i32 = 366 << 4; +// Weekday of the last day in the preceding year. +// Allows for quick day of week calculation from the 1-based ordinal. +const WEEKDAY_FLAGS_MASK: i32 = 0b111; + #[cfg(all(test, any(feature = "rustc-serialize", feature = "serde")))] fn test_encodable_json(to_string: F) where @@ -3375,6 +3388,24 @@ mod tests { } } + #[test] + fn test_weekday_with_yearflags() { + for (year, year_flags, first_weekday) in YEAR_FLAGS { + let first_day_of_year = NaiveDate::from_yo_opt(year, 1).unwrap(); + dbg!(year); + assert_eq!(first_day_of_year.of().flags(), year_flags); + assert_eq!(first_day_of_year.weekday(), first_weekday); + + let mut prev = first_day_of_year.weekday(); + for ordinal in 2u32..=year_flags.ndays() { + let date = NaiveDate::from_yo_opt(year, ordinal).unwrap(); + let expected = prev.succ(); + assert_eq!(date.weekday(), expected); + prev = expected; + } + } + } + #[test] fn test_isoweekdate_with_yearflags() { for (year, year_flags, _) in YEAR_FLAGS { diff --git a/src/naive/internals.rs b/src/naive/internals.rs index 451d937235..3da29f51e1 100644 --- a/src/naive/internals.rs +++ b/src/naive/internals.rs @@ -15,7 +15,6 @@ #![cfg_attr(feature = "__internal_bench", allow(missing_docs))] -use crate::Weekday; use core::fmt; /// The year flags (aka the dominical letter). @@ -316,12 +315,6 @@ impl Of { YearFlags((self.0 & 0b1111) as u8) } - #[inline] - pub(super) const fn weekday(&self) -> Weekday { - let Of(of) = *self; - weekday_from_u32_mod7((of >> 4) + (of & 0b111)) - } - #[cfg_attr(feature = "cargo-clippy", allow(clippy::wrong_self_convention))] #[inline] pub(super) const fn to_mdf(&self) -> Mdf { @@ -447,27 +440,10 @@ impl fmt::Debug for Mdf { } } -/// Create a `Weekday` from an `u32`, with Monday = 0. -/// Infallible, takes any `n` and applies `% 7`. -#[inline] -const fn weekday_from_u32_mod7(n: u32) -> Weekday { - match n % 7 { - 0 => Weekday::Mon, - 1 => Weekday::Tue, - 2 => Weekday::Wed, - 3 => Weekday::Thu, - 4 => Weekday::Fri, - 5 => Weekday::Sat, - _ => Weekday::Sun, - } -} - #[cfg(test)] mod tests { - use super::weekday_from_u32_mod7; use super::{Mdf, Of}; use super::{YearFlags, A, AG, B, BA, C, CB, D, DC, E, ED, F, FE, G, GF}; - use crate::Weekday; const NONLEAP_FLAGS: [YearFlags; 7] = [A, B, C, D, E, F, G]; const LEAP_FLAGS: [YearFlags; 7] = [AG, BA, CB, DC, ED, FE, GF]; @@ -635,34 +611,6 @@ mod tests { } } - #[test] - fn test_of_weekday() { - assert_eq!(Of::new(1, A).unwrap().weekday(), Weekday::Sun); - assert_eq!(Of::new(1, B).unwrap().weekday(), Weekday::Sat); - assert_eq!(Of::new(1, C).unwrap().weekday(), Weekday::Fri); - assert_eq!(Of::new(1, D).unwrap().weekday(), Weekday::Thu); - assert_eq!(Of::new(1, E).unwrap().weekday(), Weekday::Wed); - assert_eq!(Of::new(1, F).unwrap().weekday(), Weekday::Tue); - assert_eq!(Of::new(1, G).unwrap().weekday(), Weekday::Mon); - assert_eq!(Of::new(1, AG).unwrap().weekday(), Weekday::Sun); - assert_eq!(Of::new(1, BA).unwrap().weekday(), Weekday::Sat); - assert_eq!(Of::new(1, CB).unwrap().weekday(), Weekday::Fri); - assert_eq!(Of::new(1, DC).unwrap().weekday(), Weekday::Thu); - assert_eq!(Of::new(1, ED).unwrap().weekday(), Weekday::Wed); - assert_eq!(Of::new(1, FE).unwrap().weekday(), Weekday::Tue); - assert_eq!(Of::new(1, GF).unwrap().weekday(), Weekday::Mon); - - for &flags in FLAGS.iter() { - let mut prev = Of::new(1, flags).unwrap().weekday(); - for ordinal in 2u32..=flags.ndays() { - let of = Of::new(ordinal, flags).unwrap(); - let expected = prev.succ(); - assert_eq!(of.weekday(), expected); - prev = expected; - } - } - } - #[test] fn test_mdf_fields() { for &flags in FLAGS.iter() { @@ -789,12 +737,4 @@ mod tests { assert!(Of::from_mdf(Mdf::new(2, 29, leap_year).unwrap()).is_some()); assert!(Of::from_mdf(Mdf::new(2, 28, regular_year).unwrap()).is_some()); } - - #[test] - fn test_weekday_from_u32_mod7() { - for i in 0..=1000 { - assert_eq!(weekday_from_u32_mod7(i), Weekday::try_from((i % 7) as u8).unwrap()); - } - assert_eq!(weekday_from_u32_mod7(u32::MAX), Weekday::Thu); - } } From 671cac652fc21ce401d4dc6089ac7190a4816846 Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Fri, 11 Aug 2023 14:19:19 +0200 Subject: [PATCH 4/5] Remove `Of::new` and basic functionality test for `Of` --- src/naive/internals.rs | 48 ------------------------------------------ 1 file changed, 48 deletions(-) diff --git a/src/naive/internals.rs b/src/naive/internals.rs index 3da29f51e1..5ba10fe975 100644 --- a/src/naive/internals.rs +++ b/src/naive/internals.rs @@ -265,13 +265,6 @@ const OL_TO_MDL: &[u8; MAX_OL as usize + 1] = &[ pub(super) struct Of(u32); impl Of { - #[inline] - #[cfg(test)] - pub(super) const fn new(ordinal: u32, YearFlags(flags): YearFlags) -> Option { - let of = Of((ordinal << 4) | flags as u32); - of.validate() - } - pub(super) const fn from_date_impl(date_impl: i32) -> Of { // We assume the value in `date_impl` is valid. Of((date_impl & 0b1_1111_1111_1111) as u32) @@ -484,42 +477,6 @@ mod tests { assert_eq!(GF.nisoweeks(), 52); } - #[test] - fn test_of() { - fn check(expected: bool, flags: YearFlags, ordinal1: u32, ordinal2: u32) { - for ordinal in ordinal1..=ordinal2 { - let of = match Of::new(ordinal, flags) { - Some(of) => of, - None if !expected => continue, - None => panic!("Of::new({}, {:?}) returned None", ordinal, flags), - }; - - assert!( - of.validate().is_some() == expected, - "ordinal {} = {:?} should be {} for dominical year {:?}", - ordinal, - of, - if expected { "valid" } else { "invalid" }, - flags - ); - } - } - - for &flags in NONLEAP_FLAGS.iter() { - check(false, flags, 0, 0); - check(true, flags, 1, 365); - check(false, flags, 366, 1024); - check(false, flags, u32::MAX, u32::MAX); - } - - for &flags in LEAP_FLAGS.iter() { - check(false, flags, 0, 0); - check(true, flags, 1, 366); - check(false, flags, 367, 1024); - check(false, flags, u32::MAX, u32::MAX); - } - } - #[test] fn test_mdf_valid() { fn check(expected: bool, flags: YearFlags, month1: u32, day1: u32, month2: u32, day2: u32) { @@ -720,11 +677,6 @@ mod tests { fn test_invalid_returns_none() { let regular_year = YearFlags::from_year(2023); let leap_year = YearFlags::from_year(2024); - assert!(Of::new(0, regular_year).is_none()); - assert!(Of::new(366, regular_year).is_none()); - assert!(Of::new(366, leap_year).is_some()); - assert!(Of::new(367, regular_year).is_none()); - assert!(Mdf::new(0, 1, regular_year).is_none()); assert!(Mdf::new(13, 1, regular_year).is_none()); assert!(Mdf::new(1, 0, regular_year).is_none()); From df2e6f741f875d43e42c2171e6d4f92c5d78ecae Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Thu, 10 Aug 2023 22:06:18 +0200 Subject: [PATCH 5/5] Remove `Of::flags`, add `NaiveDate::year_flags` --- src/naive/date.rs | 28 +++++++++++++++++++++------- src/naive/internals.rs | 5 ----- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/src/naive/date.rs b/src/naive/date.rs index 22a486b7e3..b8aafdee87 100644 --- a/src/naive/date.rs +++ b/src/naive/date.rs @@ -1488,6 +1488,11 @@ impl NaiveDate { } } + #[inline] + const fn year_flags(&self) -> YearFlags { + YearFlags((self.yof & YEAR_FLAGS_MASK) as u8) + } + /// Counts the days in the proleptic Gregorian calendar, with January 1, Year 1 (CE) as day 1. // This duplicates `Datelike::num_days_from_ce()`, because trait methods can't be const yet. pub(crate) const fn num_days_from_ce(&self) -> i32 { @@ -1697,7 +1702,7 @@ impl Datelike for NaiveDate { #[inline] fn iso_week(&self) -> IsoWeek { - IsoWeek::from_yof(self.year(), self.ordinal(), self.of().flags()) + IsoWeek::from_yof(self.year(), self.ordinal(), self.year_flags()) } /// Makes a new `NaiveDate` with the year number changed, while keeping the same month and day. @@ -2345,6 +2350,8 @@ const MAX_OL: i32 = 366 << 4; // Allows for quick day of week calculation from the 1-based ordinal. const WEEKDAY_FLAGS_MASK: i32 = 0b111; +const YEAR_FLAGS_MASK: i32 = LEAP_YEAR_MASK | WEEKDAY_FLAGS_MASK; + #[cfg(all(test, any(feature = "rustc-serialize", feature = "serde")))] fn test_encodable_json(to_string: F) where @@ -2541,12 +2548,12 @@ mod tests { assert!( NaiveDate::MIN == calculated_min, "`NaiveDate::MIN` should have year flag {:?}", - calculated_min.of().flags() + calculated_min.year_flags() ); assert!( NaiveDate::MAX == calculated_max, "`NaiveDate::MAX` should have year flag {:?} and ordinal {}", - calculated_max.of().flags(), + calculated_max.year_flags(), calculated_max.ordinal() ); @@ -2561,11 +2568,11 @@ mod tests { ); const BEFORE_MIN: NaiveDate = NaiveDate::BEFORE_MIN; - assert_eq!(BEFORE_MIN.of().flags(), YearFlags::from_year(BEFORE_MIN.year())); + assert_eq!(BEFORE_MIN.year_flags(), YearFlags::from_year(BEFORE_MIN.year())); assert_eq!((BEFORE_MIN.month(), BEFORE_MIN.day()), (12, 31)); const AFTER_MAX: NaiveDate = NaiveDate::AFTER_MAX; - assert_eq!(AFTER_MAX.of().flags(), YearFlags::from_year(AFTER_MAX.year())); + assert_eq!(AFTER_MAX.year_flags(), YearFlags::from_year(AFTER_MAX.year())); assert_eq!((AFTER_MAX.month(), AFTER_MAX.day()), (1, 1)); } @@ -3388,12 +3395,19 @@ mod tests { } } + #[test] + fn test_date_yearflags() { + for (year, year_flags, _) in YEAR_FLAGS { + assert_eq!(NaiveDate::from_yo_opt(year, 1).unwrap().year_flags(), year_flags); + } + } + #[test] fn test_weekday_with_yearflags() { for (year, year_flags, first_weekday) in YEAR_FLAGS { let first_day_of_year = NaiveDate::from_yo_opt(year, 1).unwrap(); dbg!(year); - assert_eq!(first_day_of_year.of().flags(), year_flags); + assert_eq!(first_day_of_year.year_flags(), year_flags); assert_eq!(first_day_of_year.weekday(), first_weekday); let mut prev = first_day_of_year.weekday(); @@ -3412,7 +3426,7 @@ mod tests { // January 4 should be in the first week let jan4 = NaiveDate::from_ymd_opt(year, 1, 4).unwrap(); let iso_week = jan4.iso_week(); - assert_eq!(jan4.of().flags(), year_flags); + assert_eq!(jan4.year_flags(), year_flags); assert_eq!(iso_week.week(), 1); } } diff --git a/src/naive/internals.rs b/src/naive/internals.rs index 5ba10fe975..95690d0364 100644 --- a/src/naive/internals.rs +++ b/src/naive/internals.rs @@ -303,11 +303,6 @@ impl Of { } } - #[inline] - pub(super) const fn flags(&self) -> YearFlags { - YearFlags((self.0 & 0b1111) as u8) - } - #[cfg_attr(feature = "cargo-clippy", allow(clippy::wrong_self_convention))] #[inline] pub(super) const fn to_mdf(&self) -> Mdf {