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.

Sign up for GitHub

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

Jump to bottom

Use NonZeroI32 inside NaiveDate #1207

Merged
merged 2 commits into from Feb 12, 2024
Merged

Use NonZeroI32 inside NaiveDate #1207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
95e2439
Add test for the size of chrono's main data types
pitdicker Apr 25, 2023
1684fee
Use `NonZeroI32` inside `NaiveDate`
pitdicker Feb 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 24 additions & 0 deletions src/lib.rs
View file
Open in desktop
Expand Up @@ -627,3 +627,27 @@ macro_rules! expect {
}
};
}

#[cfg(test)]
mod tests {
#[cfg(feature = "clock")]
use crate::{DateTime, FixedOffset, Local, NaiveDate, NaiveDateTime, NaiveTime, Utc};

#[test]
#[allow(deprecated)]
#[cfg(feature = "clock")]
fn test_type_sizes() {
use core::mem::size_of;
assert_eq!(size_of::<NaiveDate>(), 4);
assert_eq!(size_of::<Option<NaiveDate>>(), 4);
assert_eq!(size_of::<NaiveTime>(), 8);
assert_eq!(size_of::<Option<NaiveTime>>(), 12);
assert_eq!(size_of::<NaiveDateTime>(), 12);
assert_eq!(size_of::<Option<NaiveDateTime>>(), 12);

assert_eq!(size_of::<DateTime<Utc>>(), 12);
assert_eq!(size_of::<DateTime<FixedOffset>>(), 16);
assert_eq!(size_of::<DateTime<Local>>(), 16);
assert_eq!(size_of::<Option<DateTime<FixedOffset>>>(), 16);
}
}
62 changes: 39 additions & 23 deletions src/naive/date/mod.rs
View file
Open in desktop
Expand Up @@ -16,6 +16,7 @@
#[cfg(feature = "alloc")]
use core::borrow::Borrow;
use core::iter::FusedIterator;
use core::num::NonZeroI32;
use core::ops::{Add, AddAssign, Sub, SubAssign};
use core::{fmt, str};

Expand Down Expand Up @@ -99,7 +100,7 @@ mod tests;
)]
#[cfg_attr(feature = "rkyv-validation", archive(check_bytes))]
pub struct NaiveDate {
yof: i32, // (year << 13) | of
yof: NonZeroI32, // (year << 13) | of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this change the NaiveDate MAX or MIN? If not, why not?

Copy link
Collaborator Author

@pitdicker pitdicker Feb 12, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't change that much.
Our internal representation uses an ordinal that is always in the range 1..366, and year flags that are never 0. So the value was already never 0; we now tell the type system as much.

}

/// The minimum possible `NaiveDate` (January 1, 262145 BCE).
Expand Down Expand Up @@ -140,7 +141,7 @@ impl NaiveDate {
debug_assert!(YearFlags::from_year(year).0 == flags.0);
let yof = (year << 13) | (ordinal << 4) as i32 | flags.0 as i32;
match yof & OL_MASK <= MAX_OL {
true => Some(NaiveDate { yof }),
true => Some(NaiveDate::from_yof(yof)),
false => None, // Does not exist: Ordinal 366 in a common year.
}
}
Expand Down Expand Up @@ -691,10 +692,10 @@ impl NaiveDate {
pub(crate) const fn add_days(self, days: i32) -> Option<Self> {
// fast path if the result is within the same year
const ORDINAL_MASK: i32 = 0b1_1111_1111_0000;
if let Some(ordinal) = ((self.yof & ORDINAL_MASK) >> 4).checked_add(days) {
if let Some(ordinal) = ((self.yof() & ORDINAL_MASK) >> 4).checked_add(days) {
if ordinal > 0 && ordinal <= 365 {
let year_and_flags = self.yof & !ORDINAL_MASK;
return Some(NaiveDate { yof: year_and_flags | (ordinal << 4) });
let year_and_flags = self.yof() & !ORDINAL_MASK;
return Some(NaiveDate::from_yof(year_and_flags | (ordinal << 4)));
}
}
// do the full check
Expand Down Expand Up @@ -939,7 +940,7 @@ impl NaiveDate {
/// Returns the packed month-day-flags.
#[inline]
const fn mdf(&self) -> Mdf {
Mdf::from_ol((self.yof & OL_MASK) >> 3, self.year_flags())
Mdf::from_ol((self.yof() & OL_MASK) >> 3, self.year_flags())
}

/// Makes a new `NaiveDate` with the packed month-day-flags changed.
Expand All @@ -950,7 +951,7 @@ impl NaiveDate {
debug_assert!(self.year_flags().0 == mdf.year_flags().0);
match mdf.ordinal() {
Some(ordinal) => {
Some(NaiveDate { yof: (self.yof & !ORDINAL_MASK) | (ordinal << 4) as i32 })
Some(NaiveDate::from_yof((self.yof() & !ORDINAL_MASK) | (ordinal << 4) as i32))
}
None => None, // Non-existing date
}
Expand Down Expand Up @@ -986,9 +987,9 @@ impl NaiveDate {
#[inline]
#[must_use]
pub const fn succ_opt(&self) -> Option<NaiveDate> {
let new_ol = (self.yof & OL_MASK) + (1 << 4);
let new_ol = (self.yof() & OL_MASK) + (1 << 4);
match new_ol <= MAX_OL {
true => Some(NaiveDate { yof: self.yof & !OL_MASK | new_ol }),
true => Some(NaiveDate::from_yof(self.yof() & !OL_MASK | new_ol)),
false => NaiveDate::from_yo_opt(self.year() + 1, 1),
}
}
Expand Down Expand Up @@ -1023,9 +1024,9 @@ impl NaiveDate {
#[inline]
#[must_use]
pub const fn pred_opt(&self) -> Option<NaiveDate> {
let new_shifted_ordinal = (self.yof & ORDINAL_MASK) - (1 << 4);
let new_shifted_ordinal = (self.yof() & ORDINAL_MASK) - (1 << 4);
match new_shifted_ordinal > 0 {
true => Some(NaiveDate { yof: self.yof & !ORDINAL_MASK | new_shifted_ordinal }),
true => Some(NaiveDate::from_yof(self.yof() & !ORDINAL_MASK | new_shifted_ordinal)),
false => NaiveDate::from_ymd_opt(self.year() - 1, 12, 31),
}
}
Expand Down Expand Up @@ -1330,20 +1331,20 @@ impl NaiveDate {
/// assert_eq!(NaiveDate::from_ymd_opt(2100, 1, 1).unwrap().leap_year(), false);
/// ```
pub const fn leap_year(&self) -> bool {
self.yof & (0b1000) == 0
self.yof() & (0b1000) == 0
}

// This duplicates `Datelike::year()`, because trait methods can't be const yet.
#[inline]
const fn year(&self) -> i32 {
self.yof >> 13
self.yof() >> 13
}

/// Returns the day of year starting from 1.
// This duplicates `Datelike::ordinal()`, because trait methods can't be const yet.
#[inline]
const fn ordinal(&self) -> u32 {
((self.yof & ORDINAL_MASK) >> 4) as u32
((self.yof() & ORDINAL_MASK) >> 4) as u32
}

// This duplicates `Datelike::month()`, because trait methods can't be const yet.
Expand All @@ -1362,7 +1363,7 @@ impl NaiveDate {
// This duplicates `Datelike::weekday()`, because trait methods can't be const yet.
#[inline]
pub(super) const fn weekday(&self) -> Weekday {
match (((self.yof & ORDINAL_MASK) >> 4) + (self.yof & WEEKDAY_FLAGS_MASK)) % 7 {
match (((self.yof() & ORDINAL_MASK) >> 4) + (self.yof() & WEEKDAY_FLAGS_MASK)) % 7 {
0 => Weekday::Mon,
1 => Weekday::Tue,
2 => Weekday::Wed,
Expand All @@ -1375,7 +1376,7 @@ impl NaiveDate {

#[inline]
const fn year_flags(&self) -> YearFlags {
YearFlags((self.yof & YEAR_FLAGS_MASK) as u8)
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.
Expand All @@ -1394,17 +1395,32 @@ impl NaiveDate {
ndays + self.ordinal() as i32
}

/// Create a new `NaiveDate` from a raw year-ordinal-flags `i32`.
///
/// In a valid value an ordinal is never `0`, and neither are the year flags. This method
/// doesn't do any validation; it only panics if the value is `0`.
#[inline]
const fn from_yof(yof: i32) -> NaiveDate {
NaiveDate { yof: expect!(NonZeroI32::new(yof), "invalid internal value") }
}

/// Get the raw year-ordinal-flags `i32`.
#[inline]
const fn yof(&self) -> i32 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should #[inline] this.

self.yof.get()
}

/// The minimum possible `NaiveDate` (January 1, 262144 BCE).
pub const MIN: NaiveDate = NaiveDate { yof: (MIN_YEAR << 13) | (1 << 4) | 0o12 /*D*/ };
pub const MIN: NaiveDate = NaiveDate::from_yof((MIN_YEAR << 13) | (1 << 4) | 0o12 /*D*/);
/// The maximum possible `NaiveDate` (December 31, 262142 CE).
pub const MAX: NaiveDate = NaiveDate { yof: (MAX_YEAR << 13) | (365 << 4) | 0o16 /*G*/ };
pub const MAX: NaiveDate = NaiveDate::from_yof((MAX_YEAR << 13) | (365 << 4) | 0o16 /*G*/);

/// One day before the minimum possible `NaiveDate` (December 31, 262145 BCE).
pub(crate) const BEFORE_MIN: NaiveDate =
NaiveDate { yof: ((MIN_YEAR - 1) << 13) | (366 << 4) | 0o07 /*FE*/ };
NaiveDate::from_yof(((MIN_YEAR - 1) << 13) | (366 << 4) | 0o07 /*FE*/);
/// One day after the maximum possible `NaiveDate` (January 1, 262143 CE).
pub(crate) const AFTER_MAX: NaiveDate =
NaiveDate { yof: ((MAX_YEAR + 1) << 13) | (1 << 4) | 0o17 /*F*/ };
NaiveDate::from_yof(((MAX_YEAR + 1) << 13) | (1 << 4) | 0o17 /*F*/);
}

impl Datelike for NaiveDate {
Expand Down Expand Up @@ -1550,7 +1566,7 @@ impl Datelike for NaiveDate {
/// ```
#[inline]
fn ordinal(&self) -> u32 {
((self.yof & ORDINAL_MASK) >> 4) as u32
((self.yof() & ORDINAL_MASK) >> 4) as u32
}

/// Returns the day of year starting from 0.
Expand Down Expand Up @@ -1741,9 +1757,9 @@ impl Datelike for NaiveDate {
if ordinal == 0 || ordinal > 366 {
return None;
}
let yof = (self.yof & !ORDINAL_MASK) | (ordinal << 4) as i32;
let yof = (self.yof() & !ORDINAL_MASK) | (ordinal << 4) as i32;
match yof & OL_MASK <= MAX_OL {
true => Some(NaiveDate { yof }),
true => Some(NaiveDate::from_yof(yof)),
false => None, // Does not exist: Ordinal 366 in a common year.
}
}
Expand Down