Skip to content

Commit

Permalink
Lossless Value::Number and allow +unsigned (#479)
Browse files Browse the repository at this point in the history 
* First steps towards a lossless Value::Number

* Allow parsing +unsigned as unsigned int

* Add additional tests for number parsing

* Added CHANGELOG entry

* Improve coverage by running tests across all features

* Refactor number parsing for better readability

* Extend number tests to typed ser+de

* Adjust #465 tests to lossless Value::Number
  • Loading branch information
@juntyr
juntyr committed Aug 20, 2023
1 parent dea68fe commit b7acecb
Show file tree
Hide file tree
Showing 19 changed files with 754 additions and 606 deletions.
3 changes: 3 additions & 0 deletions .github/workflows/ci.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ jobs:
env:
CARGO_TARGET_DIR: target/
- run: |
cargo test --all-targets
cargo test --features integer128 --all-targets
cargo test --features indexmap --all-targets
cargo test --all-features --all-targets
grcov . -s . --binary-path ./target/debug/ \
-t cobertura -o cobertura.xml --branch \
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fix deserialising deserialising `A('/')` into a `ron::Value` ([#465](https://github.com/ron-rs/ron/pull/465))
- Update the arbitrary fuzzer to check arbitrary serde data types, values, and `ron::ser::PrettyConfig`s ([#465](https://github.com/ron-rs/ron/pull/465))
- Add a benchmark for PRs that runs over the latest fuzzer corpus ([#465](https://github.com/ron-rs/ron/pull/465))
- Fix issue [#445](https://github.com/ron-rs/ron/issues/445) and allow parsing `+unsigned` as an unsigned int ([#479](https://github.com/ron-rs/ron/pull/479))
- Breaking: Expand the `value::Number` enum to explicitly encode all possible number types ([#479](https://github.com/ron-rs/ron/pull/479))

## [0.8.1] - 2023-08-17

Expand Down
4 changes: 2 additions & 2 deletions docs/grammar.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ For the extension names see the [`extensions.md`][exts] document.
## Value

```ebnf
value = unsigned | signed | float | string | char | bool | option | list | map | tuple | struct | enum_variant;
value = integer | float | string | char | bool | option | list | map | tuple | struct | enum_variant;
```

## Numbers
Expand All @@ -50,7 +50,7 @@ digit = "0" | "1" | "2" | "3" | "4" | "5" | "6" | "7" | "8" | "9";
hex_digit = "A" | "a" | "B" | "b" | "C" | "c" | "D" | "d" | "E" | "e" | "F" | "f";
unsigned = (["0", ("b" | "o")], digit, { digit | '_' } |
"0x", (digit | hex_digit), { digit | hex_digit | '_' });
signed = ["+" | "-"], unsigned;
integer = ["+" | "-"], unsigned;
float = ["+" | "-"], ("inf" | "NaN" | float_num);
float_num = (float_int | float_std | float_frac), [float_exp];
float_int = digit, { digit };
Expand Down
30 changes: 4 additions & 26 deletions src/de/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::{
error::{Result, SpannedResult},
extensions::Extensions,
options::Options,
parse::{AnyNum, Bytes, NewtypeMode, ParsedStr, StructType, TupleMode, BASE64_ENGINE},
parse::{Bytes, NewtypeMode, ParsedStr, StructType, TupleMode, BASE64_ENGINE},
};

mod id;
Expand Down Expand Up @@ -294,11 +294,9 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
} else if self.bytes.consume("()") {
return visitor.visit_unit();
} else if self.bytes.consume_ident("inf") {
return visitor.visit_f64(std::f64::INFINITY);
} else if self.bytes.consume_ident("-inf") {
return visitor.visit_f64(std::f64::NEG_INFINITY);
return visitor.visit_f32(std::f32::INFINITY);
} else if self.bytes.consume_ident("NaN") {
return visitor.visit_f64(std::f64::NAN);
return visitor.visit_f32(std::f32::NAN);
}

// `identifier` does not change state if it fails
Expand All @@ -314,27 +312,7 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
b'(' => self.handle_any_struct(visitor, None),
b'[' => self.deserialize_seq(visitor),
b'{' => self.deserialize_map(visitor),
b'0'..=b'9' | b'+' | b'-' => {
let any_num: AnyNum = self.bytes.any_num()?;

match any_num {
AnyNum::F32(x) => visitor.visit_f32(x),
AnyNum::F64(x) => visitor.visit_f64(x),
AnyNum::I8(x) => visitor.visit_i8(x),
AnyNum::U8(x) => visitor.visit_u8(x),
AnyNum::I16(x) => visitor.visit_i16(x),
AnyNum::U16(x) => visitor.visit_u16(x),
AnyNum::I32(x) => visitor.visit_i32(x),
AnyNum::U32(x) => visitor.visit_u32(x),
AnyNum::I64(x) => visitor.visit_i64(x),
AnyNum::U64(x) => visitor.visit_u64(x),
#[cfg(feature = "integer128")]
AnyNum::I128(x) => visitor.visit_i128(x),
#[cfg(feature = "integer128")]
AnyNum::U128(x) => visitor.visit_u128(x),
}
}
b'.' => self.deserialize_f64(visitor),
b'0'..=b'9' | b'+' | b'-' | b'.' => self.bytes.any_number()?.visit(visitor),
b'"' | b'r' => self.deserialize_string(visitor),
b'\'' => self.deserialize_char(visitor),
other => Err(Error::UnexpectedByte(other as char)),
Expand Down
88 changes: 78 additions & 10 deletions src/de/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ use serde_derive::Deserialize;
use crate::{
de::from_str,
error::{Error, Position, SpannedError, SpannedResult},
parse::{AnyNum, Bytes},
parse::Bytes,
value::Number,
};

#[derive(Debug, PartialEq, Deserialize)]
Expand Down Expand Up @@ -345,19 +346,86 @@ fn test_numbers() {
);
}

fn de_any_number(s: &str) -> AnyNum {
fn check_de_any_number<
T: Copy + PartialEq + std::fmt::Debug + Into<Number> + serde::de::DeserializeOwned,
>(
s: &str,
cmp: T,
) {
let mut bytes = Bytes::new(s.as_bytes()).unwrap();
let number = bytes.any_number().unwrap();

bytes.any_num().unwrap()
assert_eq!(number, Number::new(cmp));
assert_eq!(
Number::new(super::from_str::<T>(s).unwrap()),
Number::new(cmp)
);
}

#[test]
fn test_any_number_precision() {
assert_eq!(de_any_number("1"), AnyNum::U8(1));
assert_eq!(de_any_number("+1"), AnyNum::I8(1));
assert_eq!(de_any_number("-1"), AnyNum::I8(-1));
assert_eq!(de_any_number("-1.0"), AnyNum::F32(-1.0));
assert_eq!(de_any_number("1."), AnyNum::F32(1.));
assert_eq!(de_any_number("-1."), AnyNum::F32(-1.));
assert_eq!(de_any_number("0.3"), AnyNum::F64(0.3));
check_de_any_number("1", 1_u8);
check_de_any_number("+1", 1_u8);
check_de_any_number("-1", -1_i8);
check_de_any_number("-1.0", -1.0_f32);
check_de_any_number("1.", 1.0_f32);
check_de_any_number("-1.", -1.0_f32);
check_de_any_number(".3", 0.3_f64);
check_de_any_number("-.3", -0.3_f64);
check_de_any_number("+.3", 0.3_f64);
check_de_any_number("0.3", 0.3_f64);
check_de_any_number("NaN", f32::NAN);
check_de_any_number("-NaN", -f32::NAN);
check_de_any_number("inf", f32::INFINITY);
check_de_any_number("-inf", f32::NEG_INFINITY);

macro_rules! test_min {
($($ty:ty),*) => {
$(check_de_any_number(&format!("{}", <$ty>::MIN), <$ty>::MIN);)*
};
}

macro_rules! test_max {
($($ty:ty),*) => {
$(check_de_any_number(&format!("{}", <$ty>::MAX), <$ty>::MAX);)*
};
}

test_min! { i8, i16, i32, i64, f64 }
test_max! { u8, u16, u32, u64, f64 }
#[cfg(feature = "integer128")]
test_min! { i128 }
#[cfg(feature = "integer128")]
test_max! { u128 }
}

#[test]
fn test_value_special_floats() {
use crate::{from_str, value::Number, Value};

assert_eq!(
from_str("NaN"),
Ok(Value::Number(Number::F32(f32::NAN.into())))
);
assert_eq!(
from_str("+NaN"),
Ok(Value::Number(Number::F32(f32::NAN.into())))
);
assert_eq!(
from_str("-NaN"),
Ok(Value::Number(Number::F32((-f32::NAN).into())))
);

assert_eq!(
from_str("inf"),
Ok(Value::Number(Number::F32(f32::INFINITY.into())))
);
assert_eq!(
from_str("+inf"),
Ok(Value::Number(Number::F32(f32::INFINITY.into())))
);
assert_eq!(
from_str("-inf"),
Ok(Value::Number(Number::F32(f32::NEG_INFINITY.into())))
);
}
85 changes: 67 additions & 18 deletions src/de/value.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,27 @@ impl<'de> Visitor<'de> for ValueVisitor {
Ok(Value::Bool(v))
}

fn visit_i8<E>(self, v: i8) -> Result<Self::Value, E>
where
E: Error,
{
Ok(Value::Number(Number::new(v)))
}

fn visit_i16<E>(self, v: i16) -> Result<Self::Value, E>
where
E: Error,
{
Ok(Value::Number(Number::new(v)))
}

fn visit_i32<E>(self, v: i32) -> Result<Self::Value, E>
where
E: Error,
{
Ok(Value::Number(Number::new(v)))
}

fn visit_i64<E>(self, v: i64) -> Result<Self::Value, E>
where
E: Error,
Expand All @@ -61,7 +82,28 @@ impl<'de> Visitor<'de> for ValueVisitor {
where
E: Error,
{
self.visit_f64(v as f64)
Ok(Value::Number(Number::new(v)))
}

fn visit_u8<E>(self, v: u8) -> Result<Self::Value, E>
where
E: Error,
{
Ok(Value::Number(Number::new(v)))
}

fn visit_u16<E>(self, v: u16) -> Result<Self::Value, E>
where
E: Error,
{
Ok(Value::Number(Number::new(v)))
}

fn visit_u32<E>(self, v: u32) -> Result<Self::Value, E>
where
E: Error,
{
Ok(Value::Number(Number::new(v)))
}

fn visit_u64<E>(self, v: u64) -> Result<Self::Value, E>
Expand All @@ -76,7 +118,14 @@ impl<'de> Visitor<'de> for ValueVisitor {
where
E: Error,
{
self.visit_f64(v as f64)
Ok(Value::Number(Number::new(v)))
}

fn visit_f32<E>(self, v: f32) -> Result<Self::Value, E>
where
E: Error,
{
Ok(Value::Number(Number::new(v)))
}

fn visit_f64<E>(self, v: f64) -> Result<Self::Value, E>
Expand Down Expand Up @@ -210,9 +259,9 @@ mod tests {
assert_eq!(
eval("(3, 4.0, 5.0)"),
Value::Seq(vec![
Value::Number(Number::new(3)),
Value::Number(Number::new(4.0)),
Value::Number(Number::new(5.0)),
Value::Number(Number::U8(3)),
Value::Number(Number::F32(4.0.into())),
Value::Number(Number::F32(5.0.into())),
],),
);
}
Expand All @@ -223,9 +272,9 @@ mod tests {
eval("(true, 3, 4, 5.0)"),
Value::Seq(vec![
Value::Bool(true),
Value::Number(Number::new(3)),
Value::Number(Number::new(4)),
Value::Number(Number::new(5.0)),
Value::Number(Number::U8(3)),
Value::Number(Number::U8(4)),
Value::Number(Number::F32(5.0.into())),
]),
);
}
Expand All @@ -248,9 +297,9 @@ mod tests {
assert_eq!(
eval("(inf, -inf, NaN)"),
Value::Seq(vec![
Value::Number(Number::new(std::f64::INFINITY)),
Value::Number(Number::new(std::f64::NEG_INFINITY)),
Value::Number(Number::new(std::f64::NAN)),
Value::Number(Number::new(std::f32::INFINITY)),
Value::Number(Number::new(std::f32::NEG_INFINITY)),
Value::Number(Number::new(std::f32::NAN)),
]),
);
}
Expand Down Expand Up @@ -279,11 +328,11 @@ mod tests {
vec![
(
Value::String("width".to_owned()),
Value::Number(Number::new(20)),
Value::Number(Number::U8(20)),
),
(
Value::String("height".to_owned()),
Value::Number(Number::new(5)),
Value::Number(Number::U8(5)),
),
(
Value::String("name".to_owned()),
Expand All @@ -297,11 +346,11 @@ mod tests {
vec![
(
Value::String("width".to_owned()),
Value::Number(Number::new(10.0)),
Value::Number(Number::F32(10.0.into())),
),
(
Value::String("height".to_owned()),
Value::Number(Number::new(10.0)),
Value::Number(Number::F32(10.0.into())),
),
(
Value::String("name".to_owned()),
Expand All @@ -313,15 +362,15 @@ mod tests {
vec![
(
Value::String("Enemy1".to_owned()),
Value::Number(Number::new(3)),
Value::Number(Number::U8(3)),
),
(
Value::String("Enemy2".to_owned()),
Value::Number(Number::new(5)),
Value::Number(Number::U8(5)),
),
(
Value::String("Enemy3".to_owned()),
Value::Number(Number::new(7)),
Value::Number(Number::U8(7)),
),
]
.into_iter()
Expand Down
Loading

0 comments on commit b7acecb

Please sign in to comment.