diff --git a/zvariant/src/array.rs b/zvariant/src/array.rs index 266373151..fca8a656b 100644 --- a/zvariant/src/array.rs +++ b/zvariant/src/array.rs @@ -17,7 +17,7 @@ use crate::{ /// /// [`Value`]: enum.Value.html#variant.Array /// [`Vec`]: https://doc.rust-lang.org/std/vec/struct.Vec.html -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, Hash, PartialEq, PartialOrd, Eq, Ord)] pub struct Array<'a> { element_signature: Signature<'a>, elements: Vec>, diff --git a/zvariant/src/dict.rs b/zvariant/src/dict.rs index 360d48d56..78785718d 100644 --- a/zvariant/src/dict.rs +++ b/zvariant/src/dict.rs @@ -1,5 +1,5 @@ use std::{ - collections::HashMap, + collections::{BTreeSet, HashMap}, fmt::{Display, Write}, hash::BuildHasher, }; @@ -15,9 +15,9 @@ use crate::{value_display_fmt, Basic, DynamicType, Error, Signature, Type, Value /// /// [`Value`]: enum.Value.html#variant.Dict /// [`HashMap`]: https://doc.rust-lang.org/std/collections/struct.HashMap.html -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, Hash, PartialEq, PartialOrd, Eq, Ord)] pub struct Dict<'k, 'v> { - entries: Vec>, + entries: BTreeSet>, key_signature: Signature<'k>, value_signature: Signature<'v>, // should use a separate lifetime or everything should use the same but API break. @@ -32,7 +32,7 @@ impl<'k, 'v> Dict<'k, 'v> { let signature = create_signature(&key_signature, &value_signature); Self { - entries: vec![], + entries: BTreeSet::new(), key_signature, value_signature, signature, @@ -56,7 +56,7 @@ impl<'k, 'v> Dict<'k, 'v> { check_child_value_signature!(self.key_signature, key.value_signature(), "key"); check_child_value_signature!(self.value_signature, value.value_signature(), "value"); - self.entries.push(DictEntry { key, value }); + self.entries.insert(DictEntry { key, value }); Ok(()) } @@ -70,7 +70,7 @@ impl<'k, 'v> Dict<'k, 'v> { check_child_value_signature!(self.key_signature, K::signature(), "key"); check_child_value_signature!(self.value_signature, value.dynamic_signature(), "value"); - self.entries.push(DictEntry { + self.entries.insert(DictEntry { key: Value::new(key), value: Value::new(value), }); @@ -131,7 +131,7 @@ impl<'k, 'v> Dict<'k, 'v> { let value_signature = signature.slice(3..signature.len() - 1); Self { - entries: vec![], + entries: BTreeSet::new(), key_signature, value_signature, signature, @@ -261,7 +261,7 @@ where // TODO: Conversion of Dict from/to BTreeMap -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, Hash, PartialOrd, Ord, PartialEq, Eq)] struct DictEntry<'k, 'v> { key: Value<'k>, value: Value<'v>, diff --git a/zvariant/src/fd.rs b/zvariant/src/fd.rs index 4c8c367fd..c4fbfe4a9 100644 --- a/zvariant/src/fd.rs +++ b/zvariant/src/fd.rs @@ -16,7 +16,7 @@ use crate::{Basic, EncodingFormat, Signature, Type}; /// [`Deserialize`]: https://docs.serde.rs/serde/de/trait.Deserialize.html /// [deserializer]: fn.from_slice_fds.html /// [serializer]: fn.to_bytes_fds.html -#[derive(Debug, PartialEq, Eq, Hash, Clone, Copy)] +#[derive(Debug, PartialEq, PartialOrd, Ord, Eq, Hash, Clone, Copy)] pub struct Fd(io::RawFd); macro_rules! fd_impl { diff --git a/zvariant/src/lib.rs b/zvariant/src/lib.rs index 86fe1fe24..c1a366f88 100644 --- a/zvariant/src/lib.rs +++ b/zvariant/src/lib.rs @@ -1177,7 +1177,7 @@ mod tests { let v: Value<'_> = dict.into(); assert_eq!(v.value_signature(), "a{sv}"); let encoded = to_bytes(ctxt, &v).unwrap(); - assert_eq!(dbg!(encoded.len()), 68); + assert_eq!(dbg!(encoded.len()), 66); let v: Value<'_> = encoded.deserialize().unwrap().0; if let Value::Dict(dict) = v { assert_eq!( @@ -1256,6 +1256,21 @@ mod tests { ); } + #[test] + fn dict_compare() { + // the order in which a dict has been constructed must not play a role + // https://github.com/dbus2/zbus/issues/484 + let mut dict1 = Dict::new(<&str>::signature(), Value::signature()); + dict1.add("first", Value::new("value")).unwrap(); + dict1.add("second", Value::new("value")).unwrap(); + + let mut dict2 = Dict::new(<&str>::signature(), Value::signature()); + dict2.add("second", Value::new("value")).unwrap(); + dict2.add("first", Value::new("value")).unwrap(); + + assert_eq!(dict1, dict2); + } + #[test] fn value_value() { let ctxt = Context::::new_dbus(0); diff --git a/zvariant/src/maybe.rs b/zvariant/src/maybe.rs index e5547ba8f..63c1c800c 100644 --- a/zvariant/src/maybe.rs +++ b/zvariant/src/maybe.rs @@ -9,7 +9,7 @@ use crate::{value_display_fmt, Error, Signature, Type, Value}; /// API is provided to convert from, and to `Option`. /// /// [`Value`]: enum.Value.html -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)] pub struct Maybe<'a> { value: Box>>, value_signature: Signature<'a>, diff --git a/zvariant/src/object_path.rs b/zvariant/src/object_path.rs index 4efca673d..e042526af 100644 --- a/zvariant/src/object_path.rs +++ b/zvariant/src/object_path.rs @@ -32,7 +32,7 @@ use crate::{Basic, EncodingFormat, Error, Result, Signature, Str, Type}; /// ObjectPath::try_from("/end/with/slash/").unwrap_err(); /// ObjectPath::try_from("/ha.d").unwrap_err(); /// ``` -#[derive(PartialEq, Eq, Hash, Clone)] +#[derive(PartialEq, Eq, Hash, Clone, PartialOrd, Ord)] pub struct ObjectPath<'a>(Str<'a>); assert_impl_all!(ObjectPath<'_>: Send, Sync, Unpin); diff --git a/zvariant/src/signature.rs b/zvariant/src/signature.rs index 476158640..92516856f 100644 --- a/zvariant/src/signature.rs +++ b/zvariant/src/signature.rs @@ -1,5 +1,7 @@ use core::{ + cmp::Ordering, fmt::{self, Debug, Display, Formatter}, + hash::{Hash, Hasher}, str, }; use serde::{ @@ -20,7 +22,7 @@ use crate::{signature_parser::SignatureParser, Basic, EncodingFormat, Error, Res // breakage. // // [`bytes::Bytes`]: https://docs.rs/bytes/0.5.6/bytes/struct.Bytes.html -#[derive(PartialEq, Eq, Hash, Clone)] +#[derive(Debug, Clone)] enum Bytes<'b> { Borrowed(&'b [u8]), Static(&'static [u8]), @@ -46,7 +48,7 @@ impl<'b> Bytes<'b> { } } -impl<'b> std::ops::Deref for Bytes<'b> { +impl std::ops::Deref for Bytes<'_> { type Target = [u8]; fn deref(&self) -> &[u8] { @@ -58,6 +60,32 @@ impl<'b> std::ops::Deref for Bytes<'b> { } } +impl Eq for Bytes<'_> {} + +impl PartialEq for Bytes<'_> { + fn eq(&self, other: &Self) -> bool { + **self == **other + } +} + +impl Ord for Bytes<'_> { + fn cmp(&self, other: &Self) -> Ordering { + (**self).cmp(&**other) + } +} + +impl PartialOrd for Bytes<'_> { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Hash for Bytes<'_> { + fn hash(&self, state: &mut H) { + (**self).hash(state) + } +} + /// String that [identifies] the type of an encoded value. /// /// # Examples @@ -96,7 +124,7 @@ impl<'b> std::ops::Deref for Bytes<'b> { /// /// [identifies]: https://dbus.freedesktop.org/doc/dbus-specification.html#type-system /// [`slice`]: #method.slice -#[derive(Hash, Clone)] +#[derive(Hash, Clone, PartialOrd, Ord)] pub struct Signature<'a> { bytes: Bytes<'a>, pos: usize, @@ -551,7 +579,31 @@ impl std::fmt::Display for OwnedSignature { #[cfg(test)] mod tests { - use super::Signature; + use super::{Bytes, Signature}; + use std::sync::Arc; + + #[test] + fn bytes_equality() { + let borrowed1 = Bytes::Borrowed(b"foo"); + let borrowed2 = Bytes::Borrowed(b"foo"); + let static1 = Bytes::Static(b"foo"); + let static2 = Bytes::Static(b"foo"); + let owned1 = Bytes::Owned(Arc::new(*b"foo")); + let owned2 = Bytes::Owned(Arc::new(*b"foo")); + + assert_eq!(borrowed1, borrowed2); + assert_eq!(static1, static2); + assert_eq!(owned1, owned2); + + assert_eq!(borrowed1, static1); + assert_eq!(static1, borrowed1); + + assert_eq!(static1, owned1); + assert_eq!(owned1, static1); + + assert_eq!(borrowed1, owned1); + assert_eq!(owned1, borrowed1); + } #[test] fn signature_slicing() { diff --git a/zvariant/src/structure.rs b/zvariant/src/structure.rs index 18d783a92..b33e143a7 100644 --- a/zvariant/src/structure.rs +++ b/zvariant/src/structure.rs @@ -155,7 +155,7 @@ impl<'de> Visitor<'de> for StructureVisitor<'de> { /// API is provided to convert from, and to tuples. /// /// [`Value`]: enum.Value.html -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)] pub struct Structure<'a> { fields: Vec>, signature: Signature<'a>, diff --git a/zvariant/src/value.rs b/zvariant/src/value.rs index 01b1de9f3..6639368bb 100644 --- a/zvariant/src/value.rs +++ b/zvariant/src/value.rs @@ -1,7 +1,10 @@ -use core::str; -use std::{ +use core::{ + cmp::Ordering, fmt::{Display, Write}, + hash::{Hash, Hasher}, marker::PhantomData, + mem::discriminant, + str, }; use serde::{ @@ -72,7 +75,7 @@ use crate::Fd; /// ``` /// /// [D-Bus specification]: https://dbus.freedesktop.org/doc/dbus-specification.html#container-types -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq, PartialOrd)] pub enum Value<'a> { // Simple types U8(u8), @@ -100,6 +103,50 @@ pub enum Value<'a> { Fd(Fd), } +impl Hash for Value<'_> { + fn hash(&self, state: &mut H) { + discriminant(self).hash(state); + match self { + Self::U8(inner) => inner.hash(state), + Self::Bool(inner) => inner.hash(state), + Self::I16(inner) => inner.hash(state), + Self::U16(inner) => inner.hash(state), + Self::I32(inner) => inner.hash(state), + Self::U32(inner) => inner.hash(state), + Self::I64(inner) => inner.hash(state), + Self::U64(inner) => inner.hash(state), + Self::F64(inner) => inner.to_le_bytes().hash(state), + Self::Str(inner) => inner.hash(state), + Self::Signature(inner) => inner.hash(state), + Self::ObjectPath(inner) => inner.hash(state), + Self::Value(inner) => inner.hash(state), + Self::Array(inner) => inner.hash(state), + Self::Dict(inner) => inner.hash(state), + Self::Structure(inner) => inner.hash(state), + #[cfg(feature = "gvariant")] + Self::Maybe(inner) => inner.hash(state), + #[cfg(unix)] + Self::Fd(inner) => inner.hash(state), + } + } +} + +impl Eq for Value<'_> {} + +impl Ord for Value<'_> { + fn cmp(&self, other: &Self) -> Ordering { + self.partial_cmp(other) + .unwrap_or_else(|| match (self, other) { + (Self::F64(lhs), Self::F64(rhs)) => lhs.total_cmp(rhs), + // `partial_cmp` returns `Some(_)` if either the discriminants are different + // or if both the left hand side and right hand side is `Self::F64(_)`, + // because `f64` is the only type in this enum, that does not implement `Ord`. + // This `match`-arm is therefore unreachable. + _ => unreachable!(), + }) + } +} + assert_impl_all!(Value<'_>: Send, Sync, Unpin); macro_rules! serialize_value {