From a08880d8599daffc6b94b7716180052f3712996a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?dj8yf0=CE=BCl?= Date: Thu, 7 Dec 2023 15:16:13 +0200 Subject: [PATCH 1/2] chore: improve doc and module structure on `rc` feature --- borsh/Cargo.toml | 5 ++++- borsh/src/de/mod.rs | 49 +++++++++++++++++++++++++++++--------------- borsh/src/lib.rs | 3 +++ borsh/src/ser/mod.rs | 40 ++++++++++++++++++++++++++---------- 4 files changed, 69 insertions(+), 28 deletions(-) diff --git a/borsh/Cargo.toml b/borsh/Cargo.toml index 8e9114353..a241eeefa 100644 --- a/borsh/Cargo.toml +++ b/borsh/Cargo.toml @@ -45,7 +45,7 @@ borsh = { path = ".", default_features = false, features = ["bytes", "bson"] } insta = "1.29.0" [package.metadata.docs.rs] -features = ["derive", "unstable__schema"] +features = ["derive", "unstable__schema", "rc"] targets = ["x86_64-unknown-linux-gnu"] [features] @@ -53,5 +53,8 @@ default = ["std"] derive = ["borsh-derive"] unstable__schema = ["derive", "borsh-derive/schema"] std = [] +# Opt into impls for Rc and Arc. Serializing and deserializing these types +# does not preserve identity and may result in multiple copies of the same data. +# Be sure that this is what you want before enabling this feature. rc = [] de_strict_order = [] diff --git a/borsh/src/de/mod.rs b/borsh/src/de/mod.rs index bb6630c99..ab3b1f30f 100644 --- a/borsh/src/de/mod.rs +++ b/borsh/src/de/mod.rs @@ -19,8 +19,6 @@ use crate::__private::maybestd::{ }; use crate::io::{Error, ErrorKind, Read, Result}; -#[cfg(feature = "rc")] -use crate::__private::maybestd::{rc::Rc, sync::Arc}; use crate::error::check_zst; mod hint; @@ -883,23 +881,42 @@ impl_range!(RangeFrom, start.., start); impl_range!(RangeTo, ..end, end); impl_range!(RangeToInclusive, ..=end, end); +/// Module is available if borsh is built with `features = ["rc"]`. #[cfg(feature = "rc")] -impl BorshDeserialize for Rc -where - Box: BorshDeserialize, -{ - fn deserialize_reader(reader: &mut R) -> Result { - Ok(Box::::deserialize_reader(reader)?.into()) +pub mod rc { + //! + //! Module defines [BorshDeserialize] implementation for + //! [alloc::rc::Rc](std::rc::Rc) and [alloc::sync::Arc](std::sync::Arc). + use crate::__private::maybestd::{boxed::Box, rc::Rc, sync::Arc}; + use crate::io::{Read, Result}; + use crate::BorshDeserialize; + + /// This impl requires the [`"rc"`] Cargo feature of borsh. + /// + /// Deserializing a data structure containing `Rc` will not attempt to + /// deduplicate `Rc` references to the same data. Every deserialized `Rc` + /// will end up with a strong count of 1. + impl BorshDeserialize for Rc + where + Box: BorshDeserialize, + { + fn deserialize_reader(reader: &mut R) -> Result { + Ok(Box::::deserialize_reader(reader)?.into()) + } } -} -#[cfg(feature = "rc")] -impl BorshDeserialize for Arc -where - Box: BorshDeserialize, -{ - fn deserialize_reader(reader: &mut R) -> Result { - Ok(Box::::deserialize_reader(reader)?.into()) + /// This impl requires the [`"rc"`] Cargo feature of borsh. + /// + /// Deserializing a data structure containing `Arc` will not attempt to + /// deduplicate `Arc` references to the same data. Every deserialized `Arc` + /// will end up with a strong count of 1. + impl BorshDeserialize for Arc + where + Box: BorshDeserialize, + { + fn deserialize_reader(reader: &mut R) -> Result { + Ok(Box::::deserialize_reader(reader)?.into()) + } } } diff --git a/borsh/src/lib.rs b/borsh/src/lib.rs index 3b7cdc99b..c163751fc 100644 --- a/borsh/src/lib.rs +++ b/borsh/src/lib.rs @@ -35,6 +35,9 @@ Gates implementation of [BorshSerialize] and [BorshDeserialize] for [`Rc`](std::rc::Rc)/[`Arc`](std::sync::Arc) respectively. In `no_std` setting `Rc`/`Arc` are pulled from `alloc` crate. + Serializing and deserializing these types + does not preserve identity and may result in multiple copies of the same data. + Be sure that this is what you want before enabling this feature. * **hashbrown** - Pulls in [HashMap](std::collections::HashMap)/[HashSet](std::collections::HashSet) when no `std` is available. This feature is set to be mutually exclusive with **std** feature. diff --git a/borsh/src/ser/mod.rs b/borsh/src/ser/mod.rs index 86da9bd72..55e2e3c00 100644 --- a/borsh/src/ser/mod.rs +++ b/borsh/src/ser/mod.rs @@ -11,9 +11,6 @@ use crate::__private::maybestd::{ use crate::error::check_zst; use crate::io::{Error, ErrorKind, Result, Write}; -#[cfg(feature = "rc")] -use crate::__private::maybestd::{rc::Rc, sync::Arc}; - pub(crate) mod helpers; const FLOAT_NAN_ERR: &str = "For portability reasons we do not allow to serialize NaNs."; @@ -600,17 +597,38 @@ impl_range!(RangeFrom, this, &this.start); impl_range!(RangeTo, this, &this.end); impl_range!(RangeToInclusive, this, &this.end); +/// Module is available if borsh is built with `features = ["rc"]`. #[cfg(feature = "rc")] -impl BorshSerialize for Rc { - fn serialize(&self, writer: &mut W) -> Result<()> { - (**self).serialize(writer) +pub mod rc { + //! + //! Module defines [BorshSerialize] implementation for + //! [alloc::rc::Rc](std::rc::Rc) and [alloc::sync::Arc](std::sync::Arc). + use crate::__private::maybestd::{rc::Rc, sync::Arc}; + use crate::io::{Result, Write}; + use crate::BorshSerialize; + + /// This impl requires the [`"rc"`] Cargo feature of borsh. + /// + /// Serializing a data structure containing `Rc` will serialize a copy of + /// the contents of the `Rc` each time the `Rc` is referenced within the + /// data structure. Serialization will not attempt to deduplicate these + /// repeated data. + impl BorshSerialize for Rc { + fn serialize(&self, writer: &mut W) -> Result<()> { + (**self).serialize(writer) + } } -} -#[cfg(feature = "rc")] -impl BorshSerialize for Arc { - fn serialize(&self, writer: &mut W) -> Result<()> { - (**self).serialize(writer) + /// This impl requires the [`"rc"`] Cargo feature of borsh. + /// + /// Serializing a data structure containing `Arc` will serialize a copy of + /// the contents of the `Arc` each time the `Arc` is referenced within the + /// data structure. Serialization will not attempt to deduplicate these + /// repeated data. + impl BorshSerialize for Arc { + fn serialize(&self, writer: &mut W) -> Result<()> { + (**self).serialize(writer) + } } } From 125162edc33e998b4cc1ecfcab64638126bd5de7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?dj8yf0=CE=BCl?= Date: Thu, 7 Dec 2023 15:58:25 +0200 Subject: [PATCH 2/2] feat: impl `BorshSchema` for `Rc` and `Arc` --- .github/test.sh | 4 +-- borsh/src/schema.rs | 38 ++++++++++++++++++++++ borsh/tests/test_rc.rs | 73 +++++++++++++++++++++++++++++++++++++++++- 3 files changed, 112 insertions(+), 3 deletions(-) diff --git a/.github/test.sh b/.github/test.sh index 3816b3e34..0d935f1ae 100755 --- a/.github/test.sh +++ b/.github/test.sh @@ -8,14 +8,14 @@ cargo test cargo test --features unstable__schema,ascii --test test_ascii_strings cargo test --features derive cargo test --features unstable__schema -cargo test --test test_rc --features rc +cargo test --test test_rc --features unstable__schema,rc cargo test --test test_hash_map --test test_btree_map --features de_strict_order cargo test --no-default-features cargo test --no-default-features --features unstable__schema,ascii --test test_ascii_strings cargo test --no-default-features --features derive cargo test --no-default-features --features unstable__schema -cargo test --no-default-features --test test_rc --features rc +cargo test --no-default-features --test test_rc --features unstable__schema,rc cargo test --no-default-features --features hashbrown popd pushd borsh-derive diff --git a/borsh/src/schema.rs b/borsh/src/schema.rs index 5c0825037..ffefc653f 100644 --- a/borsh/src/schema.rs +++ b/borsh/src/schema.rs @@ -317,6 +317,44 @@ where T::declaration() } } +/// Module is available if borsh is built with `features = ["rc"]`. +#[cfg(feature = "rc")] +pub mod rc { + //! + //! Module defines [BorshSchema] implementation for + //! [alloc::rc::Rc](std::rc::Rc) and [alloc::sync::Arc](std::sync::Arc). + use crate::BorshSchema; + + use super::{Declaration, Definition}; + use crate::__private::maybestd::collections::BTreeMap; + use crate::__private::maybestd::{rc::Rc, sync::Arc}; + + impl BorshSchema for Rc + where + T: BorshSchema + ?Sized, + { + fn add_definitions_recursively(definitions: &mut BTreeMap) { + T::add_definitions_recursively(definitions); + } + + fn declaration() -> Declaration { + T::declaration() + } + } + + impl BorshSchema for Arc + where + T: BorshSchema + ?Sized, + { + fn add_definitions_recursively(definitions: &mut BTreeMap) { + T::add_definitions_recursively(definitions); + } + + fn declaration() -> Declaration { + T::declaration() + } + } +} macro_rules! impl_for_renamed_primitives { ($($ty: ty : $name: ident => $size: expr);+) => { diff --git a/borsh/tests/test_rc.rs b/borsh/tests/test_rc.rs index ba1e3f9f1..010e9dc1c 100644 --- a/borsh/tests/test_rc.rs +++ b/borsh/tests/test_rc.rs @@ -4,7 +4,6 @@ #[cfg(feature = "std")] pub use std::{rc, sync}; -#[cfg(not(feature = "std"))] extern crate alloc; #[cfg(not(feature = "std"))] pub use alloc::{rc, sync}; @@ -44,3 +43,75 @@ fn test_slice_arc() { let deserialized = from_slice::>(&serialized).unwrap(); assert_eq!(original, &*deserialized); } + +#[cfg(feature = "unstable__schema")] +mod schema { + use super::{rc, sync}; + use alloc::{ + collections::BTreeMap, + string::{String, ToString}, + }; + use borsh::schema::{BorshSchema, Definition}; + macro_rules! map( + () => { BTreeMap::new() }; + { $($key:expr => $value:expr),+ } => { + { + let mut m = BTreeMap::new(); + $( + m.insert($key.to_string(), $value); + )+ + m + } + }; + ); + fn common_map_i32() -> BTreeMap { + map! { + + "i32" => Definition::Primitive(4) + } + } + + fn common_map_slice_i32() -> BTreeMap { + map! { + "Vec" => Definition::Sequence { + length_width: Definition::DEFAULT_LENGTH_WIDTH, + length_range: Definition::DEFAULT_LENGTH_RANGE, + elements: "i32".to_string() + }, + "i32" => Definition::Primitive(4) + } + } + + #[test] + fn test_rc() { + assert_eq!("i32", as BorshSchema>::declaration()); + + let mut actual_defs = map!(); + as BorshSchema>::add_definitions_recursively(&mut actual_defs); + assert_eq!(common_map_i32(), actual_defs); + } + + #[test] + fn test_slice_rc() { + assert_eq!("Vec", as BorshSchema>::declaration()); + let mut actual_defs = map!(); + as BorshSchema>::add_definitions_recursively(&mut actual_defs); + assert_eq!(common_map_slice_i32(), actual_defs); + } + + #[test] + fn test_arc() { + assert_eq!("i32", as BorshSchema>::declaration()); + let mut actual_defs = map!(); + as BorshSchema>::add_definitions_recursively(&mut actual_defs); + assert_eq!(common_map_i32(), actual_defs); + } + + #[test] + fn test_slice_arc() { + assert_eq!("Vec", as BorshSchema>::declaration()); + let mut actual_defs = map!(); + as BorshSchema>::add_definitions_recursively(&mut actual_defs); + assert_eq!(common_map_slice_i32(), actual_defs); + } +}