From e0f2b7dab7ab10864441e0b1ee52b78d11993e37 Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Wed, 1 Nov 2023 02:16:37 -0400 Subject: [PATCH 1/9] Added serde impls for Array; gated by `serde` --- Cargo.lock | 16 +++- hybrid-array/Cargo.toml | 9 +++ hybrid-array/src/impl_serde.rs | 135 +++++++++++++++++++++++++++++++++ hybrid-array/src/lib.rs | 48 ++++++++++++ 4 files changed, 206 insertions(+), 2 deletions(-) create mode 100644 hybrid-array/src/impl_serde.rs diff --git a/Cargo.lock b/Cargo.lock index 3c62511c..3e26a8e1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,6 +2,15 @@ # It is not intended for manual editing. version = 3 +[[package]] +name = "bincode" +version = "1.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b1f45e9417d87227c7a56d22e471c6206462cba514c7590c09aff4cf6d1ddcad" +dependencies = [ + "serde", +] + [[package]] name = "blobby" version = "0.3.1" @@ -84,6 +93,9 @@ version = "0.4.1" name = "hybrid-array" version = "0.2.0-pre.5" dependencies = [ + "bincode", + "serde", + "serde_json", "typenum", ] @@ -166,9 +178,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.107" +version = "1.0.108" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6b420ce6e3d8bd882e9b243c6eed35dbc9a6110c9769e74b584e0d68d1f20c65" +checksum = "3d1c7e3eac408d115102c4c24ad393e0821bb3a5df4d506a80f85f7a742a526b" dependencies = [ "itoa", "ryu", diff --git a/hybrid-array/Cargo.toml b/hybrid-array/Cargo.toml index 9d17e836..791b9feb 100644 --- a/hybrid-array/Cargo.toml +++ b/hybrid-array/Cargo.toml @@ -18,3 +18,12 @@ rust-version = "1.65" [dependencies] typenum = "1.17" +serde = { version = "1", optional = true } + +[dev-dependencies] +bincode = "1.3" +serde_json = "1" + +[features] +default = [] +serde = ["dep:serde"] diff --git a/hybrid-array/src/impl_serde.rs b/hybrid-array/src/impl_serde.rs new file mode 100644 index 00000000..e80a953b --- /dev/null +++ b/hybrid-array/src/impl_serde.rs @@ -0,0 +1,135 @@ +// This file was modified from +// https://github.com/fizyk20/generic-array/blob/0e2a03714b05bb7a737a677f8df77d6360d19c99/src/impl_serde.rs + +use crate::{Array, ArraySize}; +use core::fmt; +use core::marker::PhantomData; +use serde::de::{self, SeqAccess, Visitor}; +use serde::{ser::SerializeTuple, Deserialize, Deserializer, Serialize, Serializer}; + +impl Serialize for Array +where + T: Serialize, +{ + #[inline] + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + let mut tup = serializer.serialize_tuple(N::USIZE)?; + for el in self { + tup.serialize_element(el)?; + } + + tup.end() + } +} + +struct GAVisitor { + _t: PhantomData, + _n: PhantomData, +} + +// to avoid extra computation when testing for extra elements in the sequence +struct Dummy; +impl<'de> Deserialize<'de> for Dummy { + fn deserialize(_deserializer: D) -> Result + where + D: Deserializer<'de>, + { + Ok(Dummy) + } +} + +impl<'de, T, N: ArraySize> Visitor<'de> for GAVisitor +where + T: Deserialize<'de>, +{ + type Value = Array; + + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(formatter, "struct Array", N::USIZE) + } + + fn visit_seq(self, mut seq: A) -> Result, A::Error> + where + A: SeqAccess<'de>, + { + // Check the length in advance + match seq.size_hint() { + Some(n) if n != N::USIZE => { + return Err(de::Error::invalid_length(n, &self)); + } + _ => {} + } + + // Deserialize the array + let arr = Array::try_from_fn(|idx| { + let next_elem_opt = seq.next_element()?; + next_elem_opt.ok_or(de::Error::invalid_length(idx, &self)) + }); + + // If there's a value allegedly remaining, and deserializing it doesn't fail, then that's a + // length mismatch error + if seq.size_hint() != Some(0) && seq.next_element::()?.is_some() { + Err(de::Error::invalid_length(N::USIZE + 1, &self)) + } else { + arr + } + } +} + +impl<'de, T, N: ArraySize> Deserialize<'de> for Array +where + T: Deserialize<'de>, +{ + fn deserialize(deserializer: D) -> Result, D::Error> + where + D: Deserializer<'de>, + { + let visitor = GAVisitor { + _t: PhantomData, + _n: PhantomData, + }; + deserializer.deserialize_tuple(N::USIZE, visitor) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_serialize() { + let array = Array::::default(); + let serialized = bincode::serialize(&array); + assert!(serialized.is_ok()); + } + + #[test] + fn test_deserialize() { + let mut array = Array::::default(); + array[0] = 1; + array[1] = 2; + let serialized = bincode::serialize(&array).unwrap(); + let deserialized = bincode::deserialize::>(&serialized); + assert!(deserialized.is_ok()); + let array = deserialized.unwrap(); + assert_eq!(array[0], 1); + assert_eq!(array[1], 2); + } + + #[test] + fn test_serialized_size() { + let array = Array::::default(); + let size = bincode::serialized_size(&array).unwrap(); + assert_eq!(size, 1); + } + + #[test] + #[should_panic] + fn test_too_many() { + let serialized = "[1, 2, 3, 4, 5]"; + let _ = serde_json::from_str::>(serialized).unwrap(); + } +} diff --git a/hybrid-array/src/lib.rs b/hybrid-array/src/lib.rs index 8f6d7a08..302d1e1b 100644 --- a/hybrid-array/src/lib.rs +++ b/hybrid-array/src/lib.rs @@ -41,6 +41,9 @@ use typenum::{Diff, Sum, Unsigned}; mod impls; +#[cfg(feature = "serde")] +mod impl_serde; + /// Hybrid typenum-based and const generic array type. /// /// Provides the flexibility of typenum-based expressions while also @@ -64,6 +67,14 @@ where Self(ArrayExt::from_fn(cb)) } + /// Create array where each array element `T` is returned by the `cb` call. + pub fn try_from_fn(cb: F) -> Result + where + F: FnMut(usize) -> Result, + { + ArrayExt::try_from_fn(cb).map(Self) + } + /// Create array from a slice. pub fn from_slice(slice: &[T]) -> Result where @@ -570,6 +581,12 @@ pub trait ArrayExt: Sized { where F: FnMut(usize) -> T; + /// Try to create an array using the given callback function for each element. Returns an error + /// if any one of the calls errors + fn try_from_fn(cb: F) -> Result + where + F: FnMut(usize) -> Result; + /// Create array from a slice, returning [`TryFromSliceError`] if the slice /// length does not match the array length. fn from_slice(slice: &[T]) -> Result @@ -591,6 +608,37 @@ impl ArrayExt for [T; N] { }) } + fn try_from_fn(mut cb: F) -> Result + where + F: FnMut(usize) -> Result, + { + // TODO: Replace this entire function with array::try_map once it stabilizes + // https://doc.rust-lang.org/std/primitive.array.html#method.try_map + + // Make an uninitialized array. We will populate it element-by-element + let mut arr: [MaybeUninit; N] = unsafe { MaybeUninit::uninit().assume_init() }; + + // Dropping a `MaybeUninit` does nothing, so if there is a panic during this loop, + // we have a memory leak, but there is no memory safety issue. + for (idx, elem) in arr.iter_mut().enumerate() { + // Run the callback. On success, write it to the array. On error, return immediately + match cb(idx) { + Ok(val) => { + elem.write(val); + } + Err(e) => { + return Err(e); + } + } + } + + // If we've made it this far, all the elements have been written. Convert the uninitialized + // array to an initialized array + // TODO: Replace this map with MaybeUninit::array_assume_init() once it stabilizes + let arr = arr.map(|elem: MaybeUninit| unsafe { elem.assume_init() }); + Ok(arr) + } + fn from_slice(slice: &[T]) -> Result where T: Copy, From c46239fb241c96ade4ac070cd87b6db09e30ad9b Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Wed, 1 Nov 2023 02:26:39 -0400 Subject: [PATCH 2/9] Updated changelog --- hybrid-array/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hybrid-array/CHANGELOG.md b/hybrid-array/CHANGELOG.md index 2dac4530..1918effd 100644 --- a/hybrid-array/CHANGELOG.md +++ b/hybrid-array/CHANGELOG.md @@ -5,5 +5,9 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Pending + +- Added `serde` feature - implements `Serialize` and `Deserialize` for `Array` + ## 0.1.0 (2022-05-07) - Initial release From 2c174b78adfc0f91560c634841634f7bb3459713 Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Wed, 1 Nov 2023 02:41:29 -0400 Subject: [PATCH 3/9] Added PERMISSIONS file --- hybrid-array/PERMISSIONS | 23 +++++++++++++++++++++++ hybrid-array/README.md | 4 ++++ 2 files changed, 27 insertions(+) create mode 100644 hybrid-array/PERMISSIONS diff --git a/hybrid-array/PERMISSIONS b/hybrid-array/PERMISSIONS new file mode 100644 index 00000000..89eff58c --- /dev/null +++ b/hybrid-array/PERMISSIONS @@ -0,0 +1,23 @@ +generic-array + +The MIT License (MIT) + +Copyright (c) 2015 Bartłomiej Kamiński + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/hybrid-array/README.md b/hybrid-array/README.md index 86093c1c..69dd6f67 100644 --- a/hybrid-array/README.md +++ b/hybrid-array/README.md @@ -40,6 +40,10 @@ Unless you explicitly state otherwise, any contribution intentionally submitted for inclusion in the work by you, as defined in the Apache-2.0 license, shall be dual licensed as above, without any additional terms or conditions. +### Permissions + +Permissions and notices for code we use or modify can be found in [PERMISSIONS](PERMISSIONS). + [//]: # (badges) [crate-image]: https://buildstats.info/crate/hybrid-array From c737d9b0db0fdea0567261340c2bb70a159cb0b0 Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Wed, 1 Nov 2023 02:44:25 -0400 Subject: [PATCH 4/9] Renamed the de::Visitor; small cleanup --- hybrid-array/src/impl_serde.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/hybrid-array/src/impl_serde.rs b/hybrid-array/src/impl_serde.rs index e80a953b..a73ea313 100644 --- a/hybrid-array/src/impl_serde.rs +++ b/hybrid-array/src/impl_serde.rs @@ -2,10 +2,12 @@ // https://github.com/fizyk20/generic-array/blob/0e2a03714b05bb7a737a677f8df77d6360d19c99/src/impl_serde.rs use crate::{Array, ArraySize}; -use core::fmt; -use core::marker::PhantomData; -use serde::de::{self, SeqAccess, Visitor}; -use serde::{ser::SerializeTuple, Deserialize, Deserializer, Serialize, Serializer}; +use core::{fmt, marker::PhantomData}; +use serde::{ + de::{self, SeqAccess, Visitor}, + ser::SerializeTuple, + Deserialize, Deserializer, Serialize, Serializer, +}; impl Serialize for Array where @@ -25,7 +27,7 @@ where } } -struct GAVisitor { +struct ArrayVisitor { _t: PhantomData, _n: PhantomData, } @@ -41,7 +43,7 @@ impl<'de> Deserialize<'de> for Dummy { } } -impl<'de, T, N: ArraySize> Visitor<'de> for GAVisitor +impl<'de, T, N: ArraySize> Visitor<'de> for ArrayVisitor where T: Deserialize<'de>, { @@ -87,7 +89,7 @@ where where D: Deserializer<'de>, { - let visitor = GAVisitor { + let visitor = ArrayVisitor { _t: PhantomData, _n: PhantomData, }; From 8071174f27956a2ff23ce5130474bb054b51156e Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Wed, 1 Nov 2023 03:19:56 -0400 Subject: [PATCH 5/9] Make clippy happy --- hybrid-array/src/impl_serde.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hybrid-array/src/impl_serde.rs b/hybrid-array/src/impl_serde.rs index a73ea313..0ffed100 100644 --- a/hybrid-array/src/impl_serde.rs +++ b/hybrid-array/src/impl_serde.rs @@ -74,7 +74,9 @@ where // If there's a value allegedly remaining, and deserializing it doesn't fail, then that's a // length mismatch error if seq.size_hint() != Some(0) && seq.next_element::()?.is_some() { - Err(de::Error::invalid_length(N::USIZE + 1, &self)) + // The addition only saturates if this array is the size of all addressable memory. Not + // going to happen. And if it did, the only effect is an off-by-one error message + Err(de::Error::invalid_length(N::USIZE.saturating_add(1), &self)) } else { arr } From 2f7f15a017590aab87576068d9d415a3a8f9f54f Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Wed, 1 Nov 2023 03:38:43 -0400 Subject: [PATCH 6/9] Made sure Array::try_from_fn frees its partial contents when it errors --- hybrid-array/src/lib.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/hybrid-array/src/lib.rs b/hybrid-array/src/lib.rs index 302d1e1b..bc403fd0 100644 --- a/hybrid-array/src/lib.rs +++ b/hybrid-array/src/lib.rs @@ -618,20 +618,31 @@ impl ArrayExt for [T; N] { // Make an uninitialized array. We will populate it element-by-element let mut arr: [MaybeUninit; N] = unsafe { MaybeUninit::uninit().assume_init() }; - // Dropping a `MaybeUninit` does nothing, so if there is a panic during this loop, - // we have a memory leak, but there is no memory safety issue. + // Run the callbacks. On success, write it to the array. On error, drop the values we've + // allocated already, then return the error. Note: dropping a `MaybeUninit` does nothing, + // so if there is a panic during this loop, we have a memory leak, but there is no memory + // safety issue. + let mut err_info = None; for (idx, elem) in arr.iter_mut().enumerate() { - // Run the callback. On success, write it to the array. On error, return immediately match cb(idx) { Ok(val) => { elem.write(val); } Err(e) => { - return Err(e); + err_info = Some((idx, e)); } } } + // If an error occurred, go back and drop all the initialized values. Otherwise the values + // leak. + if let Some((err_idx, err)) = err_info { + arr.iter_mut() + .take(err_idx) + .for_each(|v| unsafe { v.assume_init_drop() }); + return Err(err); + } + // If we've made it this far, all the elements have been written. Convert the uninitialized // array to an initialized array // TODO: Replace this map with MaybeUninit::array_assume_init() once it stabilizes From 8c6466fb328528f275ec2abee4953eab42fedb1e Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Wed, 1 Nov 2023 04:36:03 -0400 Subject: [PATCH 7/9] Added default impl of from_fn; added try_from_fn to changelog --- hybrid-array/CHANGELOG.md | 1 + hybrid-array/src/lib.rs | 25 +++++++++++++++++-------- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/hybrid-array/CHANGELOG.md b/hybrid-array/CHANGELOG.md index 1918effd..59df6f04 100644 --- a/hybrid-array/CHANGELOG.md +++ b/hybrid-array/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Pending - Added `serde` feature - implements `Serialize` and `Deserialize` for `Array` +- Added `ArrayExt::try_from_fn` ## 0.1.0 (2022-05-07) - Initial release diff --git a/hybrid-array/src/lib.rs b/hybrid-array/src/lib.rs index bc403fd0..1385a7eb 100644 --- a/hybrid-array/src/lib.rs +++ b/hybrid-array/src/lib.rs @@ -576,17 +576,23 @@ pub trait ArrayOps: /// Extension trait with helper functions for core arrays. pub trait ArrayExt: Sized { - /// Create array using the given callback function for each element. - fn from_fn(cb: F) -> Self - where - F: FnMut(usize) -> T; - /// Try to create an array using the given callback function for each element. Returns an error /// if any one of the calls errors fn try_from_fn(cb: F) -> Result where F: FnMut(usize) -> Result; + /// Create array using the given callback function for each element. + fn from_fn(mut cb: F) -> Self + where + F: FnMut(usize) -> T, + { + // Turn the ordinary callback into a Result callback that always returns Ok + let wrapped_cb = |x| Result::::Ok(cb(x)); + // Now use the try_from version of this method + Self::try_from_fn(wrapped_cb).unwrap() + } + /// Create array from a slice, returning [`TryFromSliceError`] if the slice /// length does not match the array length. fn from_slice(slice: &[T]) -> Result @@ -595,6 +601,9 @@ pub trait ArrayExt: Sized { } impl ArrayExt for [T; N] { + // TODO: Eventually remove this and just use the default implementation. It's still + // here because the current Self::try_from_fn might be doing multiple passes over the data, + // depending on optimizations. Thus, this may be faster. fn from_fn(mut cb: F) -> Self where F: FnMut(usize) -> T, @@ -628,8 +637,8 @@ impl ArrayExt for [T; N] { Ok(val) => { elem.write(val); } - Err(e) => { - err_info = Some((idx, e)); + Err(err) => { + err_info = Some((idx, err)); } } } @@ -646,7 +655,7 @@ impl ArrayExt for [T; N] { // If we've made it this far, all the elements have been written. Convert the uninitialized // array to an initialized array // TODO: Replace this map with MaybeUninit::array_assume_init() once it stabilizes - let arr = arr.map(|elem: MaybeUninit| unsafe { elem.assume_init() }); + let arr = arr.map(|v: MaybeUninit| unsafe { v.assume_init() }); Ok(arr) } From cbc7738085af01f323d7ac7f9c1dd6e8be14c78d Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Wed, 1 Nov 2023 04:49:34 -0400 Subject: [PATCH 8/9] Make clippy happy --- hybrid-array/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hybrid-array/src/lib.rs b/hybrid-array/src/lib.rs index 1385a7eb..d8f5173e 100644 --- a/hybrid-array/src/lib.rs +++ b/hybrid-array/src/lib.rs @@ -583,12 +583,13 @@ pub trait ArrayExt: Sized { F: FnMut(usize) -> Result; /// Create array using the given callback function for each element. + #[allow(clippy::unwrap_used)] fn from_fn(mut cb: F) -> Self where F: FnMut(usize) -> T, { // Turn the ordinary callback into a Result callback that always returns Ok - let wrapped_cb = |x| Result::::Ok(cb(x)); + let wrapped_cb = |idx| Result::<_, ::core::convert::Infallible>::Ok(cb(idx)); // Now use the try_from version of this method Self::try_from_fn(wrapped_cb).unwrap() } From 9898435d86e918372e23091bf5f9a1822fcfa028 Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Wed, 1 Nov 2023 04:53:52 -0400 Subject: [PATCH 9/9] Make clippy happy --- hybrid-array/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hybrid-array/src/lib.rs b/hybrid-array/src/lib.rs index d8f5173e..bbb4b193 100644 --- a/hybrid-array/src/lib.rs +++ b/hybrid-array/src/lib.rs @@ -583,13 +583,13 @@ pub trait ArrayExt: Sized { F: FnMut(usize) -> Result; /// Create array using the given callback function for each element. - #[allow(clippy::unwrap_used)] + #[allow(clippy::unwrap_used, unused_qualifications)] fn from_fn(mut cb: F) -> Self where F: FnMut(usize) -> T, { // Turn the ordinary callback into a Result callback that always returns Ok - let wrapped_cb = |idx| Result::<_, ::core::convert::Infallible>::Ok(cb(idx)); + let wrapped_cb = |idx| Result::::Ok(cb(idx)); // Now use the try_from version of this method Self::try_from_fn(wrapped_cb).unwrap() }