Feat: Added external IndexMap and IndexSet types#149
Feat: Added external IndexMap and IndexSet types#149adpthegreat wants to merge 1 commit intoanza-xyz:masterfrom
Conversation
|
Hello, any plans to merge this? |
|
have to rebase and tag a maintainer brb |
| thiserror = { version = "2.0.16", default-features = false } | ||
| wincode-derive = { version = "0.3.1", path = "../wincode-derive", optional = true } | ||
| uuid = { version = "1.20.0", optional = true, default-features = false } | ||
| indexmap = { version = "2.13.0", features = ["arbitrary", "serde" ], optional = true } |
There was a problem hiding this comment.
We shouldn't require arbitrary and serde in the [dependencies] section. If features are needed for tests, they should be included in [dev-dependencies] specifically.
It also doesn't appear that the arbitrary feature is used.
| thiserror = { version = "2.0.16", default-features = false } | ||
| wincode-derive = { version = "0.3.1", path = "../wincode-derive", optional = true } | ||
| uuid = { version = "1.20.0", optional = true, default-features = false } | ||
| indexmap = { version = "2.13.0", features = ["arbitrary", "serde" ], optional = true } |
There was a problem hiding this comment.
Additionally, IndexMap works without std, so we should accommodate that as well.
| }; | ||
|
|
||
| #[cfg(feature = "indexmap")] | ||
| unsafe impl<C: Config, K, V> SchemaWrite<C> for IndexMap<K, V> |
There was a problem hiding this comment.
Should make this generic over a BuildHasher, like the HashMap implementation.
| } | ||
|
|
||
| #[cfg(feature = "indexmap")] | ||
| unsafe impl<'de, C: Config, K, V> SchemaRead<'de, C> for IndexMap<K, V> |
There was a problem hiding this comment.
Should make this generic over a BuildHasher, like the HashMap implementation.
| } | ||
|
|
||
| #[cfg(feature = "indexmap")] | ||
| unsafe impl<C: Config, K> SchemaWrite<C> for IndexSet<K> |
There was a problem hiding this comment.
Should make this generic over a BuildHasher, like the HashSet implementation.
| } | ||
|
|
||
| #[cfg(feature = "indexmap")] | ||
| unsafe impl<'de, C: Config, K> SchemaRead<'de, C> for IndexSet<K> |
There was a problem hiding this comment.
Should make this generic over a BuildHasher, like the HashSet implementation.
| + (key_size + value_size) * len; | ||
| // SAFETY: `K::TYPE_META` and `V::TYPE_META` specify static sizes, so `len` writes of `(K::Src, V::Src)` | ||
| // and `<BincodeLen>::write` will write `needed` bytes, fully initializing the trusted window. | ||
| let writer = &mut unsafe { writer.as_trusted_for(needed) }?; |
There was a problem hiding this comment.
Should avoid using &mut on writers and prefer by_ref.
See #177
| ) = (K::TYPE_META, V::TYPE_META) | ||
| { | ||
| let reader = | ||
| &mut unsafe { reader.as_trusted_for((key_size + value_size) * len) }?; |
There was a problem hiding this comment.
Should avoid using &mut on readers and prefer by_ref.
See #177
| // SAFETY: `K::TYPE_META` specifies a static size, so `len` reads of `T::Dst` | ||
| // will consume `size * len` bytes, fully consuming the trusted window. | ||
| let reader = | ||
| &mut unsafe { reader.as_trusted_for(size * len) }?; |
There was a problem hiding this comment.
Should avoid using &mut on readers and prefer by_ref.
See #177
|
Closing for inactivity |
3 participants
This PR adds external types
IndexMapandIndexSetfrom theIndexMapcrate gated behind theindexmapfeature flag with appropriate tests, it is heavily based on theimpl_seqmacro which is the macro to implement sequential typesWill add more tests to test serialization and deserialization with different ordering