Skip to content

Commit

Permalink
Ensure Of is always valid
Browse files Browse the repository at this point in the history
  • Loading branch information
@pitdicker
pitdicker committed May 15, 2023
1 parent 616a90c commit 261ec4d
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 61 deletions.
28 changes: 15 additions & 13 deletions src/naive/date.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -924,28 +924,24 @@ impl NaiveDate {
/// Returns the packed ordinal-flags.
#[inline]
const fn of(&self) -> Of {
Of((self.ymdf & 0b1_1111_1111_1111) as u32)
Of::from_date_impl(self.ymdf)
}

/// Makes a new `NaiveDate` with the packed month-day-flags changed.
///
/// Returns `None` when the resulting `NaiveDate` would be invalid.
#[inline]
fn with_mdf(&self, mdf: Mdf) -> Option<NaiveDate> {
self.with_of(mdf.to_of())
Some(self.with_of(mdf.to_of()?))
}

/// Makes a new `NaiveDate` with the packed ordinal-flags changed.
///
/// Returns `None` when the resulting `NaiveDate` would be invalid.
/// Does not check if the year flags match the year.
#[inline]
fn with_of(&self, of: Of) -> Option<NaiveDate> {
if of.valid() {
let Of(of) = of;
Some(NaiveDate { ymdf: (self.ymdf & !0b1_1111_1111_1111) | of as DateImpl })
} else {
None
}
const fn with_of(&self, of: Of) -> NaiveDate {
NaiveDate { ymdf: (self.ymdf & !0b1_1111_1111_1111) | of.inner() as DateImpl }
}

/// Makes a new `NaiveDate` for the next calendar date.
Expand Down Expand Up @@ -974,7 +970,10 @@ impl NaiveDate {
#[inline]
#[must_use]
pub fn succ_opt(&self) -> Option<NaiveDate> {
self.with_of(self.of().succ()).or_else(|| NaiveDate::from_ymd_opt(self.year() + 1, 1, 1))
match self.of().succ() {
Some(of) => Some(self.with_of(of)),
None => NaiveDate::from_ymd_opt(self.year() + 1, 1, 1),
}
}

/// Makes a new `NaiveDate` for the previous calendar date.
Expand Down Expand Up @@ -1003,7 +1002,10 @@ impl NaiveDate {
#[inline]
#[must_use]
pub fn pred_opt(&self) -> Option<NaiveDate> {
self.with_of(self.of().pred()).or_else(|| NaiveDate::from_ymd_opt(self.year() - 1, 12, 31))
match self.of().pred() {
Some(of) => Some(self.with_of(of)),
None => NaiveDate::from_ymd_opt(self.year() - 1, 12, 31),
}
}

/// Adds the `days` part of given `Duration` to the current date.
Expand Down Expand Up @@ -1622,7 +1624,7 @@ impl Datelike for NaiveDate {
/// ```
#[inline]
fn with_ordinal(&self, ordinal: u32) -> Option<NaiveDate> {
self.with_of(self.of().with_ordinal(ordinal)?)
self.of().with_ordinal(ordinal).map(|of| self.with_of(of))
}

/// Makes a new `NaiveDate` with the day of year (starting from 0) changed.
Expand All @@ -1647,7 +1649,7 @@ impl Datelike for NaiveDate {
#[inline]
fn with_ordinal0(&self, ordinal0: u32) -> Option<NaiveDate> {
let ordinal = ordinal0.checked_add(1)?;
self.with_of(self.of().with_ordinal(ordinal)?)
self.with_ordinal(ordinal)
}
}

Expand Down
143 changes: 95 additions & 48 deletions src/naive/internals.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
use crate::Weekday;
use core::{fmt, i32};

/// The internal date representation. This also includes the packed `Mdf` value.
/// The internal date representation: `year << 13 | Of`
pub(super) type DateImpl = i32;

pub(super) const MAX_YEAR: DateImpl = i32::MAX >> 13;
Expand Down Expand Up @@ -172,8 +172,9 @@ impl fmt::Debug for YearFlags {
}
}

// OL: (ordinal << 1) | leap year flag
pub(super) const MIN_OL: u32 = 1 << 1;
pub(super) const MAX_OL: u32 = 366 << 1; // larger than the non-leap last day `(365 << 1) | 1`
pub(super) const MAX_OL: u32 = 366 << 1; // `(366 << 1) | 1` would be day 366 in a non-leap year
pub(super) const MAX_MDL: u32 = (12 << 6) | (31 << 1) | 1;

const XX: i8 = -128;
Expand Down Expand Up @@ -265,20 +266,25 @@ 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.
#[derive(PartialEq, PartialOrd, Copy, Clone)]
pub(super) struct Of(pub(crate) u32);
pub(super) struct Of(u32);

impl Of {
#[inline]
pub(super) const fn new(ordinal: u32, YearFlags(flags): YearFlags) -> Option<Of> {
match ordinal <= 366 {
true => Some(Of((ordinal << 4) | flags as u32)),
false => None,
}
let of = Of((ordinal << 4) | flags as u32);
of.validate()
}

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

#[inline]
pub(super) const fn from_mdf(Mdf(mdf): Mdf) -> Of {
pub(super) const fn from_mdf(Mdf(mdf): Mdf) -> Option<Of> {
let mdl = mdf >> 3;
if mdl <= MAX_MDL {
// Array is indexed from `[1..=MAX_MDL]`, with a `0` index having a meaningless value.
Expand All @@ -288,35 +294,45 @@ impl Of {
// Panicking here would be reasonable, but we are just going on with a safe value.
Of(0)
}
let v = MDL_TO_OL[mdl as usize];
let of = Of(mdf.wrapping_sub((v as i32 as u32 & 0x3ff) << 3));
of.validate()
}

#[inline]
pub(super) const fn valid(&self) -> bool {
let Of(of) = *self;
let ol = of >> 3;
ol >= MIN_OL && ol <= MAX_OL
pub(super) const fn inner(&self) -> u32 {
self.0
}

/// Returns `(ordinal << 1) | leap-year-flag`.
#[inline]
pub(super) const fn ordinal(&self) -> u32 {
let Of(of) = *self;
of >> 4
const fn ol(&self) -> u32 {
self.0 >> 3
}

#[inline]
pub(super) const fn with_ordinal(&self, ordinal: u32) -> Option<Of> {
if ordinal > 366 {
return None;
const fn validate(self) -> Option<Of> {
let ol = self.ol();
match ol >= MIN_OL && ol <= MAX_OL {
true => Some(self),
false => None,
}
}

let Of(of) = *self;
Some(Of((of & 0b1111) | (ordinal << 4)))
#[inline]
pub(super) const fn ordinal(&self) -> u32 {
self.0 >> 4
}

#[inline]
pub(super) const fn with_ordinal(&self, ordinal: u32) -> Option<Of> {
let of = Of((ordinal << 4) | (self.0 & 0b1111));
of.validate()
}

#[inline]
pub(super) const fn flags(&self) -> YearFlags {
let Of(of) = *self;
YearFlags((of & 0b1111) as u8)
YearFlags((self.0 & 0b1111) as u8)
}

#[inline]
Expand All @@ -339,16 +355,20 @@ impl Of {
Mdf::from_of(*self)
}

/// Returns an `Of` with the next day, or `None` if this is the last day of the year.
#[inline]
pub(super) const fn succ(&self) -> Of {
let Of(of) = *self;
Of(of + (1 << 4))
pub(super) const fn succ(&self) -> Option<Of> {
let of = Of(self.0 + (1 << 4));
of.validate()
}

/// Returns an `Of` with the previous day, or `None` if this is the first day of the year.
#[inline]
pub(super) const fn pred(&self) -> Of {
let Of(of) = *self;
Of(of - (1 << 4))
pub(super) const fn pred(&self) -> Option<Of> {
if self.ordinal() == 1 {
return None;
}
Some(Of(self.0 - (1 << 4)))
}
}

Expand All @@ -370,13 +390,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.
/// Use `Mdl::valid` to check whether the date is valid.
#[derive(PartialEq, PartialOrd, Copy, Clone)]
pub(super) struct Mdf(u32);

impl Mdf {
#[inline]
pub(super) const fn new(month: u32, day: u32, YearFlags(flags): YearFlags) -> Option<Mdf> {
match month <= 12 && day <= 31 {
match month >= 1 && month <= 12 && day >= 1 && day <= 31 {
true => Some(Mdf((month << 9) | (day << 4) | flags as u32)),
false => None,
}
Expand Down Expand Up @@ -447,7 +471,7 @@ impl Mdf {

#[cfg_attr(feature = "cargo-clippy", allow(clippy::wrong_self_convention))]
#[inline]
pub(super) const fn to_of(&self) -> Of {
pub(super) const fn to_of(&self) -> Option<Of> {
Of::from_mdf(*self)
}
}
Expand Down Expand Up @@ -542,7 +566,7 @@ mod tests {
};

assert!(
of.valid() == expected,
of.validate().is_some() == expected,
"ordinal {} = {:?} should be {} for dominical year {:?}",
ordinal,
of,
Expand Down Expand Up @@ -662,8 +686,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) {
assert_eq!(of.ordinal(), ordinal);
}
}
Expand All @@ -676,14 +699,9 @@ mod tests {
let of = Of::new(ordinal, flags).unwrap();

for ordinal in range_inclusive(0u32, 1024) {
let of = match of.with_ordinal(ordinal) {
Some(of) => of,
None if ordinal > 366 => continue,
None => panic!("failed to create Of with ordinal {}", ordinal),
};

assert_eq!(of.valid(), Of::new(ordinal, flags).unwrap().valid());
if of.valid() {
let of = of.with_ordinal(ordinal);
assert_eq!(of, Of::new(ordinal, flags));
if let Some(of) = of {
assert_eq!(of.ordinal(), ordinal);
}
}
Expand Down Expand Up @@ -808,25 +826,25 @@ mod tests {
#[test]
fn test_of_to_mdf() {
for i in range_inclusive(0u32, 8192) {
let of = Of(i);
assert_eq!(of.valid(), of.to_mdf().valid());
if let Some(of) = Of(i).validate() {
assert!(of.to_mdf().valid());
}
}
}

#[test]
fn test_mdf_to_of() {
for i in range_inclusive(0u32, 8192) {
let mdf = Mdf(i);
assert_eq!(mdf.valid(), mdf.to_of().valid());
assert_eq!(mdf.valid(), mdf.to_of().is_some());
}
}

#[test]
fn test_of_to_mdf_to_of() {
for i in range_inclusive(0u32, 8192) {
let of = Of(i);
if of.valid() {
assert_eq!(of, of.to_mdf().to_of());
if let Some(of) = Of(i).validate() {
assert_eq!(of, of.to_mdf().to_of().unwrap());
}
}
}
Expand All @@ -836,11 +854,40 @@ mod tests {
for i in range_inclusive(0u32, 8192) {
let mdf = Mdf(i);
if mdf.valid() {
assert_eq!(mdf, mdf.to_of().to_mdf());
assert_eq!(mdf, mdf.to_of().unwrap().to_mdf());
}
}
}

#[test]
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());
assert!(Mdf::new(1, 32, regular_year).is_none());
assert!(Mdf::new(2, 31, regular_year).is_some());

assert!(Of::from_mdf(Mdf::new(2, 30, regular_year).unwrap()).is_none());
assert!(Of::from_mdf(Mdf::new(2, 30, leap_year).unwrap()).is_none());
assert!(Of::from_mdf(Mdf::new(2, 29, regular_year).unwrap()).is_none());
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());

assert!(Of::new(365, regular_year).unwrap().succ().is_none());
assert!(Of::new(365, leap_year).unwrap().succ().is_some());
assert!(Of::new(366, leap_year).unwrap().succ().is_none());

assert!(Of::new(1, regular_year).unwrap().pred().is_none());
assert!(Of::new(1, leap_year).unwrap().pred().is_none());
}

#[test]
fn test_weekday_from_u32_mod7() {
for i in 0..=1000 {
Expand Down

0 comments on commit 261ec4d

Please sign in to comment.