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 committed Apr 30, 2023
1 parent 9167548 commit 6540cbf
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 83 deletions.
73 changes: 41 additions & 32 deletions src/naive/date.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,19 +242,29 @@ impl NaiveDate {
pub(crate) fn weeks_from(&self, day: Weekday) -> i32 {
(self.ordinal() as i32 - self.weekday().num_days_from(day) as i32 + 6) / 7
}
/// Makes a new `NaiveDate` from year and packed ordinal-flags, with a verification.
fn from_of(year: i32, of: Of) -> Option<NaiveDate> {
if (MIN_YEAR..=MAX_YEAR).contains(&year) && of.valid() {
let Of(of) = of;
Some(NaiveDate { ymdf: (year << 13) | (of as DateImpl) })
} else {
None

/// 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> {
if year < MIN_YEAR || year > MAX_YEAR {
return None; // Out-of-range
}
match Of::new(ordinal, flags) {
Some(of) => Some(NaiveDate { ymdf: (year << 13) | (of.inner() as DateImpl) }),
None => None, // Invalid: Ordinal outside of the nr of days in a year with those flags.
}
}

/// Makes a new `NaiveDate` from year and packed month-day-flags, with a verification.
fn from_mdf(year: i32, mdf: Mdf) -> Option<NaiveDate> {
NaiveDate::from_of(year, mdf.to_of())
/// Makes a new `NaiveDate` from year and packed month-day-flags.
/// Does not check whether the flags are correct for the provided year.
const fn from_mdf(year: i32, mdf: Mdf) -> Option<NaiveDate> {
if year < MIN_YEAR || year > MAX_YEAR {
return None; // Out-of-range
}
match mdf.to_of() {
Some(of) => Some(NaiveDate { ymdf: (year << 13) | (of.inner() as DateImpl) }),
None => None, // Non-existing date
}
}

/// Makes a new `NaiveDate` from the [calendar date](#calendar-date)
Expand Down Expand Up @@ -325,7 +335,7 @@ impl NaiveDate {
#[must_use]
pub fn from_yo_opt(year: i32, ordinal: u32) -> Option<NaiveDate> {
let flags = YearFlags::from_year(year);
NaiveDate::from_of(year, Of::new(ordinal, flags)?)
NaiveDate::from_of(year, ordinal, flags)
}

/// Makes a new `NaiveDate` from the [ISO week date](#week-date)
Expand Down Expand Up @@ -394,20 +404,17 @@ impl NaiveDate {
if weekord <= delta {
// ordinal < 1, previous year
let prevflags = YearFlags::from_year(year - 1);
NaiveDate::from_of(
year - 1,
Of::new(weekord + prevflags.ndays() - delta, prevflags)?,
)
NaiveDate::from_of(year - 1, weekord + prevflags.ndays() - delta, prevflags)
} else {
let ordinal = weekord - delta;
let ndays = flags.ndays();
if ordinal <= ndays {
// this year
NaiveDate::from_of(year, Of::new(ordinal, flags)?)
NaiveDate::from_of(year, ordinal, flags)
} else {
// ordinal > ndays, next year
let nextflags = YearFlags::from_year(year + 1);
NaiveDate::from_of(year + 1, Of::new(ordinal - ndays, nextflags)?)
NaiveDate::from_of(year + 1, ordinal - ndays, nextflags)
}
}
} else {
Expand Down Expand Up @@ -452,7 +459,7 @@ impl NaiveDate {
let (year_div_400, cycle) = div_mod_floor(days, 146_097);
let (year_mod_400, ordinal) = internals::cycle_to_yo(cycle as u32);
let flags = YearFlags::from_year_mod_400(year_mod_400 as i32);
NaiveDate::from_of(year_div_400 * 400 + year_mod_400 as i32, Of::new(ordinal, flags)?)
NaiveDate::from_of(year_div_400 * 400 + year_mod_400 as i32, ordinal, flags)
}

/// Makes a new `NaiveDate` by counting the number of occurrences of a particular day-of-week
Expand Down Expand Up @@ -933,20 +940,16 @@ impl NaiveDate {
/// 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 @@ -975,7 +978,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 @@ -1004,7 +1010,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 @@ -1036,7 +1045,7 @@ impl NaiveDate {

let (year_mod_400, ordinal) = internals::cycle_to_yo(cycle as u32);
let flags = YearFlags::from_year_mod_400(year_mod_400 as i32);
NaiveDate::from_of(year_div_400 * 400 + year_mod_400 as i32, Of::new(ordinal, flags)?)
NaiveDate::from_of(year_div_400 * 400 + year_mod_400 as i32, ordinal, flags)
}

/// Subtracts the `days` part of given `Duration` from the current date.
Expand Down Expand Up @@ -1068,7 +1077,7 @@ impl NaiveDate {

let (year_mod_400, ordinal) = internals::cycle_to_yo(cycle as u32);
let flags = YearFlags::from_year_mod_400(year_mod_400 as i32);
NaiveDate::from_of(year_div_400 * 400 + year_mod_400 as i32, Of::new(ordinal, flags)?)
NaiveDate::from_of(year_div_400 * 400 + year_mod_400 as i32, ordinal, flags)
}

/// Subtracts another `NaiveDate` from the current date.
Expand Down Expand Up @@ -1623,7 +1632,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 @@ -1648,7 +1657,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
106 changes: 55 additions & 51 deletions src/naive/internals.rs
Original file line number Diff line number Diff line change
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 @@ -271,51 +272,56 @@ pub(super) struct Of(pub(crate) 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()
}

#[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 {
let v = MDL_TO_OL[mdl as usize];
Of(mdf.wrapping_sub((v as i32 as u32 & 0x3ff) << 3))
} else {
// Panicking here would be reasonable, but we are just going on with a safe value.
Of(0)
if mdl > MAX_MDL {
// Panicking on out-of-bounds indexing would be reasonable, but just return `None`.
return None;
}
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
pub(super) 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 @@ -338,16 +344,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 Down Expand Up @@ -444,7 +454,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 @@ -522,7 +532,7 @@ mod tests {
};

assert!(
of.valid() == expected,
of.validate().is_some() == expected,
"ordinal {} = {:?} should be {} for dominical year {:?}",
ordinal,
of,
Expand Down Expand Up @@ -642,8 +652,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 @@ -656,14 +665,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 @@ -788,25 +792,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 @@ -816,7 +820,7 @@ 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());
}
}
}
Expand Down

0 comments on commit 6540cbf

Please sign in to comment.