From e758ff9fec48bc0c4668807e12795707069666a7 Mon Sep 17 00:00:00 2001 From: Mikhail Katychev Date: Mon, 9 Oct 2023 13:44:25 -0500 Subject: [PATCH 1/6] * added `rkyv-validation` feature flag * specified latest hotfix version of `rkyv` that includes `bytecheck` reexport --- Cargo.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 7d82ba3eca..c6edb2a311 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,6 +26,7 @@ clock = ["std", "winapi", "iana-time-zone", "android-tzdata"] oldtime = [] wasmbind = ["wasm-bindgen", "js-sys"] unstable-locales = ["pure-rust-locales"] +rkyv-validation = ["rkyv/validation"] __internal_bench = [] [dependencies] @@ -33,7 +34,7 @@ num-traits = { version = "0.2", default-features = false } rustc-serialize = { version = "0.3.20", optional = true } serde = { version = "1.0.99", default-features = false, optional = true } pure-rust-locales = { version = "0.7", optional = true } -rkyv = { version = "0.7", optional = true } +rkyv = { version = "0.7.41", optional = true } arbitrary = { version = "1.0.0", features = ["derive"], optional = true } [target.'cfg(all(target_arch = "wasm32", not(any(target_os = "emscripten", target_os = "wasi"))))'.dependencies] From 3faca1bf7b3631af1f4557f4bd80900e467c414a Mon Sep 17 00:00:00 2001 From: Mikhail Katychev Date: Mon, 9 Oct 2023 14:05:30 -0500 Subject: [PATCH 2/6] * added `archive(compare(PartialEq/PartialOrd))` derives to `rkyv` feature flags * added `archive(check_bytes)` derives `rkyv-validation` feature flags --- src/datetime/mod.rs | 2 ++ src/duration.rs | 2 ++ src/month.rs | 2 ++ src/naive/date.rs | 2 ++ src/naive/datetime/mod.rs | 2 ++ src/naive/isoweek.rs | 2 ++ src/naive/time/mod.rs | 2 ++ src/offset/fixed.rs | 7 ++++++- src/offset/local/mod.rs | 3 ++- src/offset/utc.rs | 7 ++++++- src/weekday.rs | 7 ++++++- 11 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/datetime/mod.rs b/src/datetime/mod.rs index b26296823e..50895d48b7 100644 --- a/src/datetime/mod.rs +++ b/src/datetime/mod.rs @@ -51,6 +51,8 @@ mod tests; /// [`TimeZone`](./offset/trait.TimeZone.html) implementations. #[derive(Clone)] #[cfg_attr(feature = "rkyv", derive(Archive, Deserialize, Serialize))] +#[cfg_attr(feature = "rkyv", archive(compare(PartialEq, PartialOrd)))] +#[cfg_attr(feature = "rkyv-validation", archive(check_bytes))] pub struct DateTime { datetime: NaiveDateTime, offset: Tz::Offset, diff --git a/src/duration.rs b/src/duration.rs index 1e5eb67512..e323e6fb91 100644 --- a/src/duration.rs +++ b/src/duration.rs @@ -54,8 +54,10 @@ macro_rules! try_opt { #[cfg_attr(feature = "rkyv", derive(Archive, Deserialize, Serialize))] #[cfg_attr( feature = "rkyv", + archive(compare(PartialEq, PartialOrd)), archive_attr(derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)) )] +#[cfg_attr(feature = "rkyv-validation", archive(check_bytes))] pub struct Duration { secs: i64, nanos: i32, // Always 0 <= nanos < NANOS_PER_SEC diff --git a/src/month.rs b/src/month.rs index a409bb155d..7a8180bc9b 100644 --- a/src/month.rs +++ b/src/month.rs @@ -33,8 +33,10 @@ use crate::OutOfRange; #[cfg_attr(feature = "rkyv", derive(Archive, Deserialize, Serialize))] #[cfg_attr( feature = "rkyv", + archive(compare(PartialEq)), archive_attr(derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)) )] +#[cfg_attr(feature = "rkyv-validation", archive(check_bytes))] #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub enum Month { /// January diff --git a/src/naive/date.rs b/src/naive/date.rs index 69f413edb7..9874bb1f60 100644 --- a/src/naive/date.rs +++ b/src/naive/date.rs @@ -192,8 +192,10 @@ impl Days { #[cfg_attr(feature = "rkyv", derive(Archive, Deserialize, Serialize))] #[cfg_attr( feature = "rkyv", + archive(compare(PartialEq, PartialOrd)), archive_attr(derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)) )] +#[cfg_attr(feature = "rkyv-validation", archive(check_bytes))] pub struct NaiveDate { ymdf: DateImpl, // (year << 13) | of } diff --git a/src/naive/datetime/mod.rs b/src/naive/datetime/mod.rs index 88d780a4c1..2f9b2db6d1 100644 --- a/src/naive/datetime/mod.rs +++ b/src/naive/datetime/mod.rs @@ -77,8 +77,10 @@ pub const MAX_DATETIME: NaiveDateTime = NaiveDateTime::MAX; #[cfg_attr(feature = "rkyv", derive(Archive, Deserialize, Serialize))] #[cfg_attr( feature = "rkyv", + archive(compare(PartialEq, PartialOrd)), archive_attr(derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)) )] +#[cfg_attr(feature = "rkyv-validation", archive(check_bytes))] #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub struct NaiveDateTime { date: NaiveDate, diff --git a/src/naive/isoweek.rs b/src/naive/isoweek.rs index 606699659b..237d841f9e 100644 --- a/src/naive/isoweek.rs +++ b/src/naive/isoweek.rs @@ -20,8 +20,10 @@ use rkyv::{Archive, Deserialize, Serialize}; #[cfg_attr(feature = "rkyv", derive(Archive, Deserialize, Serialize))] #[cfg_attr( feature = "rkyv", + archive(compare(PartialEq)), archive_attr(derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)) )] +#[cfg_attr(feature = "rkyv-validation", archive(check_bytes))] pub struct IsoWeek { // note that this allows for larger year range than `NaiveDate`. // this is crucial because we have an edge case for the first and last week supported, diff --git a/src/naive/time/mod.rs b/src/naive/time/mod.rs index de29fa465e..9a2e5d8f76 100644 --- a/src/naive/time/mod.rs +++ b/src/naive/time/mod.rs @@ -206,8 +206,10 @@ mod tests; #[cfg_attr(feature = "rkyv", derive(Archive, Deserialize, Serialize))] #[cfg_attr( feature = "rkyv", + archive(compare(PartialEq, PartialOrd)), archive_attr(derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)) )] +#[cfg_attr(feature = "rkyv-validation", archive(check_bytes))] pub struct NaiveTime { secs: u32, frac: u32, diff --git a/src/offset/fixed.rs b/src/offset/fixed.rs index 6d3aebdb68..806d13cca9 100644 --- a/src/offset/fixed.rs +++ b/src/offset/fixed.rs @@ -21,7 +21,12 @@ use crate::naive::{NaiveDate, NaiveDateTime}; /// [`west_opt`](#method.west_opt) methods for examples. #[derive(PartialEq, Eq, Hash, Copy, Clone)] #[cfg_attr(feature = "rkyv", derive(Archive, Deserialize, Serialize))] -#[cfg_attr(feature = "rkyv", archive_attr(derive(Clone, Copy, PartialEq, Eq, Hash, Debug)))] +#[cfg_attr( + feature = "rkyv", + archive(compare(PartialEq)), + archive_attr(derive(Clone, Copy, PartialEq, Eq, Hash, Debug)) +)] +#[cfg_attr(feature = "rkyv-validation", archive(check_bytes))] pub struct FixedOffset { local_minus_utc: i32, } diff --git a/src/offset/local/mod.rs b/src/offset/local/mod.rs index e4f4e1631f..91a8baed62 100644 --- a/src/offset/local/mod.rs +++ b/src/offset/local/mod.rs @@ -105,7 +105,8 @@ mod tz_info; /// ``` #[derive(Copy, Clone, Debug)] #[cfg_attr(feature = "rkyv", derive(Archive, Deserialize, Serialize))] -#[cfg_attr(feature = "rkyv", archive_attr(derive(Clone, Copy, Debug)))] +#[cfg_attr(feature = "rkyv", archive(compare(PartialEq)), archive_attr(derive(Clone, Copy, Debug)))] +#[cfg_attr(feature = "rkyv-validation", archive(check_bytes))] #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub struct Local; diff --git a/src/offset/utc.rs b/src/offset/utc.rs index 3b0a85d56a..11e9dad931 100644 --- a/src/offset/utc.rs +++ b/src/offset/utc.rs @@ -42,7 +42,12 @@ use crate::{Date, DateTime}; /// ``` #[derive(Copy, Clone, PartialEq, Eq, Hash)] #[cfg_attr(feature = "rkyv", derive(Archive, Deserialize, Serialize))] -#[cfg_attr(feature = "rkyv", archive_attr(derive(Clone, Copy, PartialEq, Eq, Debug, Hash)))] +#[cfg_attr( + feature = "rkyv", + archive(compare(PartialEq)), + archive_attr(derive(Clone, Copy, PartialEq, Eq, Debug, Hash)) +)] +#[cfg_attr(feature = "rkyv-validation", archive(check_bytes))] #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub struct Utc; diff --git a/src/weekday.rs b/src/weekday.rs index fb88c3b929..444cfbaed4 100644 --- a/src/weekday.rs +++ b/src/weekday.rs @@ -32,7 +32,12 @@ use crate::OutOfRange; #[derive(PartialEq, Eq, Copy, Clone, Debug, Hash)] #[cfg_attr(feature = "rustc-serialize", derive(RustcEncodable, RustcDecodable))] #[cfg_attr(feature = "rkyv", derive(Archive, Deserialize, Serialize))] -#[cfg_attr(feature = "rkyv", archive_attr(derive(Clone, Copy, PartialEq, Eq, Debug, Hash)))] +#[cfg_attr( + feature = "rkyv", + archive(compare(PartialEq)), + archive_attr(derive(Clone, Copy, PartialEq, Eq, Debug, Hash)) +)] +#[cfg_attr(feature = "rkyv-validation", archive(check_bytes))] #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub enum Weekday { /// Monday. From 57c2fbd7de7c552d1d1151a21867565f64fc0a2e Mon Sep 17 00:00:00 2001 From: Mikhail Katychev Date: Mon, 9 Oct 2023 14:09:55 -0500 Subject: [PATCH 3/6] hand implemented `fmt::Debug` for `ArchivedDateTime` --- src/datetime/mod.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/datetime/mod.rs b/src/datetime/mod.rs index 50895d48b7..ac9ef511d0 100644 --- a/src/datetime/mod.rs +++ b/src/datetime/mod.rs @@ -1506,6 +1506,31 @@ impl fmt::Debug for DateTime { } } +// `fmt::Debug` is hand implemented for the `rkyv::Archive` variant of `DateTime` because +// deriving a trait recursively does not propagate trait defined associated types with their own +// constraints: +// In our case `<::Offset as Archive>::Archived` +// cannot be formatted using `{:?}` because it doesn't implement `Debug`. +// See below for further discussion: +// * https://github.com/rust-lang/rust/issues/26925 +// * https://github.com/rkyv/rkyv/issues/333 +// * https://github.com/dtolnay/syn/issues/370 +#[cfg(feature = "rkyv-validation")] +impl fmt::Debug for ArchivedDateTime +where + Tz: Archive, + ::Archived: fmt::Debug, + <::Offset as Archive>::Archived: fmt::Debug, + ::Offset: fmt::Debug + Archive, +{ + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.debug_struct("ArchivedDateTime") + .field("datetime", &self.datetime) + .field("offset", &self.offset) + .finish() + } +} + impl fmt::Display for DateTime where Tz::Offset: fmt::Display, From f6f2eddcc9c96246957127c1709a8b6661859883 Mon Sep 17 00:00:00 2001 From: Mikhail Katychev Date: Mon, 9 Oct 2023 14:11:30 -0500 Subject: [PATCH 4/6] Added unit tests for rkyv/rkyv-validation: chrono month::tests::test_rkyv_validation chrono naive::date::tests::test_rkyv_validation chrono duration::tests::test_rkyv_validation chrono naive::datetime::tests::test_rkyv_validation chrono naive::isoweek::tests::test_rkyv_validation chrono naive::time::tests::test_rkyv_validation chrono offset::fixed::tests::test_rkyv_validation chrono offset::local::tests::test_rkyv_validation chrono weekday::tests::test_rkyv_validation --- src/duration.rs | 8 ++++++++ src/month.rs | 8 ++++++++ src/naive/date.rs | 12 ++++++++++++ src/naive/datetime/tests.rs | 12 ++++++++++++ src/naive/isoweek.rs | 14 ++++++++++++++ src/naive/time/tests.rs | 12 ++++++++++++ src/offset/fixed.rs | 8 ++++++++ src/offset/local/mod.rs | 13 +++++++++++++ src/weekday.rs | 9 +++++++++ 9 files changed, 96 insertions(+) diff --git a/src/duration.rs b/src/duration.rs index e323e6fb91..f00b03663e 100644 --- a/src/duration.rs +++ b/src/duration.rs @@ -797,4 +797,12 @@ mod tests { Err(OutOfRangeError(())) ); } + + #[test] + #[cfg(feature = "rkyv-validation")] + fn test_rkyv_validation() { + let duration = Duration::seconds(1); + let bytes = rkyv::to_bytes::<_, 16>(&duration).unwrap(); + assert_eq!(rkyv::from_bytes::(&bytes).unwrap(), duration); + } } diff --git a/src/month.rs b/src/month.rs index 7a8180bc9b..26597bd4b0 100644 --- a/src/month.rs +++ b/src/month.rs @@ -419,4 +419,12 @@ mod tests { from_str::(string).unwrap_err(); } } + + #[test] + #[cfg(feature = "rkyv-validation")] + fn test_rkyv_validation() { + let month = Month::January; + let bytes = rkyv::to_bytes::<_, 1>(&month).unwrap(); + assert_eq!(rkyv::from_bytes::(&bytes).unwrap(), month); + } } diff --git a/src/naive/date.rs b/src/naive/date.rs index 9874bb1f60..11952e2766 100644 --- a/src/naive/date.rs +++ b/src/naive/date.rs @@ -3326,6 +3326,18 @@ mod tests { } } + #[test] + #[cfg(feature = "rkyv-validation")] + fn test_rkyv_validation() { + let date_min = NaiveDate::MIN; + let bytes = rkyv::to_bytes::<_, 4>(&date_min).unwrap(); + assert_eq!(rkyv::from_bytes::(&bytes).unwrap(), date_min); + + let date_max = NaiveDate::MAX; + let bytes = rkyv::to_bytes::<_, 4>(&date_max).unwrap(); + assert_eq!(rkyv::from_bytes::(&bytes).unwrap(), date_max); + } + // MAX_YEAR-12-31 minus 0000-01-01 // = (MAX_YEAR-12-31 minus 0000-12-31) + (0000-12-31 - 0000-01-01) // = MAX_YEAR * 365 + (# of leap years from 0001 to MAX_YEAR) + 365 diff --git a/src/naive/datetime/tests.rs b/src/naive/datetime/tests.rs index 97604db74f..0dbbe7bc5e 100644 --- a/src/naive/datetime/tests.rs +++ b/src/naive/datetime/tests.rs @@ -536,3 +536,15 @@ fn test_and_timezone_min_max_dates() { } } } + +#[test] +#[cfg(feature = "rkyv-validation")] +fn test_rkyv_validation() { + let dt_min = NaiveDateTime::MIN; + let bytes = rkyv::to_bytes::<_, 12>(&dt_min).unwrap(); + assert_eq!(rkyv::from_bytes::(&bytes).unwrap(), dt_min); + + let dt_max = NaiveDateTime::MAX; + let bytes = rkyv::to_bytes::<_, 12>(&dt_max).unwrap(); + assert_eq!(rkyv::from_bytes::(&bytes).unwrap(), dt_max); +} diff --git a/src/naive/isoweek.rs b/src/naive/isoweek.rs index 237d841f9e..50ffcdc169 100644 --- a/src/naive/isoweek.rs +++ b/src/naive/isoweek.rs @@ -153,6 +153,8 @@ impl fmt::Debug for IsoWeek { #[cfg(test)] mod tests { + #[cfg(feature = "rkyv-validation")] + use super::IsoWeek; use crate::naive::{internals, NaiveDate}; use crate::Datelike; @@ -207,4 +209,16 @@ mod tests { assert!(monday.iso_week() >= friday.iso_week()); assert!(monday.iso_week() <= friday.iso_week()); } + + #[test] + #[cfg(feature = "rkyv-validation")] + fn test_rkyv_validation() { + let minweek = NaiveDate::MIN.iso_week(); + let bytes = rkyv::to_bytes::<_, 4>(&minweek).unwrap(); + assert_eq!(rkyv::from_bytes::(&bytes).unwrap(), minweek); + + let maxweek = NaiveDate::MAX.iso_week(); + let bytes = rkyv::to_bytes::<_, 4>(&maxweek).unwrap(); + assert_eq!(rkyv::from_bytes::(&bytes).unwrap(), maxweek); + } } diff --git a/src/naive/time/tests.rs b/src/naive/time/tests.rs index 30c1af9537..784a8390b8 100644 --- a/src/naive/time/tests.rs +++ b/src/naive/time/tests.rs @@ -376,3 +376,15 @@ fn test_overflowing_offset() { assert_eq!(t.overflowing_add_offset(positive_offset).0, t + positive_offset); assert_eq!(t.overflowing_sub_offset(positive_offset).0, t - positive_offset); } + +#[test] +#[cfg(feature = "rkyv-validation")] +fn test_rkyv_validation() { + let t_min = NaiveTime::MIN; + let bytes = rkyv::to_bytes::<_, 8>(&t_min).unwrap(); + assert_eq!(rkyv::from_bytes::(&bytes).unwrap(), t_min); + + let t_max = NaiveTime::MAX; + let bytes = rkyv::to_bytes::<_, 8>(&t_max).unwrap(); + assert_eq!(rkyv::from_bytes::(&bytes).unwrap(), t_max); +} diff --git a/src/offset/fixed.rs b/src/offset/fixed.rs index 806d13cca9..1826ec4ec2 100644 --- a/src/offset/fixed.rs +++ b/src/offset/fixed.rs @@ -227,4 +227,12 @@ mod tests { let offset = FixedOffset::from_str("+06:30").unwrap(); assert_eq!(offset.local_minus_utc, (6 * 3600) + 1800); } + + #[test] + #[cfg(feature = "rkyv-validation")] + fn test_rkyv_validation() { + let offset = FixedOffset::from_str("-0500").unwrap(); + let bytes = rkyv::to_bytes::<_, 4>(&offset).unwrap(); + assert_eq!(rkyv::from_bytes::(&bytes).unwrap(), offset); + } } diff --git a/src/offset/local/mod.rs b/src/offset/local/mod.rs index 91a8baed62..af1c21e7ef 100644 --- a/src/offset/local/mod.rs +++ b/src/offset/local/mod.rs @@ -259,4 +259,17 @@ mod tests { ); } } + + #[test] + #[cfg(feature = "rkyv-validation")] + fn test_rkyv_validation() { + let local = Local; + // Local is a ZST and serializes to 0 bytes + let bytes = rkyv::to_bytes::<_, 0>(&local).unwrap(); + assert_eq!(bytes.len(), 0); + + // but is deserialized to an archived variant without a + // wrapping object + assert_eq!(rkyv::from_bytes::(&bytes).unwrap(), super::ArchivedLocal); + } } diff --git a/src/weekday.rs b/src/weekday.rs index 444cfbaed4..786d516c7b 100644 --- a/src/weekday.rs +++ b/src/weekday.rs @@ -386,4 +386,13 @@ mod tests { from_str::(str).unwrap_err(); } } + + #[test] + #[cfg(feature = "rkyv-validation")] + fn test_rkyv_validation() { + let mon = Weekday::Mon; + let bytes = rkyv::to_bytes::<_, 1>(&mon).unwrap(); + + assert_eq!(rkyv::from_bytes::(&bytes).unwrap(), mon); + } } From 4e1734e1420a3ad9b0e4ab4a5fe486fa0ab1af4a Mon Sep 17 00:00:00 2001 From: Mikhail Katychev Date: Mon, 9 Oct 2023 14:12:54 -0500 Subject: [PATCH 5/6] removed rkyv from `cargo hack check` in CI based on feedback --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f1bdd2a4c2..a096504d97 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -75,7 +75,7 @@ jobs: - uses: taiki-e/install-action@cargo-hack - uses: Swatinem/rust-cache@v2 - run: | - cargo hack check --feature-powerset --optional-deps serde,rkyv \ + cargo hack check --feature-powerset --optional-deps serde \ --skip __internal_bench,iana-time-zone,pure-rust-locales,libc,winapi \ --all-targets # run using `bash` on all platforms for consistent From a5500afd1c691c13b7f2da16754ac9788aa26e28 Mon Sep 17 00:00:00 2001 From: Mikhail Katychev Date: Mon, 9 Oct 2023 16:35:21 -0500 Subject: [PATCH 6/6] clippy fix --- src/date.rs | 2 +- src/format/scan.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/date.rs b/src/date.rs index e82a3f0eee..8a8511b528 100644 --- a/src/date.rs +++ b/src/date.rs @@ -479,7 +479,7 @@ impl Eq for Date {} impl PartialOrd for Date { fn partial_cmp(&self, other: &Date) -> Option { - self.date.partial_cmp(&other.date) + Some(self.cmp(other)) } } diff --git a/src/format/scan.rs b/src/format/scan.rs index 98f3673ff1..45b5bcbec0 100644 --- a/src/format/scan.rs +++ b/src/format/scan.rs @@ -274,7 +274,7 @@ where }; s = match s.len() { len if len >= 2 => &s[2..], - len if len == 0 => s, + 0 => s, _ => return Err(TOO_SHORT), };