From 46a186060159e680d983a039e25d8aeaff14028d Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 11 Jun 2016 00:43:02 -0700 Subject: [PATCH 1/5] De/serialize for HashMap --- serde/src/de/impls.rs | 47 ++++++++++++++--------------------- serde/src/ser/impls.rs | 5 ++-- serde_tests/Cargo.toml | 1 + serde_tests/tests/macros.rs | 8 ++++++ serde_tests/tests/test_de.rs | 14 +++++++++++ serde_tests/tests/test_ser.rs | 18 +++++++++++++- 6 files changed, 62 insertions(+), 31 deletions(-) diff --git a/serde/src/de/impls.rs b/serde/src/de/impls.rs index ed727aa2..ad3d39eb 100644 --- a/serde/src/de/impls.rs +++ b/serde/src/de/impls.rs @@ -32,7 +32,7 @@ use collections::enum_set::{CLike, EnumSet}; #[cfg(all(feature = "nightly", feature = "collections"))] use collections::borrow::ToOwned; -use core::hash::Hash; +use core::hash::{Hash, BuildHasher}; use core::marker::PhantomData; #[cfg(feature = "std")] use std::net; @@ -726,31 +726,26 @@ tuple_impls! { macro_rules! map_impl { ( $ty:ty, - < $($constraints:ident),* >, - $visitor_name:ident, + $visitor_ty:ident < $($typaram:ident : $bound1:ident $(+ $bound2:ident)*),* >, $visitor:ident, $ctor:expr, - $with_capacity:expr, - $insert:expr + $with_capacity:expr ) => { /// A visitor that produces a map. - pub struct $visitor_name { + pub struct $visitor_ty<$($typaram),*> { marker: PhantomData<$ty>, } - impl $visitor_name { + impl<$($typaram : $bound1 $(+ $bound2)*),*> $visitor_ty<$($typaram),*> { /// Construct a `MapVisitor*`. pub fn new() -> Self { - $visitor_name { + $visitor_ty { marker: PhantomData, } } } - impl Visitor for $visitor_name - where K: $($constraints +)*, - V: Deserialize, - { + impl<$($typaram : $bound1 $(+ $bound2)*),*> Visitor for $visitor_ty<$($typaram),*> { type Value = $ty; #[inline] @@ -767,7 +762,7 @@ macro_rules! map_impl { let mut values = $with_capacity; while let Some((key, value)) = try!($visitor.visit()) { - $insert(&mut values, key, value); + values.insert(key, value); } try!($visitor.end()); @@ -776,14 +771,11 @@ macro_rules! map_impl { } } - impl Deserialize for $ty - where K: $($constraints +)*, - V: Deserialize, - { + impl<$($typaram : $bound1 $(+ $bound2)*),*> Deserialize for $ty { fn deserialize(deserializer: &mut D) -> Result<$ty, D::Error> where D: Deserializer, { - deserializer.deserialize_map($visitor_name::new()) + deserializer.deserialize_map($visitor_ty::new()) } } } @@ -792,22 +784,21 @@ macro_rules! map_impl { #[cfg(any(feature = "std", feature = "collections"))] map_impl!( BTreeMap, - , - BTreeMapVisitor, + BTreeMapVisitor, visitor, BTreeMap::new(), - BTreeMap::new(), - BTreeMap::insert); + BTreeMap::new()); #[cfg(feature = "std")] map_impl!( - HashMap, - , - HashMapVisitor, + HashMap, + HashMapVisitor, visitor, - HashMap::new(), - HashMap::with_capacity(visitor.size_hint().0), - HashMap::insert); + HashMap::with_hasher(S::default()), + HashMap::with_capacity_and_hasher(visitor.size_hint().0, S::default())); /////////////////////////////////////////////////////////////////////////////// diff --git a/serde/src/ser/impls.rs b/serde/src/ser/impls.rs index b51226a9..a05ae862 100644 --- a/serde/src/ser/impls.rs +++ b/serde/src/ser/impls.rs @@ -31,7 +31,7 @@ use collections::enum_set::{CLike, EnumSet}; #[cfg(all(feature = "nightly", feature = "collections"))] use collections::borrow::ToOwned; -use core::hash::Hash; +use core::hash::{Hash, BuildHasher}; #[cfg(feature = "nightly")] use core::iter; #[cfg(feature = "std")] @@ -651,9 +651,10 @@ impl Serialize for BTreeMap } #[cfg(feature = "std")] -impl Serialize for HashMap +impl Serialize for HashMap where K: Serialize + Eq + Hash, V: Serialize, + H: BuildHasher, { #[inline] fn serialize(&self, serializer: &mut S) -> Result<(), S::Error> diff --git a/serde_tests/Cargo.toml b/serde_tests/Cargo.toml index adb7a940..47792176 100644 --- a/serde_tests/Cargo.toml +++ b/serde_tests/Cargo.toml @@ -19,6 +19,7 @@ syntex_syntax = { version = "^0.35.0" } serde_codegen = { version = "*", path = "../serde_codegen", features = ["with-syntex"] } [dev-dependencies] +fnv = "1.0" rustc-serialize = "^0.3.16" serde = { version = "*", path = "../serde" } syntex = "^0.35.0" diff --git a/serde_tests/tests/macros.rs b/serde_tests/tests/macros.rs index 069d4f26..c0b38b3d 100644 --- a/serde_tests/tests/macros.rs +++ b/serde_tests/tests/macros.rs @@ -75,5 +75,13 @@ macro_rules! hashmap { $(map.insert($key, $value);)+ map } + }; + ($hasher:ident @ $($key:expr => $value:expr),+) => { + { + use std::hash::BuildHasherDefault; + let mut map = HashMap::with_hasher(BuildHasherDefault::<$hasher>::default()); + $(map.insert($key, $value);)+ + map + } } } diff --git a/serde_tests/tests/test_de.rs b/serde_tests/tests/test_de.rs index 932b4b10..8081b9ea 100644 --- a/serde_tests/tests/test_de.rs +++ b/serde_tests/tests/test_de.rs @@ -2,6 +2,9 @@ use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use std::net; use std::path::PathBuf; +extern crate fnv; +use self::fnv::FnvHasher; + use token::{ Error, Token, @@ -532,6 +535,17 @@ declare_tests! { Token::StructStart("Anything", Some(0)), Token::MapEnd, ], + hashmap![FnvHasher @ 1 => 2, 3 => 4] => vec![ + Token::MapStart(Some(2)), + Token::MapSep, + Token::I32(1), + Token::I32(2), + + Token::MapSep, + Token::I32(3), + Token::I32(4), + Token::MapEnd, + ], } test_struct { Struct { a: 1, b: 2, c: 0 } => vec![ diff --git a/serde_tests/tests/test_ser.rs b/serde_tests/tests/test_ser.rs index 2a8117a1..4b6e4c70 100644 --- a/serde_tests/tests/test_ser.rs +++ b/serde_tests/tests/test_ser.rs @@ -1,10 +1,13 @@ -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashMap}; use std::net; use std::path::{Path, PathBuf}; use std::str; use token::{self, Token}; +extern crate fnv; +use self::fnv::FnvHasher; + ////////////////////////////////////////////////////////////////////////// #[derive(Serialize)] @@ -204,6 +207,19 @@ declare_ser_tests! { Token::MapEnd, ], } + test_hashmap { + hashmap![FnvHasher @ 1 => 2, 3 => 4] => &[ + Token::MapStart(Some(2)), + Token::MapSep, + Token::I32(1), + Token::I32(2), + + Token::MapSep, + Token::I32(3), + Token::I32(4), + Token::MapEnd, + ], + } test_unit_struct { UnitStruct => &[Token::UnitStruct("UnitStruct")], } From dd3f653103ed7a4665044f554005aca1b5aec2c1 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 11 Jun 2016 09:41:34 -0700 Subject: [PATCH 2/5] Move bounds to where-clause to increase legibility --- serde/src/de/impls.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/serde/src/de/impls.rs b/serde/src/de/impls.rs index ad3d39eb..a4e0c29f 100644 --- a/serde/src/de/impls.rs +++ b/serde/src/de/impls.rs @@ -736,7 +736,9 @@ macro_rules! map_impl { marker: PhantomData<$ty>, } - impl<$($typaram : $bound1 $(+ $bound2)*),*> $visitor_ty<$($typaram),*> { + impl<$($typaram),*> $visitor_ty<$($typaram),*> + where $($typaram: $bound1 $(+ $bound2)*),* + { /// Construct a `MapVisitor*`. pub fn new() -> Self { $visitor_ty { @@ -745,7 +747,9 @@ macro_rules! map_impl { } } - impl<$($typaram : $bound1 $(+ $bound2)*),*> Visitor for $visitor_ty<$($typaram),*> { + impl<$($typaram),*> Visitor for $visitor_ty<$($typaram),*> + where $($typaram: $bound1 $(+ $bound2)*),* + { type Value = $ty; #[inline] @@ -771,7 +775,9 @@ macro_rules! map_impl { } } - impl<$($typaram : $bound1 $(+ $bound2)*),*> Deserialize for $ty { + impl<$($typaram),*> Deserialize for $ty + where $($typaram: $bound1 $(+ $bound2)*),* + { fn deserialize(deserializer: &mut D) -> Result<$ty, D::Error> where D: Deserializer, { From 322d7a90dbafe2db208555f11dc140103546b5ad Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 11 Jun 2016 09:42:02 -0700 Subject: [PATCH 3/5] Add ser tests for normal HashMap --- serde_tests/tests/test_ser.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/serde_tests/tests/test_ser.rs b/serde_tests/tests/test_ser.rs index 4b6e4c70..2fa8cb9d 100644 --- a/serde_tests/tests/test_ser.rs +++ b/serde_tests/tests/test_ser.rs @@ -208,15 +208,22 @@ declare_ser_tests! { ], } test_hashmap { - hashmap![FnvHasher @ 1 => 2, 3 => 4] => &[ - Token::MapStart(Some(2)), + HashMap::::new() => &[ + Token::MapStart(Some(0)), + Token::MapEnd, + ], + hashmap![1 => 2] => &[ + Token::MapStart(Some(1)), Token::MapSep, Token::I32(1), Token::I32(2), - + Token::MapEnd, + ], + hashmap![FnvHasher @ 1 => 2] => &[ + Token::MapStart(Some(1)), Token::MapSep, - Token::I32(3), - Token::I32(4), + Token::I32(1), + Token::I32(2), Token::MapEnd, ], } From decc5719889f7abbef97cefec6030530c79bd8ef Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 11 Jun 2016 09:59:29 -0700 Subject: [PATCH 4/5] De/serialize for HashSet --- serde/src/de/impls.rs | 51 ++++++++++++++++------------------- serde/src/ser/impls.rs | 3 ++- serde_tests/tests/macros.rs | 8 ++++++ serde_tests/tests/test_de.rs | 12 +++++++++ serde_tests/tests/test_ser.rs | 20 +++++++++++++- 5 files changed, 64 insertions(+), 30 deletions(-) diff --git a/serde/src/de/impls.rs b/serde/src/de/impls.rs index a4e0c29f..bdd547c7 100644 --- a/serde/src/de/impls.rs +++ b/serde/src/de/impls.rs @@ -381,29 +381,30 @@ impl Deserialize for PhantomData where T: Deserialize { macro_rules! seq_impl { ( $ty:ty, - < $($constraints:ident),* >, - $visitor_name:ident, + $visitor_ty:ident < $($typaram:ident : $bound1:ident $(+ $bound2:ident)*),* >, $visitor:ident, $ctor:expr, $with_capacity:expr, $insert:expr ) => { /// A visitor that produces a sequence. - pub struct $visitor_name { - marker: PhantomData, + pub struct $visitor_ty<$($typaram),*> { + marker: PhantomData<$ty>, } - impl $visitor_name { + impl<$($typaram),*> $visitor_ty<$($typaram),*> + where $($typaram: $bound1 $(+ $bound2)*),* + { /// Construct a new sequence visitor. pub fn new() -> Self { - $visitor_name { + $visitor_ty { marker: PhantomData, } } } - impl Visitor for $visitor_name - where T: $($constraints +)*, + impl<$($typaram),*> Visitor for $visitor_ty<$($typaram),*> + where $($typaram: $bound1 $(+ $bound2)*),* { type Value = $ty; @@ -430,13 +431,13 @@ macro_rules! seq_impl { } } - impl Deserialize for $ty - where T: $($constraints +)*, + impl<$($typaram),*> Deserialize for $ty + where $($typaram: $bound1 $(+ $bound2)*),* { fn deserialize(deserializer: &mut D) -> Result<$ty, D::Error> where D: Deserializer, { - deserializer.deserialize_seq($visitor_name::new()) + deserializer.deserialize_seq($visitor_ty::new()) } } } @@ -445,8 +446,7 @@ macro_rules! seq_impl { #[cfg(any(feature = "std", feature = "collections"))] seq_impl!( BinaryHeap, - , - BinaryHeapVisitor, + BinaryHeapVisitor, visitor, BinaryHeap::new(), BinaryHeap::with_capacity(visitor.size_hint().0), @@ -455,8 +455,7 @@ seq_impl!( #[cfg(any(feature = "std", feature = "collections"))] seq_impl!( BTreeSet, - , - BTreeSetVisitor, + BTreeSetVisitor, visitor, BTreeSet::new(), BTreeSet::new(), @@ -465,8 +464,7 @@ seq_impl!( #[cfg(all(feature = "nightly", feature = "collections"))] seq_impl!( EnumSet, - , - EnumSetVisitor, + EnumSetVisitor, visitor, EnumSet::new(), EnumSet::new(), @@ -475,8 +473,7 @@ seq_impl!( #[cfg(any(feature = "std", feature = "collections"))] seq_impl!( LinkedList, - , - LinkedListVisitor, + LinkedListVisitor, visitor, LinkedList::new(), LinkedList::new(), @@ -484,19 +481,18 @@ seq_impl!( #[cfg(feature = "std")] seq_impl!( - HashSet, - , - HashSetVisitor, + HashSet, + HashSetVisitor, visitor, - HashSet::new(), - HashSet::with_capacity(visitor.size_hint().0), + HashSet::with_hasher(S::default()), + HashSet::with_capacity_and_hasher(visitor.size_hint().0, S::default()), HashSet::insert); #[cfg(any(feature = "std", feature = "collections"))] seq_impl!( Vec, - , - VecVisitor, + VecVisitor, visitor, Vec::new(), Vec::with_capacity(visitor.size_hint().0), @@ -505,8 +501,7 @@ seq_impl!( #[cfg(any(feature = "std", feature = "collections"))] seq_impl!( VecDeque, - , - VecDequeVisitor, + VecDequeVisitor, visitor, VecDeque::new(), VecDeque::with_capacity(visitor.size_hint().0), diff --git a/serde/src/ser/impls.rs b/serde/src/ser/impls.rs index a05ae862..dbc58cc3 100644 --- a/serde/src/ser/impls.rs +++ b/serde/src/ser/impls.rs @@ -330,8 +330,9 @@ impl Serialize for EnumSet } #[cfg(feature = "std")] -impl Serialize for HashSet +impl Serialize for HashSet where T: Serialize + Eq + Hash, + H: BuildHasher, { #[inline] fn serialize(&self, serializer: &mut S) -> Result<(), S::Error> diff --git a/serde_tests/tests/macros.rs b/serde_tests/tests/macros.rs index c0b38b3d..2446150b 100644 --- a/serde_tests/tests/macros.rs +++ b/serde_tests/tests/macros.rs @@ -62,6 +62,14 @@ macro_rules! hashset { $(set.insert($value);)+ set } + }; + ($hasher:ident @ $($value:expr),+) => { + { + use std::hash::BuildHasherDefault; + let mut set = HashSet::with_hasher(BuildHasherDefault::<$hasher>::default()); + $(set.insert($value);)+ + set + } } } diff --git a/serde_tests/tests/test_de.rs b/serde_tests/tests/test_de.rs index 8081b9ea..2b9d0035 100644 --- a/serde_tests/tests/test_de.rs +++ b/serde_tests/tests/test_de.rs @@ -287,6 +287,18 @@ declare_tests! { Token::TupleStructStart("Anything", Some(0)), Token::SeqEnd, ], + hashset![FnvHasher @ 1, 2, 3] => vec![ + Token::SeqStart(Some(3)), + Token::SeqSep, + Token::I32(1), + + Token::SeqSep, + Token::I32(2), + + Token::SeqSep, + Token::I32(3), + Token::SeqEnd, + ], } test_vec { Vec::::new() => vec![ diff --git a/serde_tests/tests/test_ser.rs b/serde_tests/tests/test_ser.rs index 2fa8cb9d..5b3f215b 100644 --- a/serde_tests/tests/test_ser.rs +++ b/serde_tests/tests/test_ser.rs @@ -1,4 +1,4 @@ -use std::collections::{BTreeMap, HashMap}; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::net; use std::path::{Path, PathBuf}; use std::str; @@ -147,6 +147,24 @@ declare_ser_tests! { Token::SeqEnd, ], } + test_hashset { + HashSet::::new() => &[ + Token::SeqStart(Some(0)), + Token::SeqEnd, + ], + hashset![1] => &[ + Token::SeqStart(Some(1)), + Token::SeqSep, + Token::I32(1), + Token::SeqEnd, + ], + hashset![FnvHasher @ 1] => &[ + Token::SeqStart(Some(1)), + Token::SeqSep, + Token::I32(1), + Token::SeqEnd, + ], + } test_tuple { (1,) => &[ Token::TupleStart(1), From 1576b5a8a05ae1121d49787291e0fe124a191acd Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 11 Jun 2016 11:15:10 -0700 Subject: [PATCH 5/5] Serde_macros tests depend on fnv --- serde_macros/Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/serde_macros/Cargo.toml b/serde_macros/Cargo.toml index 6ed3af89..dc72e77b 100644 --- a/serde_macros/Cargo.toml +++ b/serde_macros/Cargo.toml @@ -22,6 +22,7 @@ serde_codegen = { version = "^0.7.10", path = "../serde_codegen", default-featur [dev-dependencies] compiletest_rs = "^0.1.1" +fnv = "1.0" rustc-serialize = "^0.3.16" serde = { version = "^0.7.9", path = "../serde" }