From 2a4cb44f7cc591a4542cbb1a73ca5bdc8a02c0d7 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 19 Oct 2024 09:21:55 -0700 Subject: [PATCH 1/3] Rearrange 'match peek' --- src/de.rs | 60 ++++++++++++++++++++++++------------------------------- 1 file changed, 26 insertions(+), 34 deletions(-) diff --git a/src/de.rs b/src/de.rs index d6cb502da..88ca3e66c 100644 --- a/src/de.rs +++ b/src/de.rs @@ -1929,31 +1929,25 @@ impl<'de, 'a, R: Read<'de> + 'a> de::SeqAccess<'de> for SeqAccess<'a, R> { fn has_next_element<'de, 'a, R: Read<'de> + 'a>( seq: &mut SeqAccess<'a, R>, ) -> Result { - let peek = match tri!(seq.de.parse_whitespace()) { - Some(b']') => { - return Ok(false); - } + match tri!(seq.de.parse_whitespace()) { + Some(b']') => Ok(false), Some(b',') if !seq.first => { seq.de.eat_char(); - tri!(seq.de.parse_whitespace()) + match tri!(seq.de.parse_whitespace()) { + Some(b']') => Err(seq.de.peek_error(ErrorCode::TrailingComma)), + Some(_) => Ok(true), + None => Err(seq.de.peek_error(ErrorCode::EofWhileParsingValue)), + } } - Some(b) => { + Some(_) => { if seq.first { seq.first = false; - Some(b) + Ok(true) } else { - return Err(seq.de.peek_error(ErrorCode::ExpectedListCommaOrEnd)); + Err(seq.de.peek_error(ErrorCode::ExpectedListCommaOrEnd)) } } - None => { - return Err(seq.de.peek_error(ErrorCode::EofWhileParsingList)); - } - }; - - match peek { - Some(b']') => Err(seq.de.peek_error(ErrorCode::TrailingComma)), - Some(_) => Ok(true), - None => Err(seq.de.peek_error(ErrorCode::EofWhileParsingValue)), + None => Err(seq.de.peek_error(ErrorCode::EofWhileParsingList)), } } @@ -1984,32 +1978,30 @@ impl<'de, 'a, R: Read<'de> + 'a> de::MapAccess<'de> for MapAccess<'a, R> { K: de::DeserializeSeed<'de>, { fn has_next_key<'de, 'a, R: Read<'de> + 'a>(map: &mut MapAccess<'a, R>) -> Result { - let peek = match tri!(map.de.parse_whitespace()) { - Some(b'}') => { - return Ok(false); - } + match tri!(map.de.parse_whitespace()) { + Some(b'}') => Ok(false), Some(b',') if !map.first => { map.de.eat_char(); - tri!(map.de.parse_whitespace()) + match tri!(map.de.parse_whitespace()) { + Some(b'"') => Ok(true), + Some(b'}') => Err(map.de.peek_error(ErrorCode::TrailingComma)), + Some(_) => Err(map.de.peek_error(ErrorCode::KeyMustBeAString)), + None => Err(map.de.peek_error(ErrorCode::EofWhileParsingValue)), + } } Some(b) => { if map.first { map.first = false; - Some(b) + if b == b'"' { + Ok(true) + } else { + Err(map.de.peek_error(ErrorCode::KeyMustBeAString)) + } } else { - return Err(map.de.peek_error(ErrorCode::ExpectedObjectCommaOrEnd)); + Err(map.de.peek_error(ErrorCode::ExpectedObjectCommaOrEnd)) } } - None => { - return Err(map.de.peek_error(ErrorCode::EofWhileParsingObject)); - } - }; - - match peek { - Some(b'"') => Ok(true), - Some(b'}') => Err(map.de.peek_error(ErrorCode::TrailingComma)), - Some(_) => Err(map.de.peek_error(ErrorCode::KeyMustBeAString)), - None => Err(map.de.peek_error(ErrorCode::EofWhileParsingValue)), + None => Err(map.de.peek_error(ErrorCode::EofWhileParsingObject)), } } From 0f54a1a0df5045aee4a2d2f8656c365d835095e5 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 19 Oct 2024 09:22:33 -0700 Subject: [PATCH 2/3] Handle early return sooner on eof in seq or map This matches how peeking is done within deserialize_any and other Deserializer methods --- src/de.rs | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/src/de.rs b/src/de.rs index 88ca3e66c..a22fcf0de 100644 --- a/src/de.rs +++ b/src/de.rs @@ -1929,9 +1929,16 @@ impl<'de, 'a, R: Read<'de> + 'a> de::SeqAccess<'de> for SeqAccess<'a, R> { fn has_next_element<'de, 'a, R: Read<'de> + 'a>( seq: &mut SeqAccess<'a, R>, ) -> Result { - match tri!(seq.de.parse_whitespace()) { - Some(b']') => Ok(false), - Some(b',') if !seq.first => { + let peek = match tri!(seq.de.parse_whitespace()) { + Some(b) => b, + None => { + return Err(seq.de.peek_error(ErrorCode::EofWhileParsingList)); + } + }; + + match peek { + b']' => Ok(false), + b',' if !seq.first => { seq.de.eat_char(); match tri!(seq.de.parse_whitespace()) { Some(b']') => Err(seq.de.peek_error(ErrorCode::TrailingComma)), @@ -1939,7 +1946,7 @@ impl<'de, 'a, R: Read<'de> + 'a> de::SeqAccess<'de> for SeqAccess<'a, R> { None => Err(seq.de.peek_error(ErrorCode::EofWhileParsingValue)), } } - Some(_) => { + _ => { if seq.first { seq.first = false; Ok(true) @@ -1947,7 +1954,6 @@ impl<'de, 'a, R: Read<'de> + 'a> de::SeqAccess<'de> for SeqAccess<'a, R> { Err(seq.de.peek_error(ErrorCode::ExpectedListCommaOrEnd)) } } - None => Err(seq.de.peek_error(ErrorCode::EofWhileParsingList)), } } @@ -1978,9 +1984,16 @@ impl<'de, 'a, R: Read<'de> + 'a> de::MapAccess<'de> for MapAccess<'a, R> { K: de::DeserializeSeed<'de>, { fn has_next_key<'de, 'a, R: Read<'de> + 'a>(map: &mut MapAccess<'a, R>) -> Result { - match tri!(map.de.parse_whitespace()) { - Some(b'}') => Ok(false), - Some(b',') if !map.first => { + let peek = match tri!(map.de.parse_whitespace()) { + Some(b) => b, + None => { + return Err(map.de.peek_error(ErrorCode::EofWhileParsingObject)); + } + }; + + match peek { + b'}' => Ok(false), + b',' if !map.first => { map.de.eat_char(); match tri!(map.de.parse_whitespace()) { Some(b'"') => Ok(true), @@ -1989,10 +2002,10 @@ impl<'de, 'a, R: Read<'de> + 'a> de::MapAccess<'de> for MapAccess<'a, R> { None => Err(map.de.peek_error(ErrorCode::EofWhileParsingValue)), } } - Some(b) => { + _ => { if map.first { map.first = false; - if b == b'"' { + if peek == b'"' { Ok(true) } else { Err(map.de.peek_error(ErrorCode::KeyMustBeAString)) @@ -2001,7 +2014,6 @@ impl<'de, 'a, R: Read<'de> + 'a> de::MapAccess<'de> for MapAccess<'a, R> { Err(map.de.peek_error(ErrorCode::ExpectedObjectCommaOrEnd)) } } - None => Err(map.de.peek_error(ErrorCode::EofWhileParsingObject)), } } From f2082d2a04b3b5d72503ac89e2182a5833bb2a1e Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 19 Oct 2024 09:23:00 -0700 Subject: [PATCH 3/3] Clearer order of comparisons We look at seq.first/map.first only once. --- src/de.rs | 68 ++++++++++++++++++++++++------------------------------- 1 file changed, 30 insertions(+), 38 deletions(-) diff --git a/src/de.rs b/src/de.rs index a22fcf0de..5b64138d8 100644 --- a/src/de.rs +++ b/src/de.rs @@ -1936,24 +1936,20 @@ impl<'de, 'a, R: Read<'de> + 'a> de::SeqAccess<'de> for SeqAccess<'a, R> { } }; - match peek { - b']' => Ok(false), - b',' if !seq.first => { - seq.de.eat_char(); - match tri!(seq.de.parse_whitespace()) { - Some(b']') => Err(seq.de.peek_error(ErrorCode::TrailingComma)), - Some(_) => Ok(true), - None => Err(seq.de.peek_error(ErrorCode::EofWhileParsingValue)), - } - } - _ => { - if seq.first { - seq.first = false; - Ok(true) - } else { - Err(seq.de.peek_error(ErrorCode::ExpectedListCommaOrEnd)) - } + if peek == b']' { + Ok(false) + } else if seq.first { + seq.first = false; + Ok(true) + } else if peek == b',' { + seq.de.eat_char(); + match tri!(seq.de.parse_whitespace()) { + Some(b']') => Err(seq.de.peek_error(ErrorCode::TrailingComma)), + Some(_) => Ok(true), + None => Err(seq.de.peek_error(ErrorCode::EofWhileParsingValue)), } + } else { + Err(seq.de.peek_error(ErrorCode::ExpectedListCommaOrEnd)) } } @@ -1991,29 +1987,25 @@ impl<'de, 'a, R: Read<'de> + 'a> de::MapAccess<'de> for MapAccess<'a, R> { } }; - match peek { - b'}' => Ok(false), - b',' if !map.first => { - map.de.eat_char(); - match tri!(map.de.parse_whitespace()) { - Some(b'"') => Ok(true), - Some(b'}') => Err(map.de.peek_error(ErrorCode::TrailingComma)), - Some(_) => Err(map.de.peek_error(ErrorCode::KeyMustBeAString)), - None => Err(map.de.peek_error(ErrorCode::EofWhileParsingValue)), - } + if peek == b'}' { + Ok(false) + } else if map.first { + map.first = false; + if peek == b'"' { + Ok(true) + } else { + Err(map.de.peek_error(ErrorCode::KeyMustBeAString)) } - _ => { - if map.first { - map.first = false; - if peek == b'"' { - Ok(true) - } else { - Err(map.de.peek_error(ErrorCode::KeyMustBeAString)) - } - } else { - Err(map.de.peek_error(ErrorCode::ExpectedObjectCommaOrEnd)) - } + } else if peek == b',' { + map.de.eat_char(); + match tri!(map.de.parse_whitespace()) { + Some(b'"') => Ok(true), + Some(b'}') => Err(map.de.peek_error(ErrorCode::TrailingComma)), + Some(_) => Err(map.de.peek_error(ErrorCode::KeyMustBeAString)), + None => Err(map.de.peek_error(ErrorCode::EofWhileParsingValue)), } + } else { + Err(map.de.peek_error(ErrorCode::ExpectedObjectCommaOrEnd)) } }