Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Feature/ordered float #399

Merged
merged 12 commits into from
Oct 22, 2024
Merged

Feature/ordered float #399

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
7913172
Clean up most issues with using the wrapped float feature from value-…
Oct 21, 2024
d06b953
Cleanups that make things compile: NOT TESTED
Oct 21, 2024
b2b5dc4
Revert local config
Oct 21, 2024
668bc92
MY clippy lints - there are a bunch of "transmute" lints that are sti…
Oct 22, 2024
2b2f2e2
Use my value-trait fork until changes merged
Oct 22, 2024
a5b704e
Remove unnecessary adapters, fix feature combination errors
Oct 22, 2024
8acd322
Use value-trait repo master branch while awaiting for publication
Oct 22, 2024
18ace2c
Fix branch name
Oct 22, 2024
2703f90
FInally derive Eq for BorrowedValue and OwnedValue and prove it works!
Oct 22, 2024
1c41d6a
Remove patch for value-trait (PR has been published)
Oct 22, 2024
91ba39c
Update doc
Oct 22, 2024
3a571c4
Cargo fmt
Oct 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ simdutf8 = { version = "0.1.4", features = ["public_imp", "aarch64_neon"] }

beef = { version = "0.5", optional = true }
halfbrown = "0.2"
value-trait = { version = "0.10.0" }
value-trait = { version = "0.10" }
# ahash known key
once_cell = { version = "1.17", optional = true }
ahash = { version = "0.8", optional = true }
Expand Down Expand Up @@ -95,6 +95,9 @@ no-inline = []
# also bench serde in the benchmarks
bench-serde = ["serde_json"]

# use an Eq wrapper for floats
ordered-float = ["value-trait/ordered-float"]

# use branch hints - requires nightly :(
hints = [] # requires nightly

Expand Down
10 changes: 10 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,16 @@ Replace [`std::borrow::Cow`](https://doc.rust-lang.org/std/borrow/enum.Cow.html)
[`beef::lean::Cow`][beef] This feature is disabled by default, because
it is a breaking change in the API.

### `ordered-float`

By default the representation of `Floats` used in `borrowed::Value ` and `owned::Value` is simply a value of `f64`.
This however has the normally-not-a-big-deal side effect of _not_ having these `Value` types be `std::cmp::Eq`. This does,
however, introduce some incompatibilities when offering `simd-json` as a quasi-drop-in replacement for `serde-json`.

So, this feature changes the internal representation of `Floats` to be an `f64` _wrapped by [an Eq-compatible adapter](https://docs.rs/ordered-float/latest/ordered_float/)_.

This probably carries with it some small performance trade-offs, hence its enablement by feature rather than by default.

### `portable`

**Currently disabled**
Expand Down
6 changes: 3 additions & 3 deletions src/numberparse/approx.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ impl<'de> Deserializer<'de> {
ErrorType::InvalidNumber,
))
} else {
Ok(StaticNode::F64(if negative { -i } else { i }))
Ok(StaticNode::from(if negative { -i } else { i }))
}
}

Expand Down Expand Up @@ -523,15 +523,15 @@ impl<'de> Deserializer<'de> {
// We want 0.1e1 to be a float.
//////////
if i == 0 {
StaticNode::F64(0.0)
StaticNode::from(0.0)
} else {
if !(-323..=308).contains(&exponent) {
return Self::parse_float(idx, buf, negative);
}

let mut d1: f64 = i as f64;
d1 *= unsafe { POWER_OF_TEN.get_kinda_unchecked((323 + exponent) as usize) };
StaticNode::F64(if negative { d1 * -1.0 } else { d1 })
StaticNode::from(if negative { d1 * -1.0 } else { d1 })
}
} else {
if unlikely!(byte_count >= 18) {
Expand Down
52 changes: 31 additions & 21 deletions src/numberparse/correct.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,9 +321,9 @@ fn f64_from_parts(
} else {
f *= get!(POW10, exponent);
}
Ok(StaticNode::F64(if positive { f } else { -f }))
Ok(StaticNode::from(if positive { f } else { -f }))
} else if significand == 0 {
Ok(StaticNode::F64(if positive { 0.0 } else { -0.0 }))
Ok(StaticNode::from(if positive { 0.0 } else { -0.0 }))
} else if (-325..=308).contains(&exponent) {
let (factor_mantissa, factor_exponent) = get!(POW10_COMPONENTS, exponent + 325);
let mut leading_zeroes = u64::from(significand.leading_zeros());
Expand Down Expand Up @@ -373,7 +373,7 @@ fn f64_from_parts(
if res.is_infinite() {
err!(offset, get!(slice, offset))
}
Ok(StaticNode::F64(res))
Ok(StaticNode::from(res))
} else {
f64_from_parts_slow(slice, offset)
}
Expand All @@ -389,7 +389,7 @@ fn f64_from_parts_slow(slice: &[u8], offset: usize) -> Result<StaticNode> {
err!(offset, get!(slice, 0))
}

Ok(StaticNode::F64(val))
Ok(StaticNode::from(val))
}
Err(_) => err!(offset, get!(slice, offset)),
}
Expand All @@ -402,9 +402,7 @@ mod test {
use crate::value::owned::to_value;
use crate::value::owned::Value;
use crate::value::owned::Value::Static;
use value_trait::StaticNode::F64;
use value_trait::StaticNode::I64;
use value_trait::StaticNode::U64;
use value_trait::StaticNode::{self, I64, U64};

fn to_value_from_str(buf: &str) -> Result<Value, Error> {
let mut val = String::from(buf);
Expand All @@ -417,28 +415,34 @@ mod test {
fn float() -> Result<(), crate::Error> {
assert_eq!(
to_value_from_str("0.4e5").expect("40000.0"),
Static(F64(40000.0))
Static(StaticNode::from(40000.0))
);
assert_eq!(
to_value_from_str("-12345678901234.56789012")?,
Static(F64(-12_345_678_901_234.568))
Static(StaticNode::from(-12_345_678_901_234.568))
);
assert_eq!(
to_value_from_str("0.4e-001")?,
Static(StaticNode::from(0.04))
);
assert_eq!(to_value_from_str("0.4e-001")?, Static(F64(0.04)));
assert_eq!(
to_value_from_str("0.123456789e-12")?,
Static(F64(1.234_567_89e-13))
Static(StaticNode::from(1.234_567_89e-13))
);
assert_eq!(to_value_from_str("1.234567890E+34")?, 1.234_567_89e34);
assert_eq!(
to_value_from_str("23456789012E66")?,
Static(F64(2.345_678_901_2e76))
Static(StaticNode::from(2.345_678_901_2e76))
);
assert_eq!(
to_value_from_str("0.0000000000000000000000000000000000000000000000000123e50")
.expect("1.23"),
Static(F64(1.23))
Static(StaticNode::from(1.23))
);
assert_eq!(
to_value_from_str("0.6").expect("0.6"),
Static(StaticNode::from(0.6))
);
assert_eq!(to_value_from_str("0.6").expect("0.6"), Static(F64(0.6)));
Ok(())
}

Expand Down Expand Up @@ -537,10 +541,16 @@ mod test {

#[test]
fn zero_float() -> Result<(), crate::Error> {
assert_eq!(to_value_from_str("0e1")?, Static(F64(0.0)));
assert_eq!(to_value_from_str("0.00e-00")?, Static(F64(0.0)));
assert_eq!(to_value_from_str("0e-1")?, Static(F64(-0.0)));
assert_eq!(to_value_from_str("-0.00e-00")?, Static(F64(-0.0)));
assert_eq!(to_value_from_str("0e1")?, Static(StaticNode::from(0.0)));
assert_eq!(
to_value_from_str("0.00e-00")?,
Static(StaticNode::from(0.0))
);
assert_eq!(to_value_from_str("0e-1")?, Static(StaticNode::from(-0.0)));
assert_eq!(
to_value_from_str("-0.00e-00")?,
Static(StaticNode::from(-0.0))
);
Ok(())
}

Expand All @@ -556,14 +566,14 @@ mod test {
fn minus_309() -> Result<(), crate::Error> {
assert_eq!(
to_value_from_str("-5.96916642387374e-309")?,
Static(F64(-5.969_166_423_873_74e-_309))
Static(StaticNode::from(-5.969_166_423_873_74e-_309))
);
Ok(())
}
#[allow(clippy::unreadable_literal)]
#[test]
fn tiny_float() -> Result<(), crate::Error> {
assert_eq!(to_value_from_str("-0.00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000596916642387374")?, Static(F64(-0.00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000596916642387374)));
assert_eq!(to_value_from_str("-0.00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000596916642387374")?, Static(StaticNode::from(-0.00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000596916642387374)));
Ok(())
}

Expand All @@ -572,7 +582,7 @@ mod test {
fn huge_int() -> Result<(), crate::Error> {
assert_eq!(
to_value_from_str("999999999999999999999999999999")?,
Static(F64(999999999999999999999999999999f64))
StaticNode::from(999999999999999999999999999999f64)
);
Ok(())
}
Expand Down
15 changes: 9 additions & 6 deletions src/serde.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ pub unsafe fn from_str<'a, T>(s: &'a mut str) -> Result<T>
where
T: Deserialize<'a>,
{
let mut deserializer = stry!(Deserializer::from_slice(s.as_bytes_mut()));
let mut deserializer = stry!(Deserializer::from_slice(unsafe { s.as_bytes_mut() }));

T::deserialize(&mut deserializer)
}
Expand Down Expand Up @@ -126,7 +126,7 @@ where
T: Deserialize<'a>,
{
let mut deserializer = stry!(Deserializer::from_slice_with_buffers(
s.as_bytes_mut(),
unsafe { s.as_bytes_mut() },
buffers
));

Expand Down Expand Up @@ -323,7 +323,8 @@ impl<'de> Deserializer<'de> {
#[allow(clippy::cast_possible_wrap, clippy::cast_precision_loss)]
fn parse_double(&mut self) -> Result<f64> {
match unsafe { self.next_() } {
Node::Static(StaticNode::F64(n)) => Ok(n),
#[allow(clippy::useless_conversion)] // .into() required by ordered-float
Node::Static(StaticNode::F64(n)) => Ok(n.into()),
Node::Static(StaticNode::I64(n)) => Ok(n as f64),
Node::Static(StaticNode::U64(n)) => Ok(n as f64),
_ => Err(Self::error(ErrorType::ExpectedFloat)),
Expand All @@ -344,7 +345,7 @@ impl TryFrom<serde_json::Value> for OwnedValue {
} else if let Some(n) = b.as_u64() {
Self::Static(StaticNode::U64(n))
} else if let Some(n) = b.as_f64() {
Self::Static(StaticNode::F64(n))
Self::Static(StaticNode::from(n))
} else {
return Err(SerdeConversionError::Oops);
}
Expand Down Expand Up @@ -384,7 +385,8 @@ impl TryInto<serde_json::Value> for OwnedValue {
.into(),
),
Self::Static(StaticNode::F64(n)) => {
if let Some(n) = serde_json::Number::from_f64(n) {
#[allow(clippy::useless_conversion)] // .into() required by ordered-float
if let Some(n) = serde_json::Number::from_f64(n.into()) {
Value::Number(n)
} else {
return Err(SerdeConversionError::NanOrInfinity);
Expand Down Expand Up @@ -450,7 +452,8 @@ impl<'value> TryInto<serde_json::Value> for BorrowedValue<'value> {
.into(),
),
BorrowedValue::Static(StaticNode::F64(n)) => {
if let Some(n) = serde_json::Number::from_f64(n) {
#[allow(clippy::useless_conversion)] // .into() required by ordered-float
if let Some(n) = serde_json::Number::from_f64(n.into()) {
Value::Number(n)
} else {
return Err(SerdeConversionError::NanOrInfinity);
Expand Down
3 changes: 2 additions & 1 deletion src/serde/de.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ where
Node::String(s) => visitor.visit_borrowed_str(s),
Node::Static(StaticNode::Null) => visitor.visit_unit(),
Node::Static(StaticNode::Bool(b)) => visitor.visit_bool(b),
Node::Static(StaticNode::F64(n)) => visitor.visit_f64(n),
#[allow(clippy::useless_conversion)] // .into() required by ordered-float
Node::Static(StaticNode::F64(n)) => visitor.visit_f64(n.into()),
Node::Static(StaticNode::I64(n)) => visitor.visit_i64(n),
#[cfg(feature = "128bit")]
Node::Static(StaticNode::I128(n)) => visitor.visit_i128(n),
Expand Down
10 changes: 6 additions & 4 deletions src/serde/value/borrowed/de.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ impl<'de> de::Deserializer<'de> for Value<'de> {
Value::Static(StaticNode::U64(n)) => visitor.visit_u64(n),
#[cfg(feature = "128bit")]
Value::Static(StaticNode::U128(n)) => visitor.visit_u128(n),
Value::Static(StaticNode::F64(n)) => visitor.visit_f64(n),
#[allow(clippy::useless_conversion)] // .into() required by ordered-float
Value::Static(StaticNode::F64(n)) => visitor.visit_f64(n.into()),
#[cfg(feature = "beef")]
Value::String(s) => {
if s.is_borrowed() {
Expand Down Expand Up @@ -400,15 +401,15 @@ impl<'de> Visitor<'de> for ValueVisitor {
where
E: de::Error,
{
Ok(Value::Static(StaticNode::F64(f64::from(value))))
Ok(Value::Static(StaticNode::from(f64::from(value))))
}

#[cfg_attr(not(feature = "no-inline"), inline)]
fn visit_f64<E>(self, value: f64) -> Result<Self::Value, E>
where
E: de::Error,
{
Ok(Value::Static(StaticNode::F64(value)))
Ok(Value::Static(StaticNode::from(value)))
}

/****************** stringy stuff ******************/
Expand Down Expand Up @@ -621,7 +622,8 @@ impl<'de> de::Deserializer<'de> for &'de Value<'de> {
Value::Static(StaticNode::U64(n)) => visitor.visit_u64(*n),
#[cfg(feature = "128bit")]
Value::Static(StaticNode::U128(n)) => visitor.visit_u128(*n),
Value::Static(StaticNode::F64(n)) => visitor.visit_f64(*n),
#[allow(clippy::useless_conversion)] // .into() required by ordered-float
Value::Static(StaticNode::F64(n)) => visitor.visit_f64((*n).into()),
Value::String(s) => visitor.visit_borrowed_str(s),
Value::Array(a) => visitor.visit_seq(ArrayRef(a.as_slice().iter())),
Value::Object(o) => visitor.visit_map(ObjectRefAccess::new(o.iter())),
Expand Down
7 changes: 4 additions & 3 deletions src/serde/value/borrowed/se.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ impl<'value> Serialize for Value<'value> {
match self {
Value::Static(StaticNode::Null) => serializer.serialize_unit(),
Value::Static(StaticNode::Bool(b)) => serializer.serialize_bool(*b),
Value::Static(StaticNode::F64(f)) => serializer.serialize_f64(*f),
#[allow(clippy::useless_conversion)] // .into() required by ordered-float
Value::Static(StaticNode::F64(f)) => serializer.serialize_f64((*f).into()),
Value::Static(StaticNode::U64(i)) => serializer.serialize_u64(*i),
#[cfg(feature = "128bit")]
Value::Static(StaticNode::U128(i)) => serializer.serialize_u128(*i),
Expand Down Expand Up @@ -133,7 +134,7 @@ impl<'se> serde::Serializer for Serializer<'se> {

#[cfg_attr(not(feature = "no-inline"), inline)]
fn serialize_f64(self, value: f64) -> Result<Value<'se>> {
Ok(Value::Static(StaticNode::F64(value)))
Ok(Value::Static(StaticNode::from(value)))
}

#[cfg_attr(not(feature = "no-inline"), inline)]
Expand Down Expand Up @@ -629,7 +630,7 @@ mod test {

#[test]
fn float() {
let v = Value::Static(StaticNode::F64(1.0));
let v = Value::Static(StaticNode::from(1.0));
let s = serde_json::to_string(&v).expect("Failed to serialize");
assert_eq!(s, "1.0");
}
Expand Down
10 changes: 6 additions & 4 deletions src/serde/value/owned/de.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ impl<'de> de::Deserializer<'de> for Value {
Value::Static(StaticNode::U64(n)) => visitor.visit_u64(n),
#[cfg(feature = "128bit")]
Value::Static(StaticNode::U128(n)) => visitor.visit_u128(n),
Value::Static(StaticNode::F64(n)) => visitor.visit_f64(n),
#[allow(clippy::useless_conversion)] // .into() required by ordered-float
Value::Static(StaticNode::F64(n)) => visitor.visit_f64(n.into()),
Value::String(s) => visitor.visit_string(s),
Value::Array(a) => visitor.visit_seq(Array(a.into_iter())),
Value::Object(o) => visitor.visit_map(ObjectAccess {
Expand Down Expand Up @@ -386,15 +387,15 @@ impl<'de> Visitor<'de> for ValueVisitor {
where
E: de::Error,
{
Ok(Value::Static(StaticNode::F64(f64::from(value))))
Ok(Value::Static(StaticNode::from(f64::from(value))))
}

#[cfg_attr(not(feature = "no-inline"), inline)]
fn visit_f64<E>(self, value: f64) -> Result<Self::Value, E>
where
E: de::Error,
{
Ok(Value::Static(StaticNode::F64(value)))
Ok(Value::Static(StaticNode::from(value)))
}

/****************** stringy stuff ******************/
Expand Down Expand Up @@ -608,7 +609,8 @@ impl<'de> de::Deserializer<'de> for &'de Value {
Value::Static(StaticNode::U64(n)) => visitor.visit_u64(*n),
#[cfg(feature = "128bit")]
Value::Static(StaticNode::U128(n)) => visitor.visit_u128(*n),
Value::Static(StaticNode::F64(n)) => visitor.visit_f64(*n),
#[allow(clippy::useless_conversion)] // .into() required by ordered-float
Value::Static(StaticNode::F64(n)) => visitor.visit_f64((*n).into()),
Value::String(s) => visitor.visit_borrowed_str(s),
Value::Array(a) => visitor.visit_seq(ArrayRef(a.as_slice().iter())),
Value::Object(o) => visitor.visit_map(ObjectRefAccess::new(o.iter())),
Expand Down
5 changes: 3 additions & 2 deletions src/serde/value/owned/se.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ impl Serialize for Value {
match self {
Value::Static(StaticNode::Null) => serializer.serialize_unit(),
Value::Static(StaticNode::Bool(b)) => serializer.serialize_bool(*b),
Value::Static(StaticNode::F64(f)) => serializer.serialize_f64(*f),
#[allow(clippy::useless_conversion)] // .into() required by ordered-float
Value::Static(StaticNode::F64(f)) => serializer.serialize_f64((*f).into()),
Value::Static(StaticNode::U64(i)) => serializer.serialize_u64(*i),
#[cfg(feature = "128bit")]
Value::Static(StaticNode::U128(i)) => serializer.serialize_u128(*i),
Expand Down Expand Up @@ -120,7 +121,7 @@ impl serde::Serializer for Serializer {

#[cfg_attr(not(feature = "no-inline"), inline)]
fn serialize_f64(self, value: f64) -> Result<Value> {
Ok(Value::Static(StaticNode::F64(value)))
Ok(Value::Static(StaticNode::from(value)))
}

#[cfg_attr(not(feature = "no-inline"), inline)]
Expand Down
Loading
Loading