Skip to content

Commit

Permalink
feat!: raise bound on keys in hashcollections PartialOrd -> Ord
Browse files Browse the repository at this point in the history
  • Loading branch information
dj8yf0μl committed Aug 22, 2023
1 parent 81d0392 commit d5ac0a5
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 23 deletions.
8 changes: 4 additions & 4 deletions borsh-derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,13 @@ Attribute adds possibility to override bounds for `BorshSerialize` in order to e
2. fixing complex cases, when derive hasn't figured out the right bounds on type parameters automatically.
```ignore
/// additional bound `T: PartialOrd` (required by `HashMap`) is injected into
/// additional bound `T: Ord` (required by `HashMap`) is injected into
/// derived trait implementation via attribute to avoid adding the bounds on the struct itself
#[derive(BorshSerialize)]
struct A<T, U> {
a: String,
#[borsh(bound(serialize =
"T: borsh::ser::BorshSerialize + PartialOrd,
"T: borsh::ser::BorshSerialize + Ord,
U: borsh::ser::BorshSerialize"))]
b: HashMap<T, U>,
}
Expand Down Expand Up @@ -360,14 +360,14 @@ Attribute adds possibility to override bounds for `BorshDeserialize` in order to
2. fixing complex cases, when derive hasn't figured out the right bounds on type parameters automatically.
```ignore
/// additional bounds `T: PartialOrd + Hash + Eq` (required by `HashMap`) are injected into
/// additional bounds `T: Ord + Hash + Eq` (required by `HashMap`) are injected into
/// derived trait implementation via attribute to avoid adding the bounds on the struct itself
#[derive(BorshDeserialize)]
struct A<T, U> {
a: String,
#[borsh(bound(
deserialize =
"T: PartialOrd + Hash + Eq + borsh::de::BorshDeserialize,
"T: Ord + Hash + Eq + borsh::de::BorshDeserialize,
U: borsh::de::BorshDeserialize"
))]
b: HashMap<T, U>,
Expand Down
10 changes: 4 additions & 6 deletions borsh/src/de/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -493,12 +493,10 @@ pub mod hashes {
const ERROR_WRONG_ORDER_OF_KEYS: &str = "keys were not serialized in ascending order";
#[cfg(feature = "de_strict_order")]
use crate::__private::maybestd::io::{Error, ErrorKind};
#[cfg(feature = "de_strict_order")]
use core::cmp::Ordering;

impl<T, H> BorshDeserialize for HashSet<T, H>
where
T: BorshDeserialize + Eq + Hash + PartialOrd,
T: BorshDeserialize + Eq + Hash + Ord,
H: BuildHasher + Default,
{
#[inline]
Expand All @@ -516,7 +514,7 @@ pub mod hashes {
let [a, b] = pair else {
unreachable!("`windows` always return a slice of length 2 or nothing");
};
let cmp_result = a.partial_cmp(b).map_or(false, Ordering::is_lt);
let cmp_result = a.cmp(b).is_lt();
if !cmp_result {
return Err(Error::new(
ErrorKind::InvalidData,
Expand All @@ -531,7 +529,7 @@ pub mod hashes {

impl<K, V, H> BorshDeserialize for HashMap<K, V, H>
where
K: BorshDeserialize + Eq + Hash + PartialOrd,
K: BorshDeserialize + Eq + Hash + Ord,
V: BorshDeserialize,
H: BuildHasher + Default,
{
Expand All @@ -550,7 +548,7 @@ pub mod hashes {
let [(a_k, _a_v), (b_k, _b_v)] = pair else {
unreachable!("`windows` always return a slice of length 2 or nothing");
};
let cmp_result = a_k.partial_cmp(b_k).map_or(false, Ordering::is_lt);
let cmp_result = a_k.cmp(b_k).is_lt();
if !cmp_result {
return Err(Error::new(
ErrorKind::InvalidData,
Expand Down
8 changes: 4 additions & 4 deletions borsh/src/ser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,14 +361,14 @@ pub mod hashes {

impl<K, V, H> BorshSerialize for HashMap<K, V, H>
where
K: BorshSerialize + PartialOrd,
K: BorshSerialize + Ord,
V: BorshSerialize,
H: BuildHasher,
{
#[inline]
fn serialize<W: Write>(&self, writer: &mut W) -> Result<()> {
let mut vec = self.iter().collect::<Vec<_>>();
vec.sort_by(|(a, _), (b, _)| a.partial_cmp(b).unwrap());
vec.sort_by(|(a, _), (b, _)| a.cmp(b));
u32::try_from(vec.len())
.map_err(|_| ErrorKind::InvalidData)?
.serialize(writer)?;
Expand All @@ -382,13 +382,13 @@ pub mod hashes {

impl<T, H> BorshSerialize for HashSet<T, H>
where
T: BorshSerialize + PartialOrd,
T: BorshSerialize + Ord,
H: BuildHasher,
{
#[inline]
fn serialize<W: Write>(&self, writer: &mut W) -> Result<()> {
let mut vec = self.iter().collect::<Vec<_>>();
vec.sort_by(|a, b| a.partial_cmp(b).unwrap());
vec.sort();
u32::try_from(vec.len())
.map_err(|_| ErrorKind::InvalidData)?
.serialize(writer)?;
Expand Down
14 changes: 6 additions & 8 deletions borsh/tests/test_generic_struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ struct NamedA<T> {
/// `T: Hash + Eq` bound is required for `BorshDeserialize` derive to be successful
#[cfg(hash_collections)]
#[derive(BorshSerialize, BorshDeserialize)]
struct C<T: PartialOrd + Hash + Eq, U> {
struct C<T: Ord + Hash + Eq, U> {
a: String,
b: HashMap<T, U>,
}
Expand All @@ -65,7 +65,7 @@ struct C<T: PartialOrd + Hash + Eq, U> {
#[derive(BorshSerialize)]
struct C1<T, U> {
a: String,
#[borsh(bound(serialize = "T: borsh::ser::BorshSerialize + PartialOrd,
#[borsh(bound(serialize = "T: borsh::ser::BorshSerialize + Ord,
U: borsh::ser::BorshSerialize"))]
b: HashMap<T, U>,
}
Expand All @@ -77,10 +77,8 @@ struct C1<T, U> {
#[derive(BorshDeserialize)]
struct C2<T, U> {
a: String,
#[borsh(bound(
deserialize = "T: PartialOrd + Hash + Eq + borsh::de::BorshDeserialize,
U: borsh::de::BorshDeserialize"
))]
#[borsh(bound(deserialize = "T: Ord + Hash + Eq + borsh::de::BorshDeserialize,
U: borsh::de::BorshDeserialize"))]
b: HashMap<T, U>,
}

Expand All @@ -101,7 +99,7 @@ struct G1<K, V, U>(#[borsh(skip)] HashMap<K, V>, U);

#[cfg(hash_collections)]
#[derive(BorshDeserialize)]
struct G2<K: PartialOrd + Hash + Eq, V, U>(HashMap<K, V>, #[borsh(skip)] U);
struct G2<K: Ord + Hash + Eq, V, U>(HashMap<K, V>, #[borsh(skip)] U);

/// implicit derived `core::default::Default` bounds on `K` and `V` are dropped by empty bound
/// specified, as `HashMap` hash its own `Default` implementation
Expand Down Expand Up @@ -139,7 +137,7 @@ enum I1<K, V, U> {

#[cfg(hash_collections)]
#[derive(BorshSerialize, BorshDeserialize, Debug)]
enum I2<K: PartialOrd + Eq + Hash, V, U> {
enum I2<K: Ord + Eq + Hash, V, U> {
B { x: HashMap<K, V>, y: String },
C(K, #[borsh(skip)] U),
}
Expand Down
2 changes: 1 addition & 1 deletion borsh/tests/test_recursive_structs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use alloc::{boxed::Box, string::String, vec::Vec};

#[cfg(hash_collections)]
#[derive(BorshSerialize, BorshDeserialize)]
struct CRec<U: PartialOrd + Hash + Eq> {
struct CRec<U: Ord + Hash + Eq> {
a: String,
b: HashMap<U, CRec<U>>,
}
Expand Down

0 comments on commit d5ac0a5

Please sign in to comment.