Skip to content

Lossless Value::Number and allow +unsigned #479

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

Merged
merged 10 commits into from
Aug 20, 2023
Merged

Lossless Value::Number and allow +unsigned #479

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9d8f34e
First steps towards a lossless Value::Number
juntyr Aug 19, 2023
ad062a2
Complete impl and fix tests
juntyr Aug 19, 2023
37bc4a2
Allow parsing +unsigned as unsigned int
juntyr Aug 19, 2023
7768752
Add additional tests for number parsing
juntyr Aug 19, 2023
8313585
Added CHANGELOG entry
juntyr Aug 19, 2023
f7cf3c2
Small code cleanup
juntyr Aug 19, 2023
070ff5b
Improve coverage by running tests across all features
juntyr Aug 19, 2023
6259c50
Refactor number parsing for better readability
juntyr Aug 20, 2023
22797ac
Extend number tests to typed ser+de
juntyr Aug 20, 2023
c3af695
Adjust #465 tests to lossless Value::Number
juntyr Aug 20, 2023
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
3 changes: 3 additions & 0 deletions .github/workflows/ci.yaml
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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