From 4470237d71c3a2bc3f6cf0ea89912bd37f0890a4 Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Thu, 2 Feb 2023 14:51:47 -0500 Subject: [PATCH 01/30] untested parsing --- Cargo.toml | 3 ++- src/decimal128.rs | 60 ++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index db54bf7c..8329dcaf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -67,6 +67,7 @@ uuid = { version = "1.1.2", features = ["serde", "v4"] } serde_bytes = "0.11.5" serde_with = { version = "1", optional = true } time = { version = "0.3.9", features = ["formatting", "parsing", "macros", "large-dates"] } +bitvec = "1.0.1" [dev-dependencies] assert_matches = "1.2" @@ -78,4 +79,4 @@ chrono = { version = "0.4", features = ["serde", "clock", "std"], default-featur [package.metadata.docs.rs] all-features = true -rustdoc-args = ["--cfg", "docsrs"] \ No newline at end of file +rustdoc-args = ["--cfg", "docsrs"] diff --git a/src/decimal128.rs b/src/decimal128.rs index 533b10dd..b5045726 100644 --- a/src/decimal128.rs +++ b/src/decimal128.rs @@ -2,6 +2,8 @@ use std::{convert::TryInto, fmt}; +use bitvec::prelude::*; + /// Struct representing a BSON Decimal128 type. /// /// Currently, this type can only be used to round-trip through BSON. See @@ -9,7 +11,7 @@ use std::{convert::TryInto, fmt}; #[derive(Copy, Clone, PartialEq)] pub struct Decimal128 { /// BSON bytes containing the decimal128. Stored for round tripping. - pub(crate) bytes: [u8; 128 / 8], + pub(crate) bytes: [u8; 16], } impl Decimal128 { @@ -42,3 +44,59 @@ impl fmt::Display for Decimal128 { write!(f, "{:?}", self) } } + +#[derive(Clone)] +struct ParsedDecimal128 { + sign: bool, + kind: Decimal128Kind, +} + +#[derive(Clone)] +enum Decimal128Kind { + NaN { signalling: bool }, + Infinity, + Finite { + exponent: BitVec, + significand: BitVec, + } +} + +impl ParsedDecimal128 { + fn new(source: &Decimal128) -> Self { + let bits = source.bytes.view_bits::(); + let sign = bits[0]; + let combination = &bits[1..5]; + let kind = if combination[0..4].all() { + // Special value + if combination[4] { + if bits[5] { + Decimal128Kind::NaN { signalling: true } + } else { + Decimal128Kind::NaN { signalling: false } + } + } else { + Decimal128Kind::Infinity + } + } else { + // Finite + let mut exponent = bitvec![u8, Msb0;]; + let mut significand = bitvec![u8, Msb0;]; + // Extract initial bits from combination + if combination[0..1].all() { + exponent.extend(&combination[2..4]); + significand.extend(bits![u8, Msb0; 1, 0, 0]); + significand.push(combination[4]); + } else { + exponent.extend(&combination[0..1]); + significand.push(false); + significand.extend(&combination[2..5]); + } + // Exponent continuation + exponent.extend(&bits[5..17]); + // Coefficient continuation + significand.extend(&bits[17..]); + Decimal128Kind::Finite { exponent, significand } + }; + ParsedDecimal128 { sign, kind } + } +} \ No newline at end of file From a10faa3ae168e6f6ba6e9f87f5b6d921562ee3a6 Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Fri, 3 Feb 2023 11:50:57 -0500 Subject: [PATCH 02/30] partial format impl and a passing test --- src/decimal128.rs | 68 ++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 56 insertions(+), 12 deletions(-) diff --git a/src/decimal128.rs b/src/decimal128.rs index b5045726..dee6ab2a 100644 --- a/src/decimal128.rs +++ b/src/decimal128.rs @@ -45,13 +45,13 @@ impl fmt::Display for Decimal128 { } } -#[derive(Clone)] +#[derive(Debug, Clone, PartialEq)] struct ParsedDecimal128 { sign: bool, kind: Decimal128Kind, } -#[derive(Clone)] +#[derive(Debug, Clone, PartialEq)] enum Decimal128Kind { NaN { signalling: bool }, Infinity, @@ -64,16 +64,16 @@ enum Decimal128Kind { impl ParsedDecimal128 { fn new(source: &Decimal128) -> Self { let bits = source.bytes.view_bits::(); + let sign = bits[0]; - let combination = &bits[1..5]; + let combination = &bits[1..6]; + let exp_continuation = &bits[6..18]; + let sig_continuation = &bits[18..]; + let kind = if combination[0..4].all() { // Special value if combination[4] { - if bits[5] { - Decimal128Kind::NaN { signalling: true } - } else { - Decimal128Kind::NaN { signalling: false } - } + Decimal128Kind::NaN { signalling: exp_continuation[0] } } else { Decimal128Kind::Infinity } @@ -91,12 +91,56 @@ impl ParsedDecimal128 { significand.push(false); significand.extend(&combination[2..5]); } - // Exponent continuation - exponent.extend(&bits[5..17]); - // Coefficient continuation - significand.extend(&bits[17..]); + exponent.extend(exp_continuation); + significand.extend(sig_continuation); Decimal128Kind::Finite { exponent, significand } }; ParsedDecimal128 { sign, kind } } +} + +impl fmt::Display for ParsedDecimal128 { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + if self.sign { + write!(f, "-")?; + } + match &self.kind { + Decimal128Kind::NaN { signalling } => { + if *signalling { + write!(f, "s")?; + } + write!(f, "NaN")?; + } + Decimal128Kind::Infinity => write!(f, "Infinity")?, + Decimal128Kind::Finite { exponent, significand } => { + let coeff_str = format!("{}", significand.load_be::()); + let exp_val = exponent.load_be::(); + let adj_exp = exp_val + (coeff_str.len() as i16) - 1; + if exp_val <= 0 && adj_exp >= -6 { + // Plain notation + } else { + // Exponential notation + } + } + } + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn negative_infinity() { + let val = Decimal128::from_bytes([ + 0xf8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + ]); + let parsed = ParsedDecimal128::new(&val); + assert_eq!(parsed, ParsedDecimal128 { + sign: true, + kind: Decimal128Kind::Infinity, + }); + } } \ No newline at end of file From 3a63afca55ef2347c9a7f5c0ec8d08677ee3ea38 Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Fri, 3 Feb 2023 13:57:17 -0500 Subject: [PATCH 03/30] rework parsing --- src/decimal128.rs | 115 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 86 insertions(+), 29 deletions(-) diff --git a/src/decimal128.rs b/src/decimal128.rs index dee6ab2a..ee420b3f 100644 --- a/src/decimal128.rs +++ b/src/decimal128.rs @@ -51,48 +51,59 @@ struct ParsedDecimal128 { kind: Decimal128Kind, } +type Order = Msb0; + #[derive(Debug, Clone, PartialEq)] enum Decimal128Kind { NaN { signalling: bool }, Infinity, Finite { - exponent: BitVec, - significand: BitVec, + exponent: BitVec, + significand: BitVec, + } +} + +macro_rules! pdbg { + ($expr: expr) => { + { + let val = $expr; + println!("{} = {}", stringify!($expr), val); + val + } } } impl ParsedDecimal128 { fn new(source: &Decimal128) -> Self { - let bits = source.bytes.view_bits::(); + let tmp: Vec<_> = source.bytes.iter().copied().rev().collect(); + let bits = tmp.view_bits::(); + pdbg!(&bits); let sign = bits[0]; - let combination = &bits[1..6]; - let exp_continuation = &bits[6..18]; - let sig_continuation = &bits[18..]; - - let kind = if combination[0..4].all() { + let kind = if bits[1..5].all() { // Special value - if combination[4] { - Decimal128Kind::NaN { signalling: exp_continuation[0] } + if bits[5] { + Decimal128Kind::NaN { signalling: bits[6] } } else { Decimal128Kind::Infinity } } else { - // Finite - let mut exponent = bitvec![u8, Msb0;]; - let mut significand = bitvec![u8, Msb0;]; - // Extract initial bits from combination - if combination[0..1].all() { - exponent.extend(&combination[2..4]); - significand.extend(bits![u8, Msb0; 1, 0, 0]); - significand.push(combination[4]); + // Finite value + let mut exponent = bitvec![u8, Order;]; + let mut significand = bitvec![u8, Order;]; + + if bits[1..3].all() { + exponent.extend(&bits[3..17]); + significand.extend(bits![1, 0, 0]); + significand.extend(&bits[17..]); } else { - exponent.extend(&combination[0..1]); - significand.push(false); - significand.extend(&combination[2..5]); + exponent.extend(&bits[1..15]); + significand.extend(bits![0]); + significand.extend(&bits[15..]); } - exponent.extend(exp_continuation); - significand.extend(sig_continuation); + + pdbg!(&exponent); + pdbg!(&significand); Decimal128Kind::Finite { exponent, significand } }; ParsedDecimal128 { sign, kind } @@ -129,18 +140,64 @@ impl fmt::Display for ParsedDecimal128 { #[cfg(test)] mod tests { + use crate::Document; + use super::*; #[test] - fn negative_infinity() { - let val = Decimal128::from_bytes([ - 0xf8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - ]); - let parsed = ParsedDecimal128::new(&val); + fn nan() { + let canonical_bson = hex::decode(&"180000001364000000000000000000000000000000007C00").unwrap(); + let val = crate::from_slice::(&canonical_bson).unwrap(); + let parsed = ParsedDecimal128::new(&val.get_decimal128("d").unwrap()); + assert_eq!(parsed, ParsedDecimal128 { + sign: false, + kind: Decimal128Kind::NaN { signalling: false }, + }); + } + + #[test] + fn negative_nan() { + let canonical_bson = hex::decode(&"18000000136400000000000000000000000000000000FC00").unwrap(); + let val = crate::from_slice::(&canonical_bson).unwrap(); + let parsed = ParsedDecimal128::new(&val.get_decimal128("d").unwrap()); assert_eq!(parsed, ParsedDecimal128 { sign: true, + kind: Decimal128Kind::NaN { signalling: false }, + }); + } + + #[test] + fn snan() { + let canonical_bson = hex::decode(&"180000001364000000000000000000000000000000007E00").unwrap(); + let val = crate::from_slice::(&canonical_bson).unwrap(); + let parsed = ParsedDecimal128::new(&val.get_decimal128("d").unwrap()); + assert_eq!(parsed, ParsedDecimal128 { + sign: false, + kind: Decimal128Kind::NaN { signalling: true }, + }); + } + + #[test] + fn inf() { + let canonical_bson = hex::decode(&"180000001364000000000000000000000000000000007800").unwrap(); + let val = crate::from_slice::(&canonical_bson).unwrap(); + let parsed = ParsedDecimal128::new(&val.get_decimal128("d").unwrap()); + assert_eq!(parsed, ParsedDecimal128 { + sign: false, kind: Decimal128Kind::Infinity, }); } + + #[test] + fn zero() { + let canonical_bson = hex::decode(&"180000001364000000000000000000000000000000403000").unwrap(); + let val = crate::from_slice::(&canonical_bson).unwrap(); + let parsed = ParsedDecimal128::new(&val.get_decimal128("d").unwrap()); + let (_exp, sig) = if let Decimal128Kind::Finite { exponent, significand } = parsed.kind { + (exponent, significand) + } else { + panic!("expected finite, got {:?}", parsed); + }; + assert_eq!(sig.load_le::(), 0); + } } \ No newline at end of file From 6e22a6d49bcc4ef3d23b8a2c6dec1752ca48f2fa Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Fri, 3 Feb 2023 15:15:33 -0500 Subject: [PATCH 04/30] tidy --- src/decimal128.rs | 69 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 47 insertions(+), 22 deletions(-) diff --git a/src/decimal128.rs b/src/decimal128.rs index ee420b3f..bab89eb0 100644 --- a/src/decimal128.rs +++ b/src/decimal128.rs @@ -58,8 +58,32 @@ enum Decimal128Kind { NaN { signalling: bool }, Infinity, Finite { - exponent: BitVec, - significand: BitVec, + exponent: Exponent, + significand: Significand, + } +} + +#[derive(Debug, Clone, PartialEq)] +struct Exponent(BitVec); + +impl Exponent { + const BIAS: i16 = 6176; + + fn raw(&self) -> u16 { + self.0.load_be::() + } + + fn value(&self) -> i16 { + (self.raw() as i16) - Self::BIAS + } +} + +#[derive(Debug, Clone, PartialEq)] +struct Significand(BitVec); + +impl Significand { + fn value(&self) -> u128 { + self.0.load_be::() } } @@ -74,6 +98,7 @@ macro_rules! pdbg { } impl ParsedDecimal128 { + fn new(source: &Decimal128) -> Self { let tmp: Vec<_> = source.bytes.iter().copied().rev().collect(); let bits = tmp.view_bits::(); @@ -104,7 +129,10 @@ impl ParsedDecimal128 { pdbg!(&exponent); pdbg!(&significand); - Decimal128Kind::Finite { exponent, significand } + Decimal128Kind::Finite { + exponent: Exponent(exponent), + significand: Significand(significand), + } }; ParsedDecimal128 { sign, kind } } @@ -124,8 +152,8 @@ impl fmt::Display for ParsedDecimal128 { } Decimal128Kind::Infinity => write!(f, "Infinity")?, Decimal128Kind::Finite { exponent, significand } => { - let coeff_str = format!("{}", significand.load_be::()); - let exp_val = exponent.load_be::(); + let coeff_str = format!("{}", significand.value()); + let exp_val = exponent.value(); let adj_exp = exp_val + (coeff_str.len() as i16) - 1; if exp_val <= 0 && adj_exp >= -6 { // Plain notation @@ -144,11 +172,15 @@ mod tests { use super::*; + fn dec_from_hex(s: &str) -> ParsedDecimal128 { + let bytes = hex::decode(s).unwrap(); + let d = crate::from_slice::(&bytes).unwrap(); + ParsedDecimal128::new(&d.get_decimal128("d").unwrap()) + } + #[test] fn nan() { - let canonical_bson = hex::decode(&"180000001364000000000000000000000000000000007C00").unwrap(); - let val = crate::from_slice::(&canonical_bson).unwrap(); - let parsed = ParsedDecimal128::new(&val.get_decimal128("d").unwrap()); + let parsed = dec_from_hex("180000001364000000000000000000000000000000007C00"); assert_eq!(parsed, ParsedDecimal128 { sign: false, kind: Decimal128Kind::NaN { signalling: false }, @@ -157,9 +189,7 @@ mod tests { #[test] fn negative_nan() { - let canonical_bson = hex::decode(&"18000000136400000000000000000000000000000000FC00").unwrap(); - let val = crate::from_slice::(&canonical_bson).unwrap(); - let parsed = ParsedDecimal128::new(&val.get_decimal128("d").unwrap()); + let parsed = dec_from_hex("18000000136400000000000000000000000000000000FC00"); assert_eq!(parsed, ParsedDecimal128 { sign: true, kind: Decimal128Kind::NaN { signalling: false }, @@ -168,9 +198,7 @@ mod tests { #[test] fn snan() { - let canonical_bson = hex::decode(&"180000001364000000000000000000000000000000007E00").unwrap(); - let val = crate::from_slice::(&canonical_bson).unwrap(); - let parsed = ParsedDecimal128::new(&val.get_decimal128("d").unwrap()); + let parsed = dec_from_hex("180000001364000000000000000000000000000000007E00"); assert_eq!(parsed, ParsedDecimal128 { sign: false, kind: Decimal128Kind::NaN { signalling: true }, @@ -179,9 +207,7 @@ mod tests { #[test] fn inf() { - let canonical_bson = hex::decode(&"180000001364000000000000000000000000000000007800").unwrap(); - let val = crate::from_slice::(&canonical_bson).unwrap(); - let parsed = ParsedDecimal128::new(&val.get_decimal128("d").unwrap()); + let parsed = dec_from_hex("180000001364000000000000000000000000000000007800"); assert_eq!(parsed, ParsedDecimal128 { sign: false, kind: Decimal128Kind::Infinity, @@ -190,14 +216,13 @@ mod tests { #[test] fn zero() { - let canonical_bson = hex::decode(&"180000001364000000000000000000000000000000403000").unwrap(); - let val = crate::from_slice::(&canonical_bson).unwrap(); - let parsed = ParsedDecimal128::new(&val.get_decimal128("d").unwrap()); - let (_exp, sig) = if let Decimal128Kind::Finite { exponent, significand } = parsed.kind { + let parsed = dec_from_hex("180000001364000000000000000000000000000000403000"); + let (exp, sig) = if let Decimal128Kind::Finite { exponent, significand } = parsed.kind { (exponent, significand) } else { panic!("expected finite, got {:?}", parsed); }; - assert_eq!(sig.load_le::(), 0); + assert_eq!(sig.value(), 0); + assert_eq!(exp.value(), 0); } } \ No newline at end of file From 00f52b6b0c5aa23e058fc25e8eb7f4d706a9ce9a Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Fri, 3 Feb 2023 15:21:56 -0500 Subject: [PATCH 05/30] another passing test --- src/decimal128.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/decimal128.rs b/src/decimal128.rs index bab89eb0..e6c68db8 100644 --- a/src/decimal128.rs +++ b/src/decimal128.rs @@ -215,7 +215,7 @@ mod tests { } #[test] - fn zero() { + fn finite_0() { let parsed = dec_from_hex("180000001364000000000000000000000000000000403000"); let (exp, sig) = if let Decimal128Kind::Finite { exponent, significand } = parsed.kind { (exponent, significand) @@ -225,4 +225,16 @@ mod tests { assert_eq!(sig.value(), 0); assert_eq!(exp.value(), 0); } + + #[test] + fn finite_0_1() { + let parsed = dec_from_hex("1800000013640001000000000000000000000000003E3000"); + let (exp, sig) = if let Decimal128Kind::Finite { exponent, significand } = parsed.kind { + (exponent, significand) + } else { + panic!("expected finite, got {:?}", parsed); + }; + assert_eq!(sig.value(), 1); + assert_eq!(exp.value(), -1); + } } \ No newline at end of file From a2139d9ec547c558ccae99dfbddc842543ed36b9 Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Fri, 3 Feb 2023 16:20:22 -0500 Subject: [PATCH 06/30] remove some dynamic alloc --- src/decimal128.rs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/decimal128.rs b/src/decimal128.rs index e6c68db8..eb1931b5 100644 --- a/src/decimal128.rs +++ b/src/decimal128.rs @@ -64,13 +64,13 @@ enum Decimal128Kind { } #[derive(Debug, Clone, PartialEq)] -struct Exponent(BitVec); +struct Exponent([u8; 2]); impl Exponent { const BIAS: i16 = 6176; fn raw(&self) -> u16 { - self.0.load_be::() + self.0.view_bits::().load_be::() } fn value(&self) -> i16 { @@ -79,11 +79,11 @@ impl Exponent { } #[derive(Debug, Clone, PartialEq)] -struct Significand(BitVec); +struct Significand([u8; 16]); impl Significand { fn value(&self) -> u128 { - self.0.load_be::() + self.0.view_bits::().load_be::() } } @@ -98,7 +98,6 @@ macro_rules! pdbg { } impl ParsedDecimal128 { - fn new(source: &Decimal128) -> Self { let tmp: Vec<_> = source.bytes.iter().copied().rev().collect(); let bits = tmp.view_bits::(); @@ -114,21 +113,22 @@ impl ParsedDecimal128 { } } else { // Finite value - let mut exponent = bitvec![u8, Order;]; - let mut significand = bitvec![u8, Order;]; + let mut exponent = [0u8; 2]; + let exponent_bits = exponent.view_bits_mut::(); + let mut significand = [0u8; 16]; + let significand_bits = &mut significand.view_bits_mut::()[14..]; if bits[1..3].all() { - exponent.extend(&bits[3..17]); - significand.extend(bits![1, 0, 0]); - significand.extend(&bits[17..]); + exponent_bits[2..].copy_from_bitslice(&bits[3..17]); + significand_bits[0..3].clone_from_bitslice(bits![1, 0, 0]); + significand_bits[3..].copy_from_bitslice(&bits[17..]); } else { - exponent.extend(&bits[1..15]); - significand.extend(bits![0]); - significand.extend(&bits[15..]); + exponent_bits[2..].copy_from_bitslice(&bits[1..15]); + significand_bits[1..].copy_from_bitslice(&bits[15..]); } - pdbg!(&exponent); - pdbg!(&significand); + pdbg!(&exponent_bits); + pdbg!(&significand_bits); Decimal128Kind::Finite { exponent: Exponent(exponent), significand: Significand(significand), From 95843b19464feb2a9003c317198f6524f6cd3ecc Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Fri, 3 Feb 2023 16:26:54 -0500 Subject: [PATCH 07/30] remove other dynamic alloc --- src/decimal128.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/decimal128.rs b/src/decimal128.rs index eb1931b5..0de6b4ce 100644 --- a/src/decimal128.rs +++ b/src/decimal128.rs @@ -99,7 +99,13 @@ macro_rules! pdbg { impl ParsedDecimal128 { fn new(source: &Decimal128) -> Self { - let tmp: Vec<_> = source.bytes.iter().copied().rev().collect(); + let tmp: [u8; 16] = { + let mut tmp = [0u8; 16]; + for i in 0..16 { + tmp[i] = source.bytes[15-i]; + } + tmp + }; let bits = tmp.view_bits::(); pdbg!(&bits); From db7628f3811cea053f9ca3efb6367a5c420b7d73 Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Tue, 7 Feb 2023 14:56:58 -0500 Subject: [PATCH 08/30] produce human-readable --- src/decimal128.rs | 81 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 67 insertions(+), 14 deletions(-) diff --git a/src/decimal128.rs b/src/decimal128.rs index 0de6b4ce..17734413 100644 --- a/src/decimal128.rs +++ b/src/decimal128.rs @@ -163,8 +163,33 @@ impl fmt::Display for ParsedDecimal128 { let adj_exp = exp_val + (coeff_str.len() as i16) - 1; if exp_val <= 0 && adj_exp >= -6 { // Plain notation + if exp_val == 0 { + write!(f, "{}", coeff_str)?; + } else { + let dec_charlen = exp_val.abs() as usize; + if dec_charlen >= coeff_str.len() { + write!(f, "0.")?; + write!(f, "{}", "0".repeat(dec_charlen - coeff_str.len()))?; + write!(f, "{}", coeff_str)?; + } else { + let (pre, post) = coeff_str.split_at(coeff_str.len() - dec_charlen); + write!(f, "{}", pre)?; + write!(f, ".")?; + write!(f, "{}", post)?; + } + } } else { // Exponential notation + let (pre, post) = coeff_str.split_at(1); + write!(f, "{}", pre)?; + if !post.is_empty() { + write!(f, ".{}", post)?; + } + write!(f, "E")?; + if adj_exp > 0 { + write!(f, "+")?; + } + write!(f, "{}", adj_exp)?; } } } @@ -191,6 +216,7 @@ mod tests { sign: false, kind: Decimal128Kind::NaN { signalling: false }, }); + assert_eq!(parsed.to_string(), "NaN"); } #[test] @@ -200,6 +226,7 @@ mod tests { sign: true, kind: Decimal128Kind::NaN { signalling: false }, }); + assert_eq!(parsed.to_string(), "-NaN"); } #[test] @@ -209,6 +236,7 @@ mod tests { sign: false, kind: Decimal128Kind::NaN { signalling: true }, }); + assert_eq!(parsed.to_string(), "sNaN"); } #[test] @@ -218,29 +246,54 @@ mod tests { sign: false, kind: Decimal128Kind::Infinity, }); + assert_eq!(parsed.to_string(), "Infinity"); + } + + fn finite_parts(parsed: ParsedDecimal128) -> (i16, u128) { + if let Decimal128Kind::Finite { exponent, significand } = parsed.kind { + (exponent.value(), significand.value()) + } else { + panic!("expected finite, got {:?}", parsed); + } } #[test] fn finite_0() { let parsed = dec_from_hex("180000001364000000000000000000000000000000403000"); - let (exp, sig) = if let Decimal128Kind::Finite { exponent, significand } = parsed.kind { - (exponent, significand) - } else { - panic!("expected finite, got {:?}", parsed); - }; - assert_eq!(sig.value(), 0); - assert_eq!(exp.value(), 0); + assert_eq!(parsed.to_string(), "0"); + assert!(!parsed.sign); + assert_eq!(finite_parts(parsed), (0, 0)); } #[test] fn finite_0_1() { let parsed = dec_from_hex("1800000013640001000000000000000000000000003E3000"); - let (exp, sig) = if let Decimal128Kind::Finite { exponent, significand } = parsed.kind { - (exponent, significand) - } else { - panic!("expected finite, got {:?}", parsed); - }; - assert_eq!(sig.value(), 1); - assert_eq!(exp.value(), -1); + assert_eq!(parsed.to_string(), "0.1"); + assert!(!parsed.sign); + assert_eq!(finite_parts(parsed), (-1, 1)); + } + + #[test] + fn finite_long_decimal() { + let parsed = dec_from_hex("18000000136400F2AF967ED05C82DE3297FF6FDE3CFC2F00"); + assert_eq!(parsed.to_string(), "0.1234567890123456789012345678901234"); + assert!(!parsed.sign); + assert_eq!(finite_parts(parsed), (-34, 1234567890123456789012345678901234)); + } + + #[test] + fn finite_smallest() { + let parsed = dec_from_hex("18000000136400D204000000000000000000000000343000"); + assert_eq!(parsed.to_string(), "0.001234"); + assert!(!parsed.sign); + assert_eq!(finite_parts(parsed), (-6, 1234)); + } + + #[test] + fn finite_fractional() { + let parsed = dec_from_hex("1800000013640064000000000000000000000000002CB000"); + assert_eq!(parsed.to_string(), "-1.00E-8"); + assert!(parsed.sign); + assert_eq!(finite_parts(parsed), (-10, 100)); } } \ No newline at end of file From 2e773287b912e2a03b9e31a370705526166c108c Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Wed, 8 Feb 2023 11:47:58 -0500 Subject: [PATCH 09/30] improve parsing readability --- src/decimal128.rs | 68 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 49 insertions(+), 19 deletions(-) diff --git a/src/decimal128.rs b/src/decimal128.rs index 17734413..1c425f1f 100644 --- a/src/decimal128.rs +++ b/src/decimal128.rs @@ -99,6 +99,7 @@ macro_rules! pdbg { impl ParsedDecimal128 { fn new(source: &Decimal128) -> Self { + // BSON byte order is the opposite of the decimal128 byte order, so flip 'em. The rest of this method could be rewritten to not need this, but readability is helped by keeping the implementation congruent with the spec. let tmp: [u8; 16] = { let mut tmp = [0u8; 16]; for i in 0..16 { @@ -106,35 +107,44 @@ impl ParsedDecimal128 { } tmp }; - let bits = tmp.view_bits::(); - pdbg!(&bits); + let src_bits = tmp.view_bits::(); + pdbg!(&src_bits); - let sign = bits[0]; - let kind = if bits[1..5].all() { + let sign = src_bits[0]; + + let kind = if src_bits[1..5].all() { // Special value - if bits[5] { - Decimal128Kind::NaN { signalling: bits[6] } + if src_bits[5] { + Decimal128Kind::NaN { signalling: src_bits[6] } } else { Decimal128Kind::Infinity } } else { // Finite value - let mut exponent = [0u8; 2]; - let exponent_bits = exponent.view_bits_mut::(); - let mut significand = [0u8; 16]; - let significand_bits = &mut significand.view_bits_mut::()[14..]; - - if bits[1..3].all() { - exponent_bits[2..].copy_from_bitslice(&bits[3..17]); - significand_bits[0..3].clone_from_bitslice(bits![1, 0, 0]); - significand_bits[3..].copy_from_bitslice(&bits[17..]); + let exponent_offset; + let significand_prefix; + if src_bits[1..3].all() { + exponent_offset = 3; + significand_prefix = bits![static 1, 0, 0]; } else { - exponent_bits[2..].copy_from_bitslice(&bits[1..15]); - significand_bits[1..].copy_from_bitslice(&bits[15..]); + exponent_offset = 1; + significand_prefix = bits![static 0]; } - pdbg!(&exponent_bits); - pdbg!(&significand_bits); + let mut exponent = [0u8; 2]; + exponent + .view_bits_mut::()[2..] + .copy_from_bitslice(&src_bits[exponent_offset..exponent_offset+14]); + + let mut significand = [0u8; 16]; + let (sig_pre, sig_post) = significand + .view_bits_mut::()[14..] + .split_at_mut(significand_prefix.len()); + sig_pre.clone_from_bitslice(significand_prefix); + sig_post.clone_from_bitslice(&src_bits[exponent_offset+14..]); + + pdbg!(exponent.view_bits::()); + pdbg!(significand.view_bits::()); Decimal128Kind::Finite { exponent: Exponent(exponent), significand: Significand(significand), @@ -142,6 +152,26 @@ impl ParsedDecimal128 { }; ParsedDecimal128 { sign, kind } } + + fn pack(&self) -> Decimal128 { + let mut bytes = [0u8; 16]; + let bits = bytes.view_bits_mut::(); + + bits.set(0, self.sign); + + match self.kind { + Decimal128Kind::NaN { signalling } => { + bits[1..6].clone_from_bitslice(bits![1, 1, 1, 1, 1]); + bits.set(6, signalling); + } + Decimal128Kind::Infinity => { + //bits[1..6] + } + _ => (), + } + + todo!() + } } impl fmt::Display for ParsedDecimal128 { From 0b938ee540b3d327e77b283195a550347fc30940 Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Wed, 8 Feb 2023 12:30:43 -0500 Subject: [PATCH 10/30] further parsing readability --- src/decimal128.rs | 84 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 58 insertions(+), 26 deletions(-) diff --git a/src/decimal128.rs b/src/decimal128.rs index 1c425f1f..55d38dd1 100644 --- a/src/decimal128.rs +++ b/src/decimal128.rs @@ -68,6 +68,19 @@ struct Exponent([u8; 2]); impl Exponent { const BIAS: i16 = 6176; + const UNUSED_BITS: usize = 2; + + fn from_bits(src_bits: &BitSlice) -> Self { + let mut bytes = [0u8; 2]; + bytes + .view_bits_mut::()[Self::UNUSED_BITS..] + .copy_from_bitslice(src_bits); + Self(bytes) + } + + fn bits(&self) -> &BitSlice { + &self.0.view_bits::()[Self::UNUSED_BITS..] + } fn raw(&self) -> u16 { self.0.view_bits::().load_be::() @@ -82,6 +95,21 @@ impl Exponent { struct Significand([u8; 16]); impl Significand { + const UNUSED_BITS: usize = 14; + + fn from_bits(src_prefix: &BitSlice, src_suffix: &BitSlice) -> Self { + let mut bytes = [0u8; 16]; + let bits = &mut bytes.view_bits_mut::()[Self::UNUSED_BITS..]; + let prefix_len = src_prefix.len(); + bits[0..prefix_len].copy_from_bitslice(src_prefix); + bits[prefix_len..].copy_from_bitslice(src_suffix); + Self(bytes) + } + + fn bits(&self) -> &BitSlice { + &self.0.view_bits::()[Self::UNUSED_BITS..] + } + fn value(&self) -> u128 { self.0.view_bits::().load_be::() } @@ -97,6 +125,8 @@ macro_rules! pdbg { } } +const EXPONENT_WIDTH: usize = 14; + impl ParsedDecimal128 { fn new(source: &Decimal128) -> Self { // BSON byte order is the opposite of the decimal128 byte order, so flip 'em. The rest of this method could be rewritten to not need this, but readability is helped by keeping the implementation congruent with the spec. @@ -125,49 +155,51 @@ impl ParsedDecimal128 { let significand_prefix; if src_bits[1..3].all() { exponent_offset = 3; - significand_prefix = bits![static 1, 0, 0]; + significand_prefix = bits![static u8, Msb0; 1, 0, 0]; } else { exponent_offset = 1; - significand_prefix = bits![static 0]; + significand_prefix = bits![static u8, Msb0; 0]; } - let mut exponent = [0u8; 2]; - exponent - .view_bits_mut::()[2..] - .copy_from_bitslice(&src_bits[exponent_offset..exponent_offset+14]); - - let mut significand = [0u8; 16]; - let (sig_pre, sig_post) = significand - .view_bits_mut::()[14..] - .split_at_mut(significand_prefix.len()); - sig_pre.clone_from_bitslice(significand_prefix); - sig_post.clone_from_bitslice(&src_bits[exponent_offset+14..]); - - pdbg!(exponent.view_bits::()); - pdbg!(significand.view_bits::()); + let exponent = Exponent::from_bits(&src_bits[exponent_offset..exponent_offset+EXPONENT_WIDTH]); + let significand = Significand::from_bits(significand_prefix, &src_bits[exponent_offset+EXPONENT_WIDTH..]); + pdbg!(exponent.bits()); + pdbg!(significand.bits()); Decimal128Kind::Finite { - exponent: Exponent(exponent), - significand: Significand(significand), + exponent, + significand, } }; ParsedDecimal128 { sign, kind } } fn pack(&self) -> Decimal128 { - let mut bytes = [0u8; 16]; - let bits = bytes.view_bits_mut::(); + let mut dest_bytes = [0u8; 16]; + let dest_bits = dest_bytes.view_bits_mut::(); - bits.set(0, self.sign); + dest_bits.set(0, self.sign); - match self.kind { + match &self.kind { Decimal128Kind::NaN { signalling } => { - bits[1..6].clone_from_bitslice(bits![1, 1, 1, 1, 1]); - bits.set(6, signalling); + dest_bits[1..6].clone_from_bitslice(bits![1, 1, 1, 1, 1]); + dest_bits.set(6, *signalling); } Decimal128Kind::Infinity => { - //bits[1..6] + dest_bits[1..6].clone_from_bitslice(bits![1, 1, 1, 1, 0]); + } + Decimal128Kind::Finite { exponent, significand } => { + let sig_bits = significand.0.view_bits::(); + let exponent_offset; + if sig_bits[0] { + dest_bits.set(1, true); + dest_bits.set(2, true); + exponent_offset = 3; + } else { + exponent_offset = 1; + }; + dest_bits[exponent_offset..exponent_offset+14] + .copy_from_bitslice(exponent.0.view_bits::()); } - _ => (), } todo!() From 010c89ef052ec847d143e8d246e9a05937686661 Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Wed, 8 Feb 2023 12:38:28 -0500 Subject: [PATCH 11/30] packing --- src/decimal128.rs | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/src/decimal128.rs b/src/decimal128.rs index 55d38dd1..23152898 100644 --- a/src/decimal128.rs +++ b/src/decimal128.rs @@ -174,8 +174,8 @@ impl ParsedDecimal128 { } fn pack(&self) -> Decimal128 { - let mut dest_bytes = [0u8; 16]; - let dest_bits = dest_bytes.view_bits_mut::(); + let mut tmp = [0u8; 16]; + let dest_bits = tmp.view_bits_mut::(); dest_bits.set(0, self.sign); @@ -188,7 +188,7 @@ impl ParsedDecimal128 { dest_bits[1..6].clone_from_bitslice(bits![1, 1, 1, 1, 0]); } Decimal128Kind::Finite { exponent, significand } => { - let sig_bits = significand.0.view_bits::(); + let sig_bits = significand.bits(); let exponent_offset; if sig_bits[0] { dest_bits.set(1, true); @@ -197,12 +197,18 @@ impl ParsedDecimal128 { } else { exponent_offset = 1; }; - dest_bits[exponent_offset..exponent_offset+14] - .copy_from_bitslice(exponent.0.view_bits::()); + dest_bits[exponent_offset..exponent_offset+EXPONENT_WIDTH] + .copy_from_bitslice(exponent.bits()); + dest_bits[exponent_offset+EXPONENT_WIDTH..] + .copy_from_bitslice(sig_bits); } } - todo!() + let mut bytes = [0u8; 16]; + for i in 0..16 { + bytes[i] = tmp[15-i]; + } + Decimal128 { bytes } } } @@ -271,14 +277,21 @@ mod tests { ParsedDecimal128::new(&d.get_decimal128("d").unwrap()) } + fn hex_from_dec(src: &ParsedDecimal128) -> String { + let bytes = crate::to_vec(&doc! { "d": src.pack() }).unwrap(); + hex::encode(bytes) + } + #[test] fn nan() { - let parsed = dec_from_hex("180000001364000000000000000000000000000000007C00"); + let hex = "180000001364000000000000000000000000000000007C00"; + let parsed = dec_from_hex(hex); assert_eq!(parsed, ParsedDecimal128 { sign: false, kind: Decimal128Kind::NaN { signalling: false }, }); assert_eq!(parsed.to_string(), "NaN"); + assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); } #[test] From 326d0aa0fb36233120caf6460ae3fb49f98f81ea Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Wed, 8 Feb 2023 12:53:28 -0500 Subject: [PATCH 12/30] roundtrip packing tests and bugfix --- src/decimal128.rs | 49 +++++++++++++++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/src/decimal128.rs b/src/decimal128.rs index 23152898..5abb34ac 100644 --- a/src/decimal128.rs +++ b/src/decimal128.rs @@ -188,13 +188,15 @@ impl ParsedDecimal128 { dest_bits[1..6].clone_from_bitslice(bits![1, 1, 1, 1, 0]); } Decimal128Kind::Finite { exponent, significand } => { - let sig_bits = significand.bits(); + let mut sig_bits = significand.bits(); let exponent_offset; if sig_bits[0] { dest_bits.set(1, true); dest_bits.set(2, true); + sig_bits = &sig_bits[3..]; exponent_offset = 3; } else { + sig_bits = &sig_bits[1..]; exponent_offset = 1; }; dest_bits[exponent_offset..exponent_offset+EXPONENT_WIDTH] @@ -296,27 +298,32 @@ mod tests { #[test] fn negative_nan() { - let parsed = dec_from_hex("18000000136400000000000000000000000000000000FC00"); + let hex = "18000000136400000000000000000000000000000000FC00"; + let parsed = dec_from_hex(hex); assert_eq!(parsed, ParsedDecimal128 { sign: true, kind: Decimal128Kind::NaN { signalling: false }, }); assert_eq!(parsed.to_string(), "-NaN"); + assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); } #[test] fn snan() { - let parsed = dec_from_hex("180000001364000000000000000000000000000000007E00"); + let hex = "180000001364000000000000000000000000000000007E00"; + let parsed = dec_from_hex(hex); assert_eq!(parsed, ParsedDecimal128 { sign: false, kind: Decimal128Kind::NaN { signalling: true }, }); assert_eq!(parsed.to_string(), "sNaN"); + assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); } #[test] fn inf() { - let parsed = dec_from_hex("180000001364000000000000000000000000000000007800"); + let hex = "180000001364000000000000000000000000000000007800"; + let parsed = dec_from_hex(hex); assert_eq!(parsed, ParsedDecimal128 { sign: false, kind: Decimal128Kind::Infinity, @@ -324,8 +331,8 @@ mod tests { assert_eq!(parsed.to_string(), "Infinity"); } - fn finite_parts(parsed: ParsedDecimal128) -> (i16, u128) { - if let Decimal128Kind::Finite { exponent, significand } = parsed.kind { + fn finite_parts(parsed: &ParsedDecimal128) -> (i16, u128) { + if let Decimal128Kind::Finite { exponent, significand } = &parsed.kind { (exponent.value(), significand.value()) } else { panic!("expected finite, got {:?}", parsed); @@ -334,41 +341,51 @@ mod tests { #[test] fn finite_0() { - let parsed = dec_from_hex("180000001364000000000000000000000000000000403000"); + let hex = "180000001364000000000000000000000000000000403000"; + let parsed = dec_from_hex(hex); assert_eq!(parsed.to_string(), "0"); assert!(!parsed.sign); - assert_eq!(finite_parts(parsed), (0, 0)); + assert_eq!(finite_parts(&parsed), (0, 0)); + assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); } #[test] fn finite_0_1() { - let parsed = dec_from_hex("1800000013640001000000000000000000000000003E3000"); + let hex = "1800000013640001000000000000000000000000003E3000"; + let parsed = dec_from_hex(hex); assert_eq!(parsed.to_string(), "0.1"); assert!(!parsed.sign); - assert_eq!(finite_parts(parsed), (-1, 1)); + assert_eq!(finite_parts(&parsed), (-1, 1)); + assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); } #[test] fn finite_long_decimal() { - let parsed = dec_from_hex("18000000136400F2AF967ED05C82DE3297FF6FDE3CFC2F00"); + let hex = "18000000136400F2AF967ED05C82DE3297FF6FDE3CFC2F00"; + let parsed = dec_from_hex(hex); assert_eq!(parsed.to_string(), "0.1234567890123456789012345678901234"); assert!(!parsed.sign); - assert_eq!(finite_parts(parsed), (-34, 1234567890123456789012345678901234)); + assert_eq!(finite_parts(&parsed), (-34, 1234567890123456789012345678901234)); + assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); } #[test] fn finite_smallest() { - let parsed = dec_from_hex("18000000136400D204000000000000000000000000343000"); + let hex = "18000000136400D204000000000000000000000000343000"; + let parsed = dec_from_hex(hex); assert_eq!(parsed.to_string(), "0.001234"); assert!(!parsed.sign); - assert_eq!(finite_parts(parsed), (-6, 1234)); + assert_eq!(finite_parts(&parsed), (-6, 1234)); + assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); } #[test] fn finite_fractional() { - let parsed = dec_from_hex("1800000013640064000000000000000000000000002CB000"); + let hex = "1800000013640064000000000000000000000000002CB000"; + let parsed = dec_from_hex(hex); assert_eq!(parsed.to_string(), "-1.00E-8"); assert!(parsed.sign); - assert_eq!(finite_parts(parsed), (-10, 100)); + assert_eq!(finite_parts(&parsed), (-10, 100)); + assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); } } \ No newline at end of file From b9819ccacb4ca1e5a79d42e3639a491e6ad49633 Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Wed, 8 Feb 2023 15:20:27 -0500 Subject: [PATCH 13/30] untested string parsing --- src/decimal128.rs | 199 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 175 insertions(+), 24 deletions(-) diff --git a/src/decimal128.rs b/src/decimal128.rs index 5abb34ac..6c436c71 100644 --- a/src/decimal128.rs +++ b/src/decimal128.rs @@ -41,7 +41,15 @@ impl fmt::Debug for Decimal128 { impl fmt::Display for Decimal128 { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{:?}", self) + write!(f, "{}", ParsedDecimal128::new(self)) + } +} + +impl std::str::FromStr for Decimal128 { + type Err = ParseError; + + fn from_str(s: &str) -> Result { + Ok(s.parse::()?.pack()) } } @@ -59,7 +67,7 @@ enum Decimal128Kind { Infinity, Finite { exponent: Exponent, - significand: Significand, + coefficient: Coefficient, } } @@ -69,6 +77,9 @@ struct Exponent([u8; 2]); impl Exponent { const BIAS: i16 = 6176; const UNUSED_BITS: usize = 2; + const WIDTH: usize = 14; + const TINY: i16 = -6176; + const MAX: i16 = 6144; fn from_bits(src_bits: &BitSlice) -> Self { let mut bytes = [0u8; 2]; @@ -78,6 +89,14 @@ impl Exponent { Self(bytes) } + fn from_native(value: i16) -> Self { + let mut bytes = [0u8; 2]; + bytes + .view_bits_mut::() + .store_be(value + Self::BIAS); + Self(bytes) + } + fn bits(&self) -> &BitSlice { &self.0.view_bits::()[Self::UNUSED_BITS..] } @@ -92,10 +111,11 @@ impl Exponent { } #[derive(Debug, Clone, PartialEq)] -struct Significand([u8; 16]); +struct Coefficient([u8; 16]); -impl Significand { +impl Coefficient { const UNUSED_BITS: usize = 14; + const MAX_DIGITS: usize = 34; fn from_bits(src_prefix: &BitSlice, src_suffix: &BitSlice) -> Self { let mut bytes = [0u8; 16]; @@ -106,6 +126,14 @@ impl Significand { Self(bytes) } + fn from_native(value: u128) -> Self { + let mut bytes = [0u8; 16]; + bytes + .view_bits_mut::() + .store_be(value); + Self(bytes) + } + fn bits(&self) -> &BitSlice { &self.0.view_bits::()[Self::UNUSED_BITS..] } @@ -125,7 +153,6 @@ macro_rules! pdbg { } } -const EXPONENT_WIDTH: usize = 14; impl ParsedDecimal128 { fn new(source: &Decimal128) -> Self { @@ -152,22 +179,22 @@ impl ParsedDecimal128 { } else { // Finite value let exponent_offset; - let significand_prefix; + let coeff_prefix; if src_bits[1..3].all() { exponent_offset = 3; - significand_prefix = bits![static u8, Msb0; 1, 0, 0]; + coeff_prefix = bits![static u8, Msb0; 1, 0, 0]; } else { exponent_offset = 1; - significand_prefix = bits![static u8, Msb0; 0]; + coeff_prefix = bits![static u8, Msb0; 0]; } - let exponent = Exponent::from_bits(&src_bits[exponent_offset..exponent_offset+EXPONENT_WIDTH]); - let significand = Significand::from_bits(significand_prefix, &src_bits[exponent_offset+EXPONENT_WIDTH..]); + let exponent = Exponent::from_bits(&src_bits[exponent_offset..exponent_offset+Exponent::WIDTH]); + let coefficient = Coefficient::from_bits(coeff_prefix, &src_bits[exponent_offset+Exponent::WIDTH..]); pdbg!(exponent.bits()); - pdbg!(significand.bits()); + pdbg!(coefficient.bits()); Decimal128Kind::Finite { exponent, - significand, + coefficient, } }; ParsedDecimal128 { sign, kind } @@ -187,22 +214,22 @@ impl ParsedDecimal128 { Decimal128Kind::Infinity => { dest_bits[1..6].clone_from_bitslice(bits![1, 1, 1, 1, 0]); } - Decimal128Kind::Finite { exponent, significand } => { - let mut sig_bits = significand.bits(); + Decimal128Kind::Finite { exponent, coefficient } => { + let mut coeff_bits = coefficient.bits(); let exponent_offset; - if sig_bits[0] { + if coeff_bits[0] { dest_bits.set(1, true); dest_bits.set(2, true); - sig_bits = &sig_bits[3..]; + coeff_bits = &coeff_bits[3..]; exponent_offset = 3; } else { - sig_bits = &sig_bits[1..]; + coeff_bits = &coeff_bits[1..]; exponent_offset = 1; }; - dest_bits[exponent_offset..exponent_offset+EXPONENT_WIDTH] + dest_bits[exponent_offset..exponent_offset+Exponent::WIDTH] .copy_from_bitslice(exponent.bits()); - dest_bits[exponent_offset+EXPONENT_WIDTH..] - .copy_from_bitslice(sig_bits); + dest_bits[exponent_offset+Exponent::WIDTH..] + .copy_from_bitslice(coeff_bits); } } @@ -227,8 +254,8 @@ impl fmt::Display for ParsedDecimal128 { write!(f, "NaN")?; } Decimal128Kind::Infinity => write!(f, "Infinity")?, - Decimal128Kind::Finite { exponent, significand } => { - let coeff_str = format!("{}", significand.value()); + Decimal128Kind::Finite { exponent, coefficient } => { + let coeff_str = format!("{}", coefficient.value()); let exp_val = exponent.value(); let adj_exp = exp_val + (coeff_str.len() as i16) - 1; if exp_val <= 0 && adj_exp >= -6 { @@ -267,6 +294,130 @@ impl fmt::Display for ParsedDecimal128 { } } +#[derive(Debug)] +#[non_exhaustive] +pub enum ParseError { + EmptyExponent, + InvalidExponent(std::num::ParseIntError), + InvalidCoefficient(std::num::ParseIntError), + Overflow, + Underflow, + InexactRounding, +} + +impl fmt::Display for ParseError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + ParseError::EmptyExponent => write!(f, "empty exponent"), + ParseError::InvalidExponent(e) => write!(f, "invalid exponent: {}", e), + ParseError::InvalidCoefficient(e) => write!(f, "invalid coefficient: {}", e), + ParseError::Overflow => write!(f, "overflow"), + ParseError::Underflow => write!(f, "underflow"), + ParseError::InexactRounding => write!(f, "inexact rounding"), + } + } +} + +impl std::error::Error for ParseError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + ParseError::InvalidExponent(e) => Some(e), + ParseError::InvalidCoefficient(e) => Some(e), + _ => None, + } + } +} + +impl std::str::FromStr for ParsedDecimal128 { + type Err = ParseError; + + fn from_str(mut s: &str) -> Result { + let sign = if let Some(rest) = s.strip_prefix('-') { + s = rest; + true + } else { + false + }; + let kind = match s.to_ascii_lowercase().as_str() { + "nan" => Decimal128Kind::NaN { signalling: false }, + "snan" => Decimal128Kind::NaN { signalling: true }, + "infinity" | "inf" => Decimal128Kind::Infinity, + finite_str => { + // Split into parts + let mut decimal_str; + let exp_str; + match finite_str.split_once('E') { + None => { + decimal_str = finite_str; + exp_str = "0"; + } + Some((_, "")) => return Err(ParseError::EmptyExponent), + Some((pre, post)) => { + decimal_str = pre; + exp_str = post; + } + } + + let mut exp = exp_str.parse::().map_err(|e| ParseError::InvalidExponent(e))?; + + // Strip leading zeros + let rest = decimal_str.trim_start_matches('0'); + decimal_str = if rest.is_empty() { + "0" + } else { + rest + }; + + // Remove decimal point and adjust exponent + let tmp_str; + if let Some((pre, post)) = decimal_str.split_once('.') { + let exp_adj = post.len().try_into().map_err(|_| ParseError::Underflow)?; + exp = exp.checked_sub(exp_adj).ok_or(ParseError::Underflow)?; + tmp_str = format!("{}{}", pre, post); + decimal_str = &tmp_str; + } + + // Check decimal precision + { + let len = decimal_str.len(); + if len > Coefficient::MAX_DIGITS { + let decimal_str = round_decimal_str(decimal_str, Coefficient::MAX_DIGITS)?; + let exp_adj = (len - decimal_str.len()).try_into().map_err(|_| ParseError::Overflow)?; + exp = exp.checked_add(exp_adj).ok_or(ParseError::Overflow)?; + } + } + + // Check exponent limits + if exp < Exponent::TINY { + let delta = (Exponent::TINY - exp).try_into().map_err(|_| ParseError::Overflow)?; + let new_precision = decimal_str.len().checked_sub(delta).ok_or(ParseError::Underflow)?; + decimal_str = round_decimal_str(decimal_str, new_precision)?; + } + if exp > Exponent::MAX { + return Err(ParseError::Overflow); + } + + // Assemble the final value + let exponent = Exponent::from_native(exp); + let coeff: u128 = decimal_str.parse().map_err(|e| ParseError::InvalidCoefficient(e))?; + let coefficient = Coefficient::from_native(coeff); + Decimal128Kind::Finite { exponent, coefficient } + }, + }; + + Ok(Self { sign, kind }) + } +} + +fn round_decimal_str(s: &str, precision: usize) -> Result<&str, ParseError> { + let (pre, post) = s.split_at(precision); + // Any nonzero trimmed digits mean it would be an imprecise round. + if post.chars().any(|c| c != '0') { + return Err(ParseError::InexactRounding); + } + Ok(pre) +} + #[cfg(test)] mod tests { use crate::Document; @@ -332,8 +483,8 @@ mod tests { } fn finite_parts(parsed: &ParsedDecimal128) -> (i16, u128) { - if let Decimal128Kind::Finite { exponent, significand } = &parsed.kind { - (exponent.value(), significand.value()) + if let Decimal128Kind::Finite { exponent, coefficient } = &parsed.kind { + (exponent.value(), coefficient.value()) } else { panic!("expected finite, got {:?}", parsed); } From 6f85924046ad5a46e343114ca7117548797d1604 Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Wed, 8 Feb 2023 15:28:30 -0500 Subject: [PATCH 14/30] some passing tests and a failing one --- src/decimal128.rs | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/src/decimal128.rs b/src/decimal128.rs index 6c436c71..b7872043 100644 --- a/src/decimal128.rs +++ b/src/decimal128.rs @@ -500,6 +500,14 @@ mod tests { assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); } + #[test] + fn finite_0_parse() { + let hex = "180000001364000000000000000000000000000000403000"; + let parsed: ParsedDecimal128 = "0".parse().unwrap(); + assert_eq!(finite_parts(&parsed), (0, 0)); + assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); + } + #[test] fn finite_0_1() { let hex = "1800000013640001000000000000000000000000003E3000"; @@ -510,6 +518,14 @@ mod tests { assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); } + #[test] + fn finite_0_1_parse() { + let hex = "1800000013640001000000000000000000000000003E3000"; + let parsed: ParsedDecimal128 = "0.1".parse().unwrap(); + assert_eq!(finite_parts(&parsed), (-1, 1)); + assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); + } + #[test] fn finite_long_decimal() { let hex = "18000000136400F2AF967ED05C82DE3297FF6FDE3CFC2F00"; @@ -539,4 +555,33 @@ mod tests { assert_eq!(finite_parts(&parsed), (-10, 100)); assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); } + + #[test] + fn finite_largest() { + let hex = "18000000136400F2AF967ED05C82DE3297FF6FDE3C403000"; + let parsed = dec_from_hex(hex); + assert_eq!(parsed.to_string(), "1234567890123456789012345678901234"); + assert!(!parsed.sign); + assert_eq!(finite_parts(&parsed), (0, 1234567890123456789012345678901234)); + assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); + } + + #[test] + fn finite_scientific_largest() { + let hex = "18000000136400FFFFFFFF638E8D37C087ADBE09EDFF5F00"; + let parsed = dec_from_hex(hex); + assert_eq!(parsed.to_string(), "9.999999999999999999999999999999999E+6144"); + assert!(!parsed.sign); + assert_eq!(finite_parts(&parsed), (6111, 9999999999999999999999999999999999)); + assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); + } + + #[test] + fn finite_scientific_largest_parse() { + let hex = "18000000136400FFFFFFFF638E8D37C087ADBE09EDFF5F00"; + let parsed: ParsedDecimal128 = "9.999999999999999999999999999999999E+6144".parse().unwrap(); + assert!(!parsed.sign); + assert_eq!(finite_parts(&parsed), (6111, 9999999999999999999999999999999999)); + assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); + } } \ No newline at end of file From 4033eb0db43fb352392289c6e8311a2a1128e420 Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Wed, 8 Feb 2023 15:31:30 -0500 Subject: [PATCH 15/30] now passing --- src/decimal128.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/decimal128.rs b/src/decimal128.rs index b7872043..ba128ab6 100644 --- a/src/decimal128.rs +++ b/src/decimal128.rs @@ -346,7 +346,7 @@ impl std::str::FromStr for ParsedDecimal128 { // Split into parts let mut decimal_str; let exp_str; - match finite_str.split_once('E') { + match finite_str.split_once('e') { None => { decimal_str = finite_str; exp_str = "0"; @@ -379,8 +379,8 @@ impl std::str::FromStr for ParsedDecimal128 { // Check decimal precision { - let len = decimal_str.len(); - if len > Coefficient::MAX_DIGITS { + let len = dbg!(dbg!(&decimal_str).len()); + if dbg!(len > Coefficient::MAX_DIGITS) { let decimal_str = round_decimal_str(decimal_str, Coefficient::MAX_DIGITS)?; let exp_adj = (len - decimal_str.len()).try_into().map_err(|_| ParseError::Overflow)?; exp = exp.checked_add(exp_adj).ok_or(ParseError::Overflow)?; @@ -388,7 +388,7 @@ impl std::str::FromStr for ParsedDecimal128 { } // Check exponent limits - if exp < Exponent::TINY { + if dbg!(exp < Exponent::TINY) { let delta = (Exponent::TINY - exp).try_into().map_err(|_| ParseError::Overflow)?; let new_precision = decimal_str.len().checked_sub(delta).ok_or(ParseError::Underflow)?; decimal_str = round_decimal_str(decimal_str, new_precision)?; From 78e1bd6cae86eda299477194036315b607fb822d Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Thu, 9 Feb 2023 11:15:00 -0500 Subject: [PATCH 16/30] zero invalid coefficients; add more tests --- src/decimal128.rs | 62 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/src/decimal128.rs b/src/decimal128.rs index ba128ab6..43375e8b 100644 --- a/src/decimal128.rs +++ b/src/decimal128.rs @@ -116,6 +116,7 @@ struct Coefficient([u8; 16]); impl Coefficient { const UNUSED_BITS: usize = 14; const MAX_DIGITS: usize = 34; + const MAX_VALUE: u128 = 9_999_999_999_999_999_999_999_999_999_999_999; fn from_bits(src_prefix: &BitSlice, src_suffix: &BitSlice) -> Self { let mut bytes = [0u8; 16]; @@ -123,7 +124,13 @@ impl Coefficient { let prefix_len = src_prefix.len(); bits[0..prefix_len].copy_from_bitslice(src_prefix); bits[prefix_len..].copy_from_bitslice(src_suffix); - Self(bytes) + let out = Self(bytes); + if out.value() > Self::MAX_VALUE { + // Invalid coefficients get silently replaced with zero. + Self([0u8; 16]) + } else { + out + } } fn from_native(value: u128) -> Self { @@ -490,6 +497,43 @@ mod tests { } } + #[test] + fn invalid_0() { + let hex = "180000001364000000000000000000000000000000106C00"; + let parsed = dec_from_hex(hex); + assert_eq!(parsed.to_string(), "0"); + assert!(!parsed.sign); + assert_eq!(finite_parts(&parsed), (0, 0)); + } + + #[test] + fn invalid_neg_0() { + let hex = "18000000136400DCBA9876543210DEADBEEF00000010EC00"; + let parsed = dec_from_hex(hex); + assert_eq!(parsed.to_string(), "-0"); + assert!(parsed.sign); + assert_eq!(finite_parts(&parsed), (0, 0)); + } + + #[test] + fn invalid_0_e3() { + let hex = "18000000136400FFFFFFFFFFFFFFFFFFFFFFFFFFFF116C00"; + let parsed = dec_from_hex(hex); + assert_eq!(parsed.to_string(), "0E+3"); + assert!(!parsed.sign); + assert_eq!(finite_parts(&parsed), (3, 0)); + } + + #[test] + fn finite_adjusted_exponent_limit() { + let hex = "18000000136400F2AF967ED05C82DE3297FF6FDE3CF22F00"; + let parsed = dec_from_hex(hex); + assert_eq!(parsed.to_string(), "0.000001234567890123456789012345678901234"); + assert!(!parsed.sign); + assert_eq!(finite_parts(&parsed), (-39, 1234567890123456789012345678901234)); + assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); + } + #[test] fn finite_0() { let hex = "180000001364000000000000000000000000000000403000"; @@ -584,4 +628,20 @@ mod tests { assert_eq!(finite_parts(&parsed), (6111, 9999999999999999999999999999999999)); assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); } + + #[test] + fn noncanonical_exponent_normalization() { + let hex = "1800000013640064000000000000000000000000002CB000"; + let parsed: ParsedDecimal128 = "-100E-10".parse().unwrap(); + assert_eq!(parsed.to_string(), "-1.00E-8"); + assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); + } + + #[test] + fn rounded_subnormal() { + let hex = "180000001364000100000000000000000000000000000000"; + let parsed: ParsedDecimal128 = "10E-6177".parse().unwrap(); + assert_eq!(parsed.to_string(), "1E-6176"); + assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); + } } \ No newline at end of file From 2d6c658aee52eee551c20c2083b687e40e369182 Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Thu, 9 Feb 2023 11:32:46 -0500 Subject: [PATCH 17/30] fix a few more bugs --- src/decimal128.rs | 39 +++++++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/src/decimal128.rs b/src/decimal128.rs index 43375e8b..1beb5475 100644 --- a/src/decimal128.rs +++ b/src/decimal128.rs @@ -79,7 +79,7 @@ impl Exponent { const UNUSED_BITS: usize = 2; const WIDTH: usize = 14; const TINY: i16 = -6176; - const MAX: i16 = 6144; + const MAX: i16 = 6111; fn from_bits(src_bits: &BitSlice) -> Self { let mut bytes = [0u8; 2]; @@ -364,7 +364,6 @@ impl std::str::FromStr for ParsedDecimal128 { exp_str = post; } } - let mut exp = exp_str.parse::().map_err(|e| ParseError::InvalidExponent(e))?; // Strip leading zeros @@ -376,19 +375,19 @@ impl std::str::FromStr for ParsedDecimal128 { }; // Remove decimal point and adjust exponent - let tmp_str; + let joined_str; if let Some((pre, post)) = decimal_str.split_once('.') { let exp_adj = post.len().try_into().map_err(|_| ParseError::Underflow)?; exp = exp.checked_sub(exp_adj).ok_or(ParseError::Underflow)?; - tmp_str = format!("{}{}", pre, post); - decimal_str = &tmp_str; + joined_str = format!("{}{}", pre, post); + decimal_str = &joined_str; } // Check decimal precision { - let len = dbg!(dbg!(&decimal_str).len()); + let len = decimal_str.len(); if dbg!(len > Coefficient::MAX_DIGITS) { - let decimal_str = round_decimal_str(decimal_str, Coefficient::MAX_DIGITS)?; + decimal_str = round_decimal_str(decimal_str, Coefficient::MAX_DIGITS)?; let exp_adj = (len - decimal_str.len()).try_into().map_err(|_| ParseError::Overflow)?; exp = exp.checked_add(exp_adj).ok_or(ParseError::Overflow)?; } @@ -399,9 +398,17 @@ impl std::str::FromStr for ParsedDecimal128 { let delta = (Exponent::TINY - exp).try_into().map_err(|_| ParseError::Overflow)?; let new_precision = decimal_str.len().checked_sub(delta).ok_or(ParseError::Underflow)?; decimal_str = round_decimal_str(decimal_str, new_precision)?; + exp = Exponent::TINY; } + let padded_str; if exp > Exponent::MAX { - return Err(ParseError::Overflow); + let delta = (exp - Exponent::MAX).try_into().map_err(|_| ParseError::Overflow)?; + if decimal_str.len().checked_add(delta).ok_or(ParseError::Overflow)? > Coefficient::MAX_DIGITS { + return Err(ParseError::Overflow); + } + padded_str = format!("{}{}", decimal_str, "0".repeat(delta)); + decimal_str = &padded_str; + exp = Exponent::MAX; } // Assemble the final value @@ -644,4 +651,20 @@ mod tests { assert_eq!(parsed.to_string(), "1E-6176"); assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); } + + #[test] + fn clamped() { + let hex = "180000001364000a00000000000000000000000000fe5f00"; + let parsed: ParsedDecimal128 = "1E6112".parse().unwrap(); + assert_eq!(parsed.to_string(), "1.0E+6112"); + assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); + } + + #[test] + fn exact_rounding() { + let hex = "18000000136400000000000a5bc138938d44c64d31cc3700"; + let parsed: ParsedDecimal128 = "1000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000".parse().unwrap(); + assert_eq!(parsed.to_string(), "1.000000000000000000000000000000000E+999"); + assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); + } } \ No newline at end of file From 0d73014282cf9c56a9a1b407ca48048aeb4f6fa6 Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Thu, 9 Feb 2023 13:29:04 -0500 Subject: [PATCH 18/30] hypothetical decimal128 serde support --- src/de/serde.rs | 11 +++++++---- src/ser/serde.rs | 12 +++++++++--- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/de/serde.rs b/src/de/serde.rs index a219ff82..a8f88122 100644 --- a/src/de/serde.rs +++ b/src/de/serde.rs @@ -466,10 +466,13 @@ impl<'de> Visitor<'de> for BsonVisitor { } "$numberDecimal" => { - return Err(Error::custom( - "deserializing decimal128 values from strings is not currently supported" - .to_string(), - )); + let string: String = visitor.next_value()?; + return Ok(Bson::Decimal128(string.parse::().map_err(|_| { + V::Error::invalid_value( + Unexpected::Str(&string), + &"decimal128 as a string", + ) + })?)); } "$numberDecimalBytes" => { diff --git a/src/ser/serde.rs b/src/ser/serde.rs index d4556bb9..e2866a54 100644 --- a/src/ser/serde.rs +++ b/src/ser/serde.rs @@ -695,9 +695,15 @@ impl Serialize for Decimal128 { where S: ser::Serializer, { - let mut state = serializer.serialize_struct("$numberDecimal", 1)?; - state.serialize_field("$numberDecimalBytes", serde_bytes::Bytes::new(&self.bytes))?; - state.end() + if serializer.is_human_readable() { + let mut state = serializer.serialize_map(Some(1))?; + state.serialize_entry("$numberDecimal", &self.to_string())?; + state.end() + } else { + let mut state = serializer.serialize_struct("$numberDecimal", 1)?; + state.serialize_field("$numberDecimalBytes", serde_bytes::Bytes::new(&self.bytes))?; + state.end() + } } } From 7fefa3bd6b47785ba72a0fc577f398143543bb9b Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Thu, 9 Feb 2023 13:39:40 -0500 Subject: [PATCH 19/30] extjson support --- src/bson.rs | 2 +- src/extjson/de.rs | 9 ++--- src/extjson/models.rs | 18 ++++++++++ src/tests/spec/corpus.rs | 74 +++++++++++++--------------------------- 4 files changed, 47 insertions(+), 56 deletions(-) diff --git a/src/bson.rs b/src/bson.rs index 3780b12e..89e78967 100644 --- a/src/bson.rs +++ b/src/bson.rs @@ -455,7 +455,7 @@ impl Bson { "$date": { "$numberLong": v.timestamp_millis().to_string() }, }), Bson::Symbol(v) => json!({ "$symbol": v }), - Bson::Decimal128(_) => panic!("Decimal128 extended JSON not implemented yet."), + Bson::Decimal128(v) => json!({ "$numberDecimal": v.to_string() }), Bson::Undefined => json!({ "$undefined": true }), Bson::MinKey => json!({ "$minKey": 1 }), Bson::MaxKey => json!({ "$maxKey": 1 }), diff --git a/src/extjson/de.rs b/src/extjson/de.rs index 0b97dd42..e4683391 100644 --- a/src/extjson/de.rs +++ b/src/extjson/de.rs @@ -112,6 +112,11 @@ impl TryFrom> for Bson { return Ok(Bson::Double(double.parse()?)); } + if obj.contains_key("$numberDecimal") { + let decimal: models::Decimal128 = serde_json::from_value(obj.into())?; + return Ok(Bson::Decimal128(decimal.parse()?)); + } + if obj.contains_key("$binary") { let binary: models::Binary = serde_json::from_value(obj.into())?; return Ok(Bson::Binary(binary.parse()?)); @@ -159,10 +164,6 @@ impl TryFrom> for Bson { return Ok(db_ptr.parse()?.into()); } - if obj.contains_key("$numberDecimal") { - return Err(Error::custom("decimal128 extjson support not implemented")); - } - if obj.contains_key("$undefined") { let undefined: models::Undefined = serde_json::from_value(obj.into())?; return undefined.parse(); diff --git a/src/extjson/models.rs b/src/extjson/models.rs index 667123c3..efda5600 100644 --- a/src/extjson/models.rs +++ b/src/extjson/models.rs @@ -72,6 +72,24 @@ impl Double { } } +#[derive(Deserialize)] +#[serde(deny_unknown_fields)] +pub(crate) struct Decimal128 { + #[serde(rename = "$numberDecimal")] + value: String, +} + +impl Decimal128 { + pub(crate) fn parse(self) -> extjson::de::Result { + self.value.parse().map_err(|_| { + extjson::de::Error::invalid_value( + Unexpected::Str(&self.value), + &"expected bson decimal128 as string", + ) + }) + } +} + #[derive(Serialize, Deserialize)] #[serde(deny_unknown_fields)] pub(crate) struct ObjectId { diff --git a/src/tests/spec/corpus.rs b/src/tests/spec/corpus.rs index 6ded2dd5..03406478 100644 --- a/src/tests/spec/corpus.rs +++ b/src/tests/spec/corpus.rs @@ -387,12 +387,6 @@ fn run_test(test: TestFile) { } } - // TODO RUST-36: Enable decimal128 tests. - // extJSON not implemented for decimal128, so we must stop here. - if test.bson_type == "0x13" { - continue; - } - let cej: serde_json::Value = serde_json::from_str(&valid.canonical_extjson).expect(&description); @@ -422,15 +416,12 @@ fn run_test(test: TestFile) { } } - // TODO RUST-36: Enable decimal128 tests. - if test.bson_type != "0x13" { - assert_eq!( - Bson::Document(documentfromreader_cb.clone()).into_canonical_extjson(), - cej_updated_float, - "{}", - description - ); - } + assert_eq!( + Bson::Document(documentfromreader_cb.clone()).into_canonical_extjson(), + cej_updated_float, + "{}", + description + ); // native_to_relaxed_extended_json( bson_to_native(cB) ) = cEJ @@ -468,15 +459,12 @@ fn run_test(test: TestFile) { .to_writer(&mut native_to_bson_json_to_native_cej) .unwrap(); - // TODO RUST-36: Enable decimal128 tests. - if test.bson_type != "0x13" { - assert_eq!( - hex::encode(native_to_bson_json_to_native_cej).to_lowercase(), - valid.canonical_bson.to_lowercase(), - "{}", - description, - ); - } + assert_eq!( + hex::encode(native_to_bson_json_to_native_cej).to_lowercase(), + valid.canonical_bson.to_lowercase(), + "{}", + description, + ); } if let Some(ref degenerate_extjson) = valid.degenerate_extjson { @@ -490,14 +478,11 @@ fn run_test(test: TestFile) { let native_to_canonical_extended_json_json_to_native_dej = json_to_native_dej.clone().into_canonical_extjson(); - // TODO RUST-36: Enable decimal128 tests. - if test.bson_type != "0x13" { - assert_eq!( - native_to_canonical_extended_json_json_to_native_dej, cej, - "{}", - description, - ); - } + assert_eq!( + native_to_canonical_extended_json_json_to_native_dej, cej, + "{}", + description, + ); // native_to_bson( json_to_native(dEJ) ) = cB (unless lossy) @@ -509,15 +494,12 @@ fn run_test(test: TestFile) { .to_writer(&mut native_to_bson_json_to_native_dej) .unwrap(); - // TODO RUST-36: Enable decimal128 tests. - if test.bson_type != "0x13" { - assert_eq!( - hex::encode(native_to_bson_json_to_native_dej).to_lowercase(), - valid.canonical_bson.to_lowercase(), - "{}", - description, - ); - } + assert_eq!( + hex::encode(native_to_bson_json_to_native_dej).to_lowercase(), + valid.canonical_bson.to_lowercase(), + "{}", + description, + ); } } @@ -571,21 +553,11 @@ fn run_test(test: TestFile) { } for parse_error in test.parse_errors { - // TODO RUST-36: Enable decimal128 tests. - if test.bson_type == "0x13" { - continue; - } - // no special support for dbref convention if parse_error.description.contains("DBRef") { continue; } - // TODO RUST-36: Enable decimal128 tests. - if parse_error.description.contains("$numberDecimal") { - continue; - } - let json: serde_json::Value = serde_json::from_str(parse_error.string.as_str()).expect(&parse_error.description); From b3da1e3b91108efa90ec137e56607ea54346a3c9 Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Thu, 9 Feb 2023 14:25:34 -0500 Subject: [PATCH 20/30] running the corpus test, and more fixes --- src/bson.rs | 3 --- src/decimal128.rs | 51 ++++++++++++++++------------------------ src/extjson/models.rs | 11 +++++---- src/tests/spec/corpus.rs | 4 ++-- 4 files changed, 28 insertions(+), 41 deletions(-) diff --git a/src/bson.rs b/src/bson.rs index 89e78967..9d64ed7c 100644 --- a/src/bson.rs +++ b/src/bson.rs @@ -474,9 +474,6 @@ impl Bson { } /// Converts the Bson value into its [canonical extended JSON representation](https://www.mongodb.com/docs/manual/reference/mongodb-extended-json/). - /// - /// Note: extended json encoding for `Decimal128` values is not supported. If this method is - /// called on a case which contains a `Decimal128` value, it will panic. pub fn into_canonical_extjson(self) -> Value { match self { Bson::Int32(i) => json!({ "$numberInt": i.to_string() }), diff --git a/src/decimal128.rs b/src/decimal128.rs index 1beb5475..32aa036d 100644 --- a/src/decimal128.rs +++ b/src/decimal128.rs @@ -150,17 +150,6 @@ impl Coefficient { } } -macro_rules! pdbg { - ($expr: expr) => { - { - let val = $expr; - println!("{} = {}", stringify!($expr), val); - val - } - } -} - - impl ParsedDecimal128 { fn new(source: &Decimal128) -> Self { // BSON byte order is the opposite of the decimal128 byte order, so flip 'em. The rest of this method could be rewritten to not need this, but readability is helped by keeping the implementation congruent with the spec. @@ -172,10 +161,8 @@ impl ParsedDecimal128 { tmp }; let src_bits = tmp.view_bits::(); - pdbg!(&src_bits); let sign = src_bits[0]; - let kind = if src_bits[1..5].all() { // Special value if src_bits[5] { @@ -197,8 +184,6 @@ impl ParsedDecimal128 { let exponent = Exponent::from_bits(&src_bits[exponent_offset..exponent_offset+Exponent::WIDTH]); let coefficient = Coefficient::from_bits(coeff_prefix, &src_bits[exponent_offset+Exponent::WIDTH..]); - pdbg!(exponent.bits()); - pdbg!(coefficient.bits()); Decimal128Kind::Finite { exponent, coefficient, @@ -250,14 +235,17 @@ impl ParsedDecimal128 { impl fmt::Display for ParsedDecimal128 { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - if self.sign { + // MongoDB diverges from the IEEE spec and requires no sign for NaN + if self.sign && !matches!(&self.kind, Decimal128Kind::NaN { .. }) { write!(f, "-")?; } match &self.kind { - Decimal128Kind::NaN { signalling } => { + Decimal128Kind::NaN { signalling: _signalling } => { + /* Likewise, MongoDB requires no 's' prefix for signalling. if *signalling { write!(f, "s")?; } + */ write!(f, "NaN")?; } Decimal128Kind::Infinity => write!(f, "Infinity")?, @@ -339,12 +327,13 @@ impl std::str::FromStr for ParsedDecimal128 { type Err = ParseError; fn from_str(mut s: &str) -> Result { - let sign = if let Some(rest) = s.strip_prefix('-') { + let sign; + if let Some(rest) = s.strip_prefix(&['-', '+']) { + sign = s.chars().next() == Some('-'); s = rest; - true } else { - false - }; + sign = false; + } let kind = match s.to_ascii_lowercase().as_str() { "nan" => Decimal128Kind::NaN { signalling: false }, "snan" => Decimal128Kind::NaN { signalling: true }, @@ -366,14 +355,6 @@ impl std::str::FromStr for ParsedDecimal128 { } let mut exp = exp_str.parse::().map_err(|e| ParseError::InvalidExponent(e))?; - // Strip leading zeros - let rest = decimal_str.trim_start_matches('0'); - decimal_str = if rest.is_empty() { - "0" - } else { - rest - }; - // Remove decimal point and adjust exponent let joined_str; if let Some((pre, post)) = decimal_str.split_once('.') { @@ -383,10 +364,18 @@ impl std::str::FromStr for ParsedDecimal128 { decimal_str = &joined_str; } + // Strip leading zeros + let rest = decimal_str.trim_start_matches('0'); + decimal_str = if rest.is_empty() { + "0" + } else { + rest + }; + // Check decimal precision { let len = decimal_str.len(); - if dbg!(len > Coefficient::MAX_DIGITS) { + if len > Coefficient::MAX_DIGITS { decimal_str = round_decimal_str(decimal_str, Coefficient::MAX_DIGITS)?; let exp_adj = (len - decimal_str.len()).try_into().map_err(|_| ParseError::Overflow)?; exp = exp.checked_add(exp_adj).ok_or(ParseError::Overflow)?; @@ -394,7 +383,7 @@ impl std::str::FromStr for ParsedDecimal128 { } // Check exponent limits - if dbg!(exp < Exponent::TINY) { + if exp < Exponent::TINY { let delta = (Exponent::TINY - exp).try_into().map_err(|_| ParseError::Overflow)?; let new_precision = decimal_str.len().checked_sub(delta).ok_or(ParseError::Underflow)?; decimal_str = round_decimal_str(decimal_str, new_precision)?; diff --git a/src/extjson/models.rs b/src/extjson/models.rs index efda5600..3de79b7d 100644 --- a/src/extjson/models.rs +++ b/src/extjson/models.rs @@ -20,7 +20,7 @@ impl Int32 { let i: i32 = self.value.parse().map_err(|_| { extjson::de::Error::invalid_value( Unexpected::Str(self.value.as_str()), - &"expected i32 as a string", + &"i32 as a string", ) })?; Ok(i) @@ -39,7 +39,7 @@ impl Int64 { let i: i64 = self.value.parse().map_err(|_| { extjson::de::Error::invalid_value( Unexpected::Str(self.value.as_str()), - &"expected i64 as a string", + &"i64 as a string", ) })?; Ok(i) @@ -63,7 +63,7 @@ impl Double { let d: f64 = other.parse().map_err(|_| { extjson::de::Error::invalid_value( Unexpected::Str(other), - &"expected bson double as string", + &"bson double as string", ) })?; Ok(d) @@ -81,10 +81,11 @@ pub(crate) struct Decimal128 { impl Decimal128 { pub(crate) fn parse(self) -> extjson::de::Result { - self.value.parse().map_err(|_| { + self.value.parse().map_err(|err| { + dbg!(err); extjson::de::Error::invalid_value( Unexpected::Str(&self.value), - &"expected bson decimal128 as string", + &"bson decimal128 as string", ) }) } diff --git a/src/tests/spec/corpus.rs b/src/tests/spec/corpus.rs index 03406478..18783ff1 100644 --- a/src/tests/spec/corpus.rs +++ b/src/tests/spec/corpus.rs @@ -558,8 +558,8 @@ fn run_test(test: TestFile) { continue; } - let json: serde_json::Value = - serde_json::from_str(parse_error.string.as_str()).expect(&parse_error.description); + let json: serde_json::Value = serde_json::Value::String(parse_error.string); + //serde_json::from_str(parse_error.string.as_str()).expect(&parse_error.description); if let Ok(bson) = Bson::try_from(json.clone()) { // if converting to bson succeeds, assert that translating that bson to bytes fails From 9076e0689923fa568ead2309291ebd6daab1e2a9 Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Thu, 9 Feb 2023 14:33:48 -0500 Subject: [PATCH 21/30] passing! --- src/decimal128.rs | 257 +++------------------------------------------- 1 file changed, 12 insertions(+), 245 deletions(-) diff --git a/src/decimal128.rs b/src/decimal128.rs index 32aa036d..bc184588 100644 --- a/src/decimal128.rs +++ b/src/decimal128.rs @@ -384,19 +384,23 @@ impl std::str::FromStr for ParsedDecimal128 { // Check exponent limits if exp < Exponent::TINY { - let delta = (Exponent::TINY - exp).try_into().map_err(|_| ParseError::Overflow)?; - let new_precision = decimal_str.len().checked_sub(delta).ok_or(ParseError::Underflow)?; - decimal_str = round_decimal_str(decimal_str, new_precision)?; + if decimal_str != "0" { + let delta = (Exponent::TINY - exp).try_into().map_err(|_| ParseError::Overflow)?; + let new_precision = decimal_str.len().checked_sub(delta).ok_or(ParseError::Underflow)?; + decimal_str = round_decimal_str(decimal_str, new_precision)?; + } exp = Exponent::TINY; } let padded_str; if exp > Exponent::MAX { - let delta = (exp - Exponent::MAX).try_into().map_err(|_| ParseError::Overflow)?; - if decimal_str.len().checked_add(delta).ok_or(ParseError::Overflow)? > Coefficient::MAX_DIGITS { - return Err(ParseError::Overflow); + if decimal_str != "0" { + let delta = (exp - Exponent::MAX).try_into().map_err(|_| ParseError::Overflow)?; + if decimal_str.len().checked_add(delta).ok_or(ParseError::Overflow)? > Coefficient::MAX_DIGITS { + return Err(ParseError::Overflow); + } + padded_str = format!("{}{}", decimal_str, "0".repeat(delta)); + decimal_str = &padded_str; } - padded_str = format!("{}{}", decimal_str, "0".repeat(delta)); - decimal_str = &padded_str; exp = Exponent::MAX; } @@ -419,241 +423,4 @@ fn round_decimal_str(s: &str, precision: usize) -> Result<&str, ParseError> { return Err(ParseError::InexactRounding); } Ok(pre) -} - -#[cfg(test)] -mod tests { - use crate::Document; - - use super::*; - - fn dec_from_hex(s: &str) -> ParsedDecimal128 { - let bytes = hex::decode(s).unwrap(); - let d = crate::from_slice::(&bytes).unwrap(); - ParsedDecimal128::new(&d.get_decimal128("d").unwrap()) - } - - fn hex_from_dec(src: &ParsedDecimal128) -> String { - let bytes = crate::to_vec(&doc! { "d": src.pack() }).unwrap(); - hex::encode(bytes) - } - - #[test] - fn nan() { - let hex = "180000001364000000000000000000000000000000007C00"; - let parsed = dec_from_hex(hex); - assert_eq!(parsed, ParsedDecimal128 { - sign: false, - kind: Decimal128Kind::NaN { signalling: false }, - }); - assert_eq!(parsed.to_string(), "NaN"); - assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); - } - - #[test] - fn negative_nan() { - let hex = "18000000136400000000000000000000000000000000FC00"; - let parsed = dec_from_hex(hex); - assert_eq!(parsed, ParsedDecimal128 { - sign: true, - kind: Decimal128Kind::NaN { signalling: false }, - }); - assert_eq!(parsed.to_string(), "-NaN"); - assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); - } - - #[test] - fn snan() { - let hex = "180000001364000000000000000000000000000000007E00"; - let parsed = dec_from_hex(hex); - assert_eq!(parsed, ParsedDecimal128 { - sign: false, - kind: Decimal128Kind::NaN { signalling: true }, - }); - assert_eq!(parsed.to_string(), "sNaN"); - assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); - } - - #[test] - fn inf() { - let hex = "180000001364000000000000000000000000000000007800"; - let parsed = dec_from_hex(hex); - assert_eq!(parsed, ParsedDecimal128 { - sign: false, - kind: Decimal128Kind::Infinity, - }); - assert_eq!(parsed.to_string(), "Infinity"); - } - - fn finite_parts(parsed: &ParsedDecimal128) -> (i16, u128) { - if let Decimal128Kind::Finite { exponent, coefficient } = &parsed.kind { - (exponent.value(), coefficient.value()) - } else { - panic!("expected finite, got {:?}", parsed); - } - } - - #[test] - fn invalid_0() { - let hex = "180000001364000000000000000000000000000000106C00"; - let parsed = dec_from_hex(hex); - assert_eq!(parsed.to_string(), "0"); - assert!(!parsed.sign); - assert_eq!(finite_parts(&parsed), (0, 0)); - } - - #[test] - fn invalid_neg_0() { - let hex = "18000000136400DCBA9876543210DEADBEEF00000010EC00"; - let parsed = dec_from_hex(hex); - assert_eq!(parsed.to_string(), "-0"); - assert!(parsed.sign); - assert_eq!(finite_parts(&parsed), (0, 0)); - } - - #[test] - fn invalid_0_e3() { - let hex = "18000000136400FFFFFFFFFFFFFFFFFFFFFFFFFFFF116C00"; - let parsed = dec_from_hex(hex); - assert_eq!(parsed.to_string(), "0E+3"); - assert!(!parsed.sign); - assert_eq!(finite_parts(&parsed), (3, 0)); - } - - #[test] - fn finite_adjusted_exponent_limit() { - let hex = "18000000136400F2AF967ED05C82DE3297FF6FDE3CF22F00"; - let parsed = dec_from_hex(hex); - assert_eq!(parsed.to_string(), "0.000001234567890123456789012345678901234"); - assert!(!parsed.sign); - assert_eq!(finite_parts(&parsed), (-39, 1234567890123456789012345678901234)); - assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); - } - - #[test] - fn finite_0() { - let hex = "180000001364000000000000000000000000000000403000"; - let parsed = dec_from_hex(hex); - assert_eq!(parsed.to_string(), "0"); - assert!(!parsed.sign); - assert_eq!(finite_parts(&parsed), (0, 0)); - assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); - } - - #[test] - fn finite_0_parse() { - let hex = "180000001364000000000000000000000000000000403000"; - let parsed: ParsedDecimal128 = "0".parse().unwrap(); - assert_eq!(finite_parts(&parsed), (0, 0)); - assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); - } - - #[test] - fn finite_0_1() { - let hex = "1800000013640001000000000000000000000000003E3000"; - let parsed = dec_from_hex(hex); - assert_eq!(parsed.to_string(), "0.1"); - assert!(!parsed.sign); - assert_eq!(finite_parts(&parsed), (-1, 1)); - assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); - } - - #[test] - fn finite_0_1_parse() { - let hex = "1800000013640001000000000000000000000000003E3000"; - let parsed: ParsedDecimal128 = "0.1".parse().unwrap(); - assert_eq!(finite_parts(&parsed), (-1, 1)); - assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); - } - - #[test] - fn finite_long_decimal() { - let hex = "18000000136400F2AF967ED05C82DE3297FF6FDE3CFC2F00"; - let parsed = dec_from_hex(hex); - assert_eq!(parsed.to_string(), "0.1234567890123456789012345678901234"); - assert!(!parsed.sign); - assert_eq!(finite_parts(&parsed), (-34, 1234567890123456789012345678901234)); - assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); - } - - #[test] - fn finite_smallest() { - let hex = "18000000136400D204000000000000000000000000343000"; - let parsed = dec_from_hex(hex); - assert_eq!(parsed.to_string(), "0.001234"); - assert!(!parsed.sign); - assert_eq!(finite_parts(&parsed), (-6, 1234)); - assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); - } - - #[test] - fn finite_fractional() { - let hex = "1800000013640064000000000000000000000000002CB000"; - let parsed = dec_from_hex(hex); - assert_eq!(parsed.to_string(), "-1.00E-8"); - assert!(parsed.sign); - assert_eq!(finite_parts(&parsed), (-10, 100)); - assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); - } - - #[test] - fn finite_largest() { - let hex = "18000000136400F2AF967ED05C82DE3297FF6FDE3C403000"; - let parsed = dec_from_hex(hex); - assert_eq!(parsed.to_string(), "1234567890123456789012345678901234"); - assert!(!parsed.sign); - assert_eq!(finite_parts(&parsed), (0, 1234567890123456789012345678901234)); - assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); - } - - #[test] - fn finite_scientific_largest() { - let hex = "18000000136400FFFFFFFF638E8D37C087ADBE09EDFF5F00"; - let parsed = dec_from_hex(hex); - assert_eq!(parsed.to_string(), "9.999999999999999999999999999999999E+6144"); - assert!(!parsed.sign); - assert_eq!(finite_parts(&parsed), (6111, 9999999999999999999999999999999999)); - assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); - } - - #[test] - fn finite_scientific_largest_parse() { - let hex = "18000000136400FFFFFFFF638E8D37C087ADBE09EDFF5F00"; - let parsed: ParsedDecimal128 = "9.999999999999999999999999999999999E+6144".parse().unwrap(); - assert!(!parsed.sign); - assert_eq!(finite_parts(&parsed), (6111, 9999999999999999999999999999999999)); - assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); - } - - #[test] - fn noncanonical_exponent_normalization() { - let hex = "1800000013640064000000000000000000000000002CB000"; - let parsed: ParsedDecimal128 = "-100E-10".parse().unwrap(); - assert_eq!(parsed.to_string(), "-1.00E-8"); - assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); - } - - #[test] - fn rounded_subnormal() { - let hex = "180000001364000100000000000000000000000000000000"; - let parsed: ParsedDecimal128 = "10E-6177".parse().unwrap(); - assert_eq!(parsed.to_string(), "1E-6176"); - assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); - } - - #[test] - fn clamped() { - let hex = "180000001364000a00000000000000000000000000fe5f00"; - let parsed: ParsedDecimal128 = "1E6112".parse().unwrap(); - assert_eq!(parsed.to_string(), "1.0E+6112"); - assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); - } - - #[test] - fn exact_rounding() { - let hex = "18000000136400000000000a5bc138938d44c64d31cc3700"; - let parsed: ParsedDecimal128 = "1000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000".parse().unwrap(); - assert_eq!(parsed.to_string(), "1.000000000000000000000000000000000E+999"); - assert_eq!(hex_from_dec(&parsed).to_ascii_lowercase(), hex.to_ascii_lowercase()); - } } \ No newline at end of file From d03effff96e997ebd4d794e1e17e30edbb309d86 Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Thu, 9 Feb 2023 14:43:07 -0500 Subject: [PATCH 22/30] format and clippy --- src/de/serde.rs | 14 ++--- src/decimal128.rs | 112 ++++++++++++++++++++++++--------------- src/tests/spec/corpus.rs | 1 - 3 files changed, 78 insertions(+), 49 deletions(-) diff --git a/src/de/serde.rs b/src/de/serde.rs index a8f88122..0ab77187 100644 --- a/src/de/serde.rs +++ b/src/de/serde.rs @@ -467,12 +467,14 @@ impl<'de> Visitor<'de> for BsonVisitor { "$numberDecimal" => { let string: String = visitor.next_value()?; - return Ok(Bson::Decimal128(string.parse::().map_err(|_| { - V::Error::invalid_value( - Unexpected::Str(&string), - &"decimal128 as a string", - ) - })?)); + return Ok(Bson::Decimal128(string.parse::().map_err( + |_| { + V::Error::invalid_value( + Unexpected::Str(&string), + &"decimal128 as a string", + ) + }, + )?)); } "$numberDecimalBytes" => { diff --git a/src/decimal128.rs b/src/decimal128.rs index bc184588..aa235580 100644 --- a/src/decimal128.rs +++ b/src/decimal128.rs @@ -63,12 +63,14 @@ type Order = Msb0; #[derive(Debug, Clone, PartialEq)] enum Decimal128Kind { - NaN { signalling: bool }, + NaN { + signalling: bool, + }, Infinity, Finite { exponent: Exponent, coefficient: Coefficient, - } + }, } #[derive(Debug, Clone, PartialEq)] @@ -83,17 +85,13 @@ impl Exponent { fn from_bits(src_bits: &BitSlice) -> Self { let mut bytes = [0u8; 2]; - bytes - .view_bits_mut::()[Self::UNUSED_BITS..] - .copy_from_bitslice(src_bits); + bytes.view_bits_mut::()[Self::UNUSED_BITS..].copy_from_bitslice(src_bits); Self(bytes) } fn from_native(value: i16) -> Self { let mut bytes = [0u8; 2]; - bytes - .view_bits_mut::() - .store_be(value + Self::BIAS); + bytes.view_bits_mut::().store_be(value + Self::BIAS); Self(bytes) } @@ -135,9 +133,7 @@ impl Coefficient { fn from_native(value: u128) -> Self { let mut bytes = [0u8; 16]; - bytes - .view_bits_mut::() - .store_be(value); + bytes.view_bits_mut::().store_be(value); Self(bytes) } @@ -152,11 +148,13 @@ impl Coefficient { impl ParsedDecimal128 { fn new(source: &Decimal128) -> Self { - // BSON byte order is the opposite of the decimal128 byte order, so flip 'em. The rest of this method could be rewritten to not need this, but readability is helped by keeping the implementation congruent with the spec. + // BSON byte order is the opposite of the decimal128 spec byte order, so flip 'em. The rest + // of this method could be rewritten to not need this, but readability is helped by + // keeping the implementation congruent with the spec. let tmp: [u8; 16] = { let mut tmp = [0u8; 16]; - for i in 0..16 { - tmp[i] = source.bytes[15-i]; + for (ix, b) in tmp.iter_mut().enumerate() { + *b = source.bytes[15 - ix]; } tmp }; @@ -166,7 +164,9 @@ impl ParsedDecimal128 { let kind = if src_bits[1..5].all() { // Special value if src_bits[5] { - Decimal128Kind::NaN { signalling: src_bits[6] } + Decimal128Kind::NaN { + signalling: src_bits[6], + } } else { Decimal128Kind::Infinity } @@ -182,8 +182,12 @@ impl ParsedDecimal128 { coeff_prefix = bits![static u8, Msb0; 0]; } - let exponent = Exponent::from_bits(&src_bits[exponent_offset..exponent_offset+Exponent::WIDTH]); - let coefficient = Coefficient::from_bits(coeff_prefix, &src_bits[exponent_offset+Exponent::WIDTH..]); + let exponent = + Exponent::from_bits(&src_bits[exponent_offset..exponent_offset + Exponent::WIDTH]); + let coefficient = Coefficient::from_bits( + coeff_prefix, + &src_bits[exponent_offset + Exponent::WIDTH..], + ); Decimal128Kind::Finite { exponent, coefficient, @@ -206,7 +210,10 @@ impl ParsedDecimal128 { Decimal128Kind::Infinity => { dest_bits[1..6].clone_from_bitslice(bits![1, 1, 1, 1, 0]); } - Decimal128Kind::Finite { exponent, coefficient } => { + Decimal128Kind::Finite { + exponent, + coefficient, + } => { let mut coeff_bits = coefficient.bits(); let exponent_offset; if coeff_bits[0] { @@ -218,16 +225,15 @@ impl ParsedDecimal128 { coeff_bits = &coeff_bits[1..]; exponent_offset = 1; }; - dest_bits[exponent_offset..exponent_offset+Exponent::WIDTH] + dest_bits[exponent_offset..exponent_offset + Exponent::WIDTH] .copy_from_bitslice(exponent.bits()); - dest_bits[exponent_offset+Exponent::WIDTH..] - .copy_from_bitslice(coeff_bits); + dest_bits[exponent_offset + Exponent::WIDTH..].copy_from_bitslice(coeff_bits); } } let mut bytes = [0u8; 16]; for i in 0..16 { - bytes[i] = tmp[15-i]; + bytes[i] = tmp[15 - i]; } Decimal128 { bytes } } @@ -240,7 +246,9 @@ impl fmt::Display for ParsedDecimal128 { write!(f, "-")?; } match &self.kind { - Decimal128Kind::NaN { signalling: _signalling } => { + Decimal128Kind::NaN { + signalling: _signalling, + } => { /* Likewise, MongoDB requires no 's' prefix for signalling. if *signalling { write!(f, "s")?; @@ -249,7 +257,10 @@ impl fmt::Display for ParsedDecimal128 { write!(f, "NaN")?; } Decimal128Kind::Infinity => write!(f, "Infinity")?, - Decimal128Kind::Finite { exponent, coefficient } => { + Decimal128Kind::Finite { + exponent, + coefficient, + } => { let coeff_str = format!("{}", coefficient.value()); let exp_val = exponent.value(); let adj_exp = exp_val + (coeff_str.len() as i16) - 1; @@ -258,7 +269,7 @@ impl fmt::Display for ParsedDecimal128 { if exp_val == 0 { write!(f, "{}", coeff_str)?; } else { - let dec_charlen = exp_val.abs() as usize; + let dec_charlen = exp_val.unsigned_abs() as usize; if dec_charlen >= coeff_str.len() { write!(f, "0.")?; write!(f, "{}", "0".repeat(dec_charlen - coeff_str.len()))?; @@ -328,8 +339,8 @@ impl std::str::FromStr for ParsedDecimal128 { fn from_str(mut s: &str) -> Result { let sign; - if let Some(rest) = s.strip_prefix(&['-', '+']) { - sign = s.chars().next() == Some('-'); + if let Some(rest) = s.strip_prefix(['-', '+']) { + sign = s.starts_with('-'); s = rest; } else { sign = false; @@ -353,7 +364,9 @@ impl std::str::FromStr for ParsedDecimal128 { exp_str = post; } } - let mut exp = exp_str.parse::().map_err(|e| ParseError::InvalidExponent(e))?; + let mut exp = exp_str + .parse::() + .map_err(ParseError::InvalidExponent)?; // Remove decimal point and adjust exponent let joined_str; @@ -366,18 +379,16 @@ impl std::str::FromStr for ParsedDecimal128 { // Strip leading zeros let rest = decimal_str.trim_start_matches('0'); - decimal_str = if rest.is_empty() { - "0" - } else { - rest - }; + decimal_str = if rest.is_empty() { "0" } else { rest }; // Check decimal precision { let len = decimal_str.len(); if len > Coefficient::MAX_DIGITS { decimal_str = round_decimal_str(decimal_str, Coefficient::MAX_DIGITS)?; - let exp_adj = (len - decimal_str.len()).try_into().map_err(|_| ParseError::Overflow)?; + let exp_adj = (len - decimal_str.len()) + .try_into() + .map_err(|_| ParseError::Overflow)?; exp = exp.checked_add(exp_adj).ok_or(ParseError::Overflow)?; } } @@ -385,8 +396,13 @@ impl std::str::FromStr for ParsedDecimal128 { // Check exponent limits if exp < Exponent::TINY { if decimal_str != "0" { - let delta = (Exponent::TINY - exp).try_into().map_err(|_| ParseError::Overflow)?; - let new_precision = decimal_str.len().checked_sub(delta).ok_or(ParseError::Underflow)?; + let delta = (Exponent::TINY - exp) + .try_into() + .map_err(|_| ParseError::Overflow)?; + let new_precision = decimal_str + .len() + .checked_sub(delta) + .ok_or(ParseError::Underflow)?; decimal_str = round_decimal_str(decimal_str, new_precision)?; } exp = Exponent::TINY; @@ -394,8 +410,15 @@ impl std::str::FromStr for ParsedDecimal128 { let padded_str; if exp > Exponent::MAX { if decimal_str != "0" { - let delta = (exp - Exponent::MAX).try_into().map_err(|_| ParseError::Overflow)?; - if decimal_str.len().checked_add(delta).ok_or(ParseError::Overflow)? > Coefficient::MAX_DIGITS { + let delta = (exp - Exponent::MAX) + .try_into() + .map_err(|_| ParseError::Overflow)?; + if decimal_str + .len() + .checked_add(delta) + .ok_or(ParseError::Overflow)? + > Coefficient::MAX_DIGITS + { return Err(ParseError::Overflow); } padded_str = format!("{}{}", decimal_str, "0".repeat(delta)); @@ -406,10 +429,15 @@ impl std::str::FromStr for ParsedDecimal128 { // Assemble the final value let exponent = Exponent::from_native(exp); - let coeff: u128 = decimal_str.parse().map_err(|e| ParseError::InvalidCoefficient(e))?; + let coeff: u128 = decimal_str + .parse() + .map_err(ParseError::InvalidCoefficient)?; let coefficient = Coefficient::from_native(coeff); - Decimal128Kind::Finite { exponent, coefficient } - }, + Decimal128Kind::Finite { + exponent, + coefficient, + } + } }; Ok(Self { sign, kind }) @@ -423,4 +451,4 @@ fn round_decimal_str(s: &str, precision: usize) -> Result<&str, ParseError> { return Err(ParseError::InexactRounding); } Ok(pre) -} \ No newline at end of file +} diff --git a/src/tests/spec/corpus.rs b/src/tests/spec/corpus.rs index 18783ff1..88d872ca 100644 --- a/src/tests/spec/corpus.rs +++ b/src/tests/spec/corpus.rs @@ -559,7 +559,6 @@ fn run_test(test: TestFile) { } let json: serde_json::Value = serde_json::Value::String(parse_error.string); - //serde_json::from_str(parse_error.string.as_str()).expect(&parse_error.description); if let Ok(bson) = Bson::try_from(json.clone()) { // if converting to bson succeeds, assert that translating that bson to bytes fails From 6d455025454723bdf8ba888cd8a9e415670710a8 Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Thu, 9 Feb 2023 15:06:59 -0500 Subject: [PATCH 23/30] readability tweaks --- src/decimal128.rs | 56 ++++++++++++++++++++++------------------------- 1 file changed, 26 insertions(+), 30 deletions(-) diff --git a/src/decimal128.rs b/src/decimal128.rs index aa235580..33d78d15 100644 --- a/src/decimal128.rs +++ b/src/decimal128.rs @@ -59,8 +59,6 @@ struct ParsedDecimal128 { kind: Decimal128Kind, } -type Order = Msb0; - #[derive(Debug, Clone, PartialEq)] enum Decimal128Kind { NaN { @@ -78,29 +76,30 @@ struct Exponent([u8; 2]); impl Exponent { const BIAS: i16 = 6176; - const UNUSED_BITS: usize = 2; - const WIDTH: usize = 14; const TINY: i16 = -6176; const MAX: i16 = 6111; - fn from_bits(src_bits: &BitSlice) -> Self { + const UNUSED_BITS: usize = 2; + const PACKED_WIDTH: usize = 14; + + fn from_bits(src_bits: &BitSlice) -> Self { let mut bytes = [0u8; 2]; - bytes.view_bits_mut::()[Self::UNUSED_BITS..].copy_from_bitslice(src_bits); + bytes.view_bits_mut::()[Self::UNUSED_BITS..].copy_from_bitslice(src_bits); Self(bytes) } fn from_native(value: i16) -> Self { let mut bytes = [0u8; 2]; - bytes.view_bits_mut::().store_be(value + Self::BIAS); + bytes.view_bits_mut::().store_be(value + Self::BIAS); Self(bytes) } - fn bits(&self) -> &BitSlice { - &self.0.view_bits::()[Self::UNUSED_BITS..] + fn bits(&self) -> &BitSlice { + &self.0.view_bits::()[Self::UNUSED_BITS..] } fn raw(&self) -> u16 { - self.0.view_bits::().load_be::() + self.0.view_bits::().load_be::() } fn value(&self) -> i16 { @@ -116,9 +115,9 @@ impl Coefficient { const MAX_DIGITS: usize = 34; const MAX_VALUE: u128 = 9_999_999_999_999_999_999_999_999_999_999_999; - fn from_bits(src_prefix: &BitSlice, src_suffix: &BitSlice) -> Self { + fn from_bits(src_prefix: &BitSlice, src_suffix: &BitSlice) -> Self { let mut bytes = [0u8; 16]; - let bits = &mut bytes.view_bits_mut::()[Self::UNUSED_BITS..]; + let bits = &mut bytes.view_bits_mut::()[Self::UNUSED_BITS..]; let prefix_len = src_prefix.len(); bits[0..prefix_len].copy_from_bitslice(src_prefix); bits[prefix_len..].copy_from_bitslice(src_suffix); @@ -133,22 +132,22 @@ impl Coefficient { fn from_native(value: u128) -> Self { let mut bytes = [0u8; 16]; - bytes.view_bits_mut::().store_be(value); + bytes.view_bits_mut::().store_be(value); Self(bytes) } - fn bits(&self) -> &BitSlice { - &self.0.view_bits::()[Self::UNUSED_BITS..] + fn bits(&self) -> &BitSlice { + &self.0.view_bits::()[Self::UNUSED_BITS..] } fn value(&self) -> u128 { - self.0.view_bits::().load_be::() + self.0.view_bits::().load_be::() } } impl ParsedDecimal128 { fn new(source: &Decimal128) -> Self { - // BSON byte order is the opposite of the decimal128 spec byte order, so flip 'em. The rest + // BSON byte Msb0 is the opposite of the decimal128 spec byte Msb0, so flip 'em. The rest // of this method could be rewritten to not need this, but readability is helped by // keeping the implementation congruent with the spec. let tmp: [u8; 16] = { @@ -158,7 +157,7 @@ impl ParsedDecimal128 { } tmp }; - let src_bits = tmp.view_bits::(); + let src_bits = tmp.view_bits::(); let sign = src_bits[0]; let kind = if src_bits[1..5].all() { @@ -181,13 +180,10 @@ impl ParsedDecimal128 { exponent_offset = 1; coeff_prefix = bits![static u8, Msb0; 0]; } + let coeff_offset = exponent_offset + Exponent::PACKED_WIDTH; - let exponent = - Exponent::from_bits(&src_bits[exponent_offset..exponent_offset + Exponent::WIDTH]); - let coefficient = Coefficient::from_bits( - coeff_prefix, - &src_bits[exponent_offset + Exponent::WIDTH..], - ); + let exponent = Exponent::from_bits(&src_bits[exponent_offset..coeff_offset]); + let coefficient = Coefficient::from_bits(coeff_prefix, &src_bits[coeff_offset..]); Decimal128Kind::Finite { exponent, coefficient, @@ -198,17 +194,17 @@ impl ParsedDecimal128 { fn pack(&self) -> Decimal128 { let mut tmp = [0u8; 16]; - let dest_bits = tmp.view_bits_mut::(); + let dest_bits = tmp.view_bits_mut::(); dest_bits.set(0, self.sign); match &self.kind { Decimal128Kind::NaN { signalling } => { - dest_bits[1..6].clone_from_bitslice(bits![1, 1, 1, 1, 1]); + dest_bits[1..6].copy_from_bitslice(bits![u8, Msb0; 1, 1, 1, 1, 1]); dest_bits.set(6, *signalling); } Decimal128Kind::Infinity => { - dest_bits[1..6].clone_from_bitslice(bits![1, 1, 1, 1, 0]); + dest_bits[1..6].copy_from_bitslice(bits![u8, Msb0; 1, 1, 1, 1, 0]); } Decimal128Kind::Finite { exponent, @@ -225,9 +221,9 @@ impl ParsedDecimal128 { coeff_bits = &coeff_bits[1..]; exponent_offset = 1; }; - dest_bits[exponent_offset..exponent_offset + Exponent::WIDTH] - .copy_from_bitslice(exponent.bits()); - dest_bits[exponent_offset + Exponent::WIDTH..].copy_from_bitslice(coeff_bits); + let coeff_offset = exponent_offset + Exponent::PACKED_WIDTH; + dest_bits[exponent_offset..coeff_offset].copy_from_bitslice(exponent.bits()); + dest_bits[coeff_offset..].copy_from_bitslice(coeff_bits); } } From 6bff1638609f360b76a063bc88c32207a05778ab Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Thu, 9 Feb 2023 15:08:43 -0500 Subject: [PATCH 24/30] bump msrv to 1.56 --- .evergreen/config.yml | 4 ++-- src/lib.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.evergreen/config.yml b/.evergreen/config.yml index fc992e4f..ff09d7ff 100644 --- a/.evergreen/config.yml +++ b/.evergreen/config.yml @@ -181,9 +181,9 @@ axes: - id: "extra-rust-versions" values: - id: "min" - display_name: "1.53 (minimum supported version)" + display_name: "1.56 (minimum supported version)" variables: - RUST_VERSION: "1.53.0" + RUST_VERSION: "1.56.0" MSRV: "true" - id: "nightly" display_name: "nightly" diff --git a/src/lib.rs b/src/lib.rs index afe69bf4..d940dae9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -43,7 +43,7 @@ //! //! ## Installation //! ### Requirements -//! - Rust 1.53+ +//! - Rust 1.56+ //! //! ### Importing //! This crate is available on [crates.io](https://crates.io/crates/bson). To use it in your application, @@ -260,7 +260,7 @@ //! //! ## Minimum supported Rust version (MSRV) //! -//! The MSRV for this crate is currently 1.53.0. This will be rarely be increased, and if it ever is, +//! The MSRV for this crate is currently 1.56.0. This will be rarely be increased, and if it ever is, //! it will only happen in a minor or major version release. #![allow(clippy::cognitive_complexity, clippy::derive_partial_eq_without_eq)] From e8e411b28f4e000cb5486b622ad5d6ea4d7d5f3e Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Thu, 9 Feb 2023 15:27:19 -0500 Subject: [PATCH 25/30] fix serde-tests --- serde-tests/json.rs | 2 +- src/bson.rs | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/serde-tests/json.rs b/serde-tests/json.rs index 473b3f21..417ea6c9 100644 --- a/serde-tests/json.rs +++ b/serde-tests/json.rs @@ -48,7 +48,7 @@ fn all_types_json() { "undefined": { "$undefined": true }, "code": { "$code": code }, "code_w_scope": { "$code": code_w_scope.code, "$scope": scope_json }, - "decimal": { "$numberDecimalBytes": v.decimal.bytes() }, + "decimal": { "$numberDecimal": v.decimal.to_string() }, "symbol": { "$symbol": "ok" }, "min_key": { "$minKey": 1 }, "max_key": { "$maxKey": 1 }, diff --git a/src/bson.rs b/src/bson.rs index 9d64ed7c..92f93a8b 100644 --- a/src/bson.rs +++ b/src/bson.rs @@ -702,6 +702,14 @@ impl Bson { _ => {} }, + ["$numberDecimal"] => { + if let Ok(d) = doc.get_str("$numberDecimal") { + if let Ok(d) = d.parse() { + return Bson::Decimal128(d); + } + } + } + ["$numberDecimalBytes"] => { if let Ok(bytes) = doc.get_binary_generic("$numberDecimalBytes") { if let Ok(b) = bytes.clone().try_into() { From 82461c05469e94810d2849c7b2183c5866972cb1 Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Thu, 9 Feb 2023 15:35:20 -0500 Subject: [PATCH 26/30] msrv compile fix --- src/decimal128.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/decimal128.rs b/src/decimal128.rs index 33d78d15..cd6c0c59 100644 --- a/src/decimal128.rs +++ b/src/decimal128.rs @@ -335,7 +335,7 @@ impl std::str::FromStr for ParsedDecimal128 { fn from_str(mut s: &str) -> Result { let sign; - if let Some(rest) = s.strip_prefix(['-', '+']) { + if let Some(rest) = s.strip_prefix(&['-', '+']) { sign = s.starts_with('-'); s = rest; } else { From 95da0985ebbf40df76688eba6769a24065eb064b Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Thu, 9 Feb 2023 15:56:18 -0500 Subject: [PATCH 27/30] fix msrv compilation again --- src/decimal128.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/decimal128.rs b/src/decimal128.rs index cd6c0c59..77de7734 100644 --- a/src/decimal128.rs +++ b/src/decimal128.rs @@ -335,7 +335,7 @@ impl std::str::FromStr for ParsedDecimal128 { fn from_str(mut s: &str) -> Result { let sign; - if let Some(rest) = s.strip_prefix(&['-', '+']) { + if let Some(rest) = s.strip_prefix(&['-', '+'][..]) { sign = s.starts_with('-'); s = rest; } else { From 869acb8e01317bc87aa4913de5fb089b34071d84 Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Fri, 10 Feb 2023 07:42:33 -0500 Subject: [PATCH 28/30] shift error handling --- src/decimal128.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/decimal128.rs b/src/decimal128.rs index 77de7734..375ecc22 100644 --- a/src/decimal128.rs +++ b/src/decimal128.rs @@ -115,7 +115,7 @@ impl Coefficient { const MAX_DIGITS: usize = 34; const MAX_VALUE: u128 = 9_999_999_999_999_999_999_999_999_999_999_999; - fn from_bits(src_prefix: &BitSlice, src_suffix: &BitSlice) -> Self { + fn from_bits(src_prefix: &BitSlice, src_suffix: &BitSlice) -> Result { let mut bytes = [0u8; 16]; let bits = &mut bytes.view_bits_mut::()[Self::UNUSED_BITS..]; let prefix_len = src_prefix.len(); @@ -123,10 +123,9 @@ impl Coefficient { bits[prefix_len..].copy_from_bitslice(src_suffix); let out = Self(bytes); if out.value() > Self::MAX_VALUE { - // Invalid coefficients get silently replaced with zero. - Self([0u8; 16]) + Err(ParseError::Overflow) } else { - out + Ok(out) } } @@ -147,7 +146,7 @@ impl Coefficient { impl ParsedDecimal128 { fn new(source: &Decimal128) -> Self { - // BSON byte Msb0 is the opposite of the decimal128 spec byte Msb0, so flip 'em. The rest + // BSON byte order is the opposite of the decimal128 spec byte order, so flip 'em. The rest // of this method could be rewritten to not need this, but readability is helped by // keeping the implementation congruent with the spec. let tmp: [u8; 16] = { @@ -183,7 +182,11 @@ impl ParsedDecimal128 { let coeff_offset = exponent_offset + Exponent::PACKED_WIDTH; let exponent = Exponent::from_bits(&src_bits[exponent_offset..coeff_offset]); - let coefficient = Coefficient::from_bits(coeff_prefix, &src_bits[coeff_offset..]); + let coefficient = match Coefficient::from_bits(coeff_prefix, &src_bits[coeff_offset..]) { + Ok(c) => c, + // Invalid coefficients get silently replaced with zero. + Err(_) => Coefficient([0u8; 16]), + }; Decimal128Kind::Finite { exponent, coefficient, From c4c4cb7f076dff65eb9f76414dbe1509766e33ac Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Fri, 10 Feb 2023 07:59:52 -0500 Subject: [PATCH 29/30] simpler byteorder flipping --- src/decimal128.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/decimal128.rs b/src/decimal128.rs index 375ecc22..71fb9b8b 100644 --- a/src/decimal128.rs +++ b/src/decimal128.rs @@ -115,7 +115,10 @@ impl Coefficient { const MAX_DIGITS: usize = 34; const MAX_VALUE: u128 = 9_999_999_999_999_999_999_999_999_999_999_999; - fn from_bits(src_prefix: &BitSlice, src_suffix: &BitSlice) -> Result { + fn from_bits( + src_prefix: &BitSlice, + src_suffix: &BitSlice, + ) -> Result { let mut bytes = [0u8; 16]; let bits = &mut bytes.view_bits_mut::()[Self::UNUSED_BITS..]; let prefix_len = src_prefix.len(); @@ -151,9 +154,8 @@ impl ParsedDecimal128 { // keeping the implementation congruent with the spec. let tmp: [u8; 16] = { let mut tmp = [0u8; 16]; - for (ix, b) in tmp.iter_mut().enumerate() { - *b = source.bytes[15 - ix]; - } + tmp.view_bits_mut::() + .store_be(source.bytes.view_bits::().load_le::()); tmp }; let src_bits = tmp.view_bits::(); @@ -182,7 +184,8 @@ impl ParsedDecimal128 { let coeff_offset = exponent_offset + Exponent::PACKED_WIDTH; let exponent = Exponent::from_bits(&src_bits[exponent_offset..coeff_offset]); - let coefficient = match Coefficient::from_bits(coeff_prefix, &src_bits[coeff_offset..]) { + let coefficient = match Coefficient::from_bits(coeff_prefix, &src_bits[coeff_offset..]) + { Ok(c) => c, // Invalid coefficients get silently replaced with zero. Err(_) => Coefficient([0u8; 16]), @@ -231,9 +234,9 @@ impl ParsedDecimal128 { } let mut bytes = [0u8; 16]; - for i in 0..16 { - bytes[i] = tmp[15 - i]; - } + bytes + .view_bits_mut::() + .store_le(tmp.view_bits::().load_be::()); Decimal128 { bytes } } } From fc99b938cf1aee7c2bbaba6babc5c8c2e6ac9243 Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Wed, 15 Feb 2023 12:05:08 -0500 Subject: [PATCH 30/30] review updates --- src/decimal128.rs | 9 +++++++++ src/extjson/models.rs | 3 +-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/decimal128.rs b/src/decimal128.rs index 71fb9b8b..eb0ee57d 100644 --- a/src/decimal128.rs +++ b/src/decimal128.rs @@ -75,11 +75,17 @@ enum Decimal128Kind { struct Exponent([u8; 2]); impl Exponent { + /// The exponent is stored as an unsigned value; `BIAS` is subtracted to get the actual value. const BIAS: i16 = 6176; + /// The minimum representable exponent. This is distinct from the specifications "min" value, + /// which marks the point at which exponents are considered subnormal. const TINY: i16 = -6176; + /// The maximum representable exponent. const MAX: i16 = 6111; + /// The number of unused bits in the parsed representation. const UNUSED_BITS: usize = 2; + /// The total number of bits in the packed representation. const PACKED_WIDTH: usize = 14; fn from_bits(src_bits: &BitSlice) -> Self { @@ -111,8 +117,11 @@ impl Exponent { struct Coefficient([u8; 16]); impl Coefficient { + /// The number of unused bits in the parsed representation. const UNUSED_BITS: usize = 14; + /// The maximum number of digits allowed in a base-10 string representation of the coefficient. const MAX_DIGITS: usize = 34; + /// The maximum allowable value of a coefficient. const MAX_VALUE: u128 = 9_999_999_999_999_999_999_999_999_999_999_999; fn from_bits( diff --git a/src/extjson/models.rs b/src/extjson/models.rs index 3de79b7d..48775113 100644 --- a/src/extjson/models.rs +++ b/src/extjson/models.rs @@ -81,8 +81,7 @@ pub(crate) struct Decimal128 { impl Decimal128 { pub(crate) fn parse(self) -> extjson::de::Result { - self.value.parse().map_err(|err| { - dbg!(err); + self.value.parse().map_err(|_| { extjson::de::Error::invalid_value( Unexpected::Str(&self.value), &"bson decimal128 as string",