Merge branch 'main' into register-macros

Author: zslayton

Diff for: src/element/annotations.rs

@@ -1,7 +1,8 @@
 use crate::element::iterators::{AnnotationsIntoIter, SymbolsIterator};
-use crate::ion_data::IonOrd;
+use crate::ion_data::{IonDataHash, IonDataOrd};
 use crate::Symbol;
 use std::cmp::Ordering;
+use std::hash::Hasher;
 
 /// An ordered sequence of symbols that convey additional, application-specific information about
 /// their associated Ion value.
@@ -141,12 +142,18 @@ impl IntoIterator for Annotations {
     }
 }
 
-impl IonOrd for Annotations {
+impl IonDataOrd for Annotations {
     fn ion_cmp(&self, other: &Self) -> Ordering {
         self.symbols.ion_cmp(&other.symbols)
     }
 }
 
+impl IonDataHash for Annotations {
+    fn ion_data_hash<H: Hasher>(&self, state: &mut H) {
+        self.symbols.ion_data_hash(state)
+    }
+}
+
 /// Defines conversion into [`Annotations`].
 ///
 /// This trait allows us to have a blanket implementations that can cover many type combinations

Diff for: src/element/mod.rs

@@ -24,14 +24,15 @@ pub use annotations::{Annotations, IntoAnnotations};
 pub use sequence::{OwnedSequenceIterator, Sequence};
 use std::cmp::Ordering;
 use std::fmt::{Display, Formatter};
+use std::hash::Hasher;
 use std::io;
 
 use crate::{ion_data, Decimal, Int, IonResult, IonType, Str, Symbol, SymbolRef, Timestamp};
 use crate::{Blob, Bytes, Clob, List, SExp, Struct};
 // Re-export the Value variant types and traits so they can be accessed directly from this module.
 use crate::element::builders::{SequenceBuilder, StructBuilder};
 use crate::element::reader::ElementReader;
-use crate::ion_data::{IonEq, IonOrd};
+use crate::ion_data::{IonDataHash, IonDataOrd, IonEq};
 use crate::lazy::any_encoding::AnyEncoding;
 use crate::lazy::encoding::Encoding;
 use crate::lazy::reader::Reader;
@@ -73,7 +74,7 @@ impl IonEq for Value {
     }
 }
 
-impl IonOrd for Value {
+impl IonDataOrd for Value {
     fn ion_cmp(&self, other: &Self) -> Ordering {
         use Value::*;
 
@@ -117,6 +118,28 @@ impl IonOrd for Value {
     }
 }
 
+impl IonDataHash for Value {
+    fn ion_data_hash<H: Hasher>(&self, state: &mut H) {
+        use Value::*;
+        self.ion_type().ion_data_hash(state);
+        match self {
+            Null(_) => state.write_u8(0),
+            Bool(this) => ion_data::ion_data_hash_bool(*this, state),
+            Int(this) => this.ion_data_hash(state),
+            Float(this) => ion_data::ion_data_hash_f64(*this, state),
+            Decimal(this) => this.ion_data_hash(state),
+            Timestamp(this) => this.ion_data_hash(state),
+            Symbol(this) => this.ion_data_hash(state),
+            String(this) => this.ion_data_hash(state),
+            Clob(this) => this.ion_data_hash(state),
+            Blob(this) => this.ion_data_hash(state),
+            List(this) => this.ion_data_hash(state),
+            SExp(this) => this.ion_data_hash(state),
+            Struct(this) => this.ion_data_hash(state),
+        }
+    }
+}
+
 /// Variants for all _values_ within an [`Element`].
 #[derive(Debug, Clone, PartialEq)]
 pub enum Value {
@@ -336,7 +359,7 @@ impl IonEq for Element {
 // 1. Ion type -- It is a logical way to group Ion values, and it is the cheapest comparison
 // 2. Annotations -- the vast majority of Ion values have few annotations, so this should usually be cheap
 // 3. Value -- compared using IonOrd
-impl IonOrd for Element {
+impl IonDataOrd for Element {
     fn ion_cmp(&self, other: &Self) -> Ordering {
         let ord = self.ion_type().ion_cmp(&other.ion_type());
         if !ord.is_eq() {
@@ -357,6 +380,13 @@ impl IonOrd for Element {
     }
 }
 
+impl IonDataHash for Element {
+    fn ion_data_hash<H: Hasher>(&self, state: &mut H) {
+        self.annotations.ion_data_hash(state);
+        self.value.ion_data_hash(state);
+    }
+}
+
 /// An `(annotations, value)` pair representing an Ion value.
 #[derive(Clone)]
 pub struct Element {

Diff for: src/element/sequence.rs

@@ -1,13 +1,14 @@
 use crate::element::builders::SequenceBuilder;
 use crate::element::iterators::SequenceIterator;
 use crate::element::Element;
-use crate::ion_data::{IonEq, IonOrd};
+use crate::ion_data::{IonDataHash, IonDataOrd, IonEq};
 use crate::lazy::encoding::Encoding;
 use crate::write_config::WriteConfig;
 use crate::IonResult;
 use std::cmp::Ordering;
 use std::collections::VecDeque;
 use std::fmt::{Debug, Formatter};
+use std::hash::Hasher;
 use std::io;
 
 /// An iterable, addressable series of Ion [`Element`]s.
@@ -188,12 +189,18 @@ impl IonEq for Sequence {
     }
 }
 
-impl IonOrd for Sequence {
+impl IonDataOrd for Sequence {
     fn ion_cmp(&self, other: &Self) -> Ordering {
         self.elements.ion_cmp(&other.elements)
     }
 }
 
+impl IonDataHash for Sequence {
+    fn ion_data_hash<H: Hasher>(&self, state: &mut H) {
+        self.elements.ion_data_hash(state)
+    }
+}
+
 #[derive(Clone)]
 pub struct OwnedSequenceIterator {
     elements: VecDeque<Element>,

Diff for: src/ion_data/ion_data_hash.rs

@@ -0,0 +1,58 @@
+use std::hash::Hasher;
+use std::ops::Deref;
+
+/// Trait used for delegating [`Hash`] in [`IonData`](crate::IonData).
+/// Implementations of [`IonDataHash`] must be consistent with [`IonEq`](crate::ion_data::IonEq).
+///
+/// This is _not_ the Ion Hash algorithm. Do not write any code that depends on a specific hash
+/// being produced by a particular value. Do not expect that the hashes will be stable between
+/// two runs of the same application. The only guarantee is that it is consistent with
+/// [`IonEq`](crate::ion_data::IonEq)
+///
+/// This is called `IonDataHash` to avoid ambiguity with the Ion Hash algorithm and its
+/// implementation in this crate.
+pub(crate) trait IonDataHash {
+    fn ion_data_hash<H: Hasher>(&self, state: &mut H);
+}
+
+impl<R: Deref> IonDataHash for R
+where
+    R::Target: IonDataHash,
+{
+    fn ion_data_hash<H: Hasher>(&self, state: &mut H) {
+        R::Target::ion_data_hash(self, state)
+    }
+}
+
+impl<T: IonDataHash> IonDataHash for [T] {
+    fn ion_data_hash<H: Hasher>(&self, state: &mut H) {
+        state.write_usize(self.len());
+        for element in self {
+            T::ion_data_hash(element, state)
+        }
+    }
+}
+
+/// In a roundabout way, implements IonDataHash for [`f64`].
+///
+/// We cannot implement [`IonDataHash`] directly on [`f64`]. If [`IonDataHash`] is implemented directly
+/// on [`f64`], then _any_ blanket impl of [`IonDataHash`] for a standard library trait will cause
+/// `error[E0119]: conflicting implementations of trait` because [`f64`] is an external type (and
+/// "upstream crates may add a new impl of trait `std::ops::Deref` for type `f64` in future versions").
+pub(crate) fn ion_data_hash_f64<H: Hasher>(this: f64, state: &mut H) {
+    if this.is_nan() {
+        // `this` could be positive or negative, signalling or quiet NaN.
+        // Ensure that all NaNs are treated equivalently, as with IonEq.
+        state.write_u64(f64::NAN.to_bits())
+    } else {
+        state.write_u64(this.to_bits())
+    }
+}
+
+/// In a roundabout way, implements IonDataHash for [`bool`].
+///
+/// See docs for [`crate::ion_data::ion_data_hash_f64`] for general rationale. Even though the implementation is trivial,
+/// this function exists to help convey the intention of using Ion equivalence at the call site.
+pub(crate) fn ion_data_hash_bool<H: Hasher>(this: bool, state: &mut H) {
+    state.write_u8(this as u8)
+}

Diff for: src/ion_data/ion_ord.rs

@@ -2,25 +2,28 @@ use std::cmp::Ordering;
 use std::ops::Deref;
 
 /// Trait used for delegating [Ord] and [PartialOrd] in [IonData](crate::IonData).
-/// Implementations of [IonOrd] must be consistent with [IonEq](crate::ion_data::IonEq).
+/// Implementations of [IonDataOrd] must be consistent with [IonEq](crate::ion_data::IonEq).
 /// Since there is no total ordering in the Ion specification, do not write any code that depends on
 /// a specific order being preserved. Only depend on the fact that a total ordering does exist.
-pub(crate) trait IonOrd {
+///
+/// This trait is named `IonDataOrd` (rather than `IonOrd`) to indicate that it is an implementation
+/// detail of `IonData` rather than being part of any Ion specification.
+pub(crate) trait IonDataOrd {
     // Intentionally not public—this trait is exposed via `impl Ord for IonData`.
     // Called ion_cmp to avoid shadowing with Ord::cmp
     fn ion_cmp(&self, other: &Self) -> Ordering;
 }
 
-impl<R: Deref> IonOrd for R
+impl<R: Deref> IonDataOrd for R
 where
-    R::Target: IonOrd,
+    R::Target: IonDataOrd,
 {
     fn ion_cmp(&self, other: &Self) -> Ordering {
         R::Target::ion_cmp(self, other)
     }
 }
 
-impl<T: IonOrd> IonOrd for [T] {
+impl<T: IonDataOrd> IonDataOrd for [T] {
     fn ion_cmp(&self, other: &Self) -> Ordering {
         let mut i0 = self.iter();
         let mut i1 = other.iter();
@@ -42,12 +45,19 @@ impl<T: IonOrd> IonOrd for [T] {
 
 /// Checks Ion ordering for [`f64`].
 ///
-/// We cannot implement [`IonOrd`] directly on [`f64`]. If [`IonOrd`] is implemented directly on
-/// [`f64`], then _any_ blanket impl of [`IonOrd`] for a standard library trait will cause
+/// We cannot implement [`IonDataOrd`] directly on [`f64`]. If [`IonDataOrd`] is implemented directly on
+/// [`f64`], then _any_ blanket impl of [`IonDataOrd`] for a standard library trait will cause
 /// `error[E0119]: conflicting implementations of trait` because [`f64`] is an external type (and
 /// "upstream crates may add a new impl of trait `std::ops::Deref` for type `f64` in future versions").
 pub(crate) fn ion_cmp_f64(this: &f64, that: &f64) -> Ordering {
-    this.total_cmp(that)
+    // f64.total_cmp treats various NaN values as non-equivalent. The Ion specification, however,
+    // treats all NaNs as equivalent, so we need logic to handle that difference.
+    match (this.is_nan(), that.is_nan()) {
+        (false, false) => this.total_cmp(that),
+        (false, true) => Ordering::Less,
+        (true, false) => Ordering::Greater,
+        (true, true) => Ordering::Equal,
+    }
 }
 
 /// Checks Ion ordering for [`bool`].

Diff for: src/ion_data/mod.rs

@@ -1,14 +1,17 @@
+mod ion_data_hash;
 mod ion_eq;
 mod ion_ord;
 
 use std::cmp::Ordering;
 use std::fmt::{Display, Formatter};
+use std::hash::{Hash, Hasher};
 use std::io::Write;
 use std::ops::Deref;
 
 use crate::{Encoding, IonResult, ValueWriter, WriteAsIon, WriteConfig};
+pub(crate) use ion_data_hash::{ion_data_hash_bool, ion_data_hash_f64, IonDataHash};
 pub(crate) use ion_eq::{ion_eq_bool, ion_eq_f64, IonEq};
-pub(crate) use ion_ord::{ion_cmp_bool, ion_cmp_f64, IonOrd};
+pub(crate) use ion_ord::{ion_cmp_bool, ion_cmp_f64, IonDataOrd};
 
 /// A wrapper for lifting Ion compatible data into using Ion-oriented comparisons (versus the Rust
 /// value semantics). This enables the default semantics to be what a Rust user expects for native
@@ -37,6 +40,9 @@ pub(crate) use ion_ord::{ion_cmp_bool, ion_cmp_f64, IonOrd};
 /// __WARNING__—The Ion specification does _not_ define a total ordering over all Ion values. Do
 /// not depend on getting any particular result from [Ord]. Use it only as an opaque total ordering
 /// over all [IonData]. _The algorithm used for [Ord] may change between versions._
+///
+/// __WARNING__—The hashing algorithm is _not_ the same as the
+/// [Ion Hash Algorithm](https://amazon-ion.github.io/ion-hash). _The algorithm used for [Hash] may change between versions._
 #[derive(Debug, Clone)]
 pub struct IonData<T>(T);
 
@@ -78,15 +84,21 @@ impl<T: Display> Display for IonData<T> {
     }
 }
 
-impl<T: IonEq + IonOrd> PartialOrd for IonData<T> {
+impl<T: IonEq + IonDataOrd> PartialOrd for IonData<T> {
     fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
         Some(self.cmp(other))
     }
 }
 
-impl<T: IonEq + IonOrd> Ord for IonData<T> {
+impl<T: IonEq + IonDataOrd> Ord for IonData<T> {
     fn cmp(&self, other: &Self) -> Ordering {
-        IonOrd::ion_cmp(&self.0, &other.0)
+        IonDataOrd::ion_cmp(&self.0, &other.0)
+    }
+}
+
+impl<T: IonDataHash> Hash for IonData<T> {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        IonDataHash::ion_data_hash(&self.0, state)
     }
 }
 
@@ -116,12 +128,13 @@ impl<T: WriteAsIon> WriteAsIon for IonData<T> {
 
 #[cfg(test)]
 mod tests {
-    use crate::ion_data::{IonEq, IonOrd};
+    use crate::ion_data::{IonDataHash, IonDataOrd, IonEq};
     use crate::lazy::encoding::TextEncoding_1_0;
     use crate::{Element, IonData, Symbol, WriteConfig};
     use rstest::*;
     use std::boxed::Box;
     use std::fmt::Debug;
+    use std::hash::{DefaultHasher, Hash, Hasher};
     use std::pin::Pin;
     use std::rc::Rc;
     use std::sync::Arc;
@@ -137,12 +150,17 @@ mod tests {
     #[case::vec_element(|s| Element::read_all(s).unwrap().into() )]
     #[case::rc_vec_element(|s| Rc::new(Element::read_all(s).unwrap()).into() )]
     #[case::box_pin_rc_vec_box_arc_element(|s| Box::new(Pin::new(Rc::new(vec![Box::new(Arc::new(Element::read_one(s).unwrap()))]))).into() )]
-    fn can_wrap_data<T: IonEq + IonOrd + Debug>(
+    fn can_wrap_data<T: IonEq + IonDataOrd + IonDataHash + Debug>(
         #[case] the_fn: impl Fn(&'static str) -> IonData<T>,
     ) {
         let id1: IonData<_> = the_fn("nan");
         let id2: IonData<_> = the_fn("nan");
         assert_eq!(id1, id2);
+        let mut h1 = DefaultHasher::default();
+        let mut h2 = DefaultHasher::default();
+        id1.hash(&mut h1);
+        id2.hash(&mut h2);
+        assert_eq!(h1.finish(), h2.finish());
 
         let id1: IonData<_> = the_fn("1.00");
         let id2: IonData<_> = the_fn("1.0");

Diff for: src/types/bytes.rs

@@ -1,5 +1,6 @@
-use crate::ion_data::{IonEq, IonOrd};
+use crate::ion_data::{IonDataHash, IonDataOrd, IonEq};
 use std::cmp::Ordering;
+use std::hash::{Hash, Hasher};
 
 /// An owned, immutable byte array.
 /// ```rust
@@ -35,12 +36,18 @@ impl IonEq for Bytes {
     }
 }
 
-impl IonOrd for Bytes {
+impl IonDataOrd for Bytes {
     fn ion_cmp(&self, other: &Self) -> Ordering {
         self.cmp(other)
     }
 }
 
+impl IonDataHash for Bytes {
+    fn ion_data_hash<H: Hasher>(&self, state: &mut H) {
+        self.data.hash(state);
+    }
+}
+
 impl PartialEq<[u8]> for Bytes {
     fn eq(&self, other: &[u8]) -> bool {
         self.as_ref().eq(other)

Diff for: src/types/decimal/coefficient.rs

@@ -14,8 +14,8 @@ use crate::{Int, UInt};
 /// When the magnitude is zero, the `Sign` can be used to distinguish between -0 and 0.
 #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug)]
 pub enum Sign {
-    Negative,
-    Positive,
+    Negative = -1,
+    Positive = 1,
 }
 
 /// A signed integer that can be used as the coefficient of a [`Decimal`](crate::Decimal) value.
@@ -293,4 +293,10 @@ mod coefficient_tests {
         assert_eq!(Int::try_from(Coefficient::new(0)), Ok(Int::from(0)));
         assert!(Int::try_from(Coefficient::negative_zero()).is_err());
     }
+
+    #[test]
+    fn test_casting_sign() {
+        assert_eq!(-1, Sign::Negative as i8);
+        assert_eq!(1, Sign::Positive as i8);
+    }
 }