Skip to content

Round-trip compact ranges containing inf/NaN - #605

Merged
juntyr merged 6 commits into
ron-rs:masterfrom
mahirhir:fix/range-inf-nan-start-bound
Sep 6, 2026
Merged

Round-trip compact ranges containing inf/NaN#605
juntyr merged 6 commits into
ron-rs:masterfrom
mahirhir:fix/range-inf-nan-start-bound

Conversation

@mahirhir

@mahirhir mahirhir commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

The compact-range serializer emits inf..2.0 / NaN..=2.0, but the deserializer rejects its own output, so a round-trip fails. PR #602 added float handling to RangeFrom but left Range and RangeInclusive unmirrored. The ~8-line fix applies the same handling to the two siblings. Round-trip verified both ways.

@mahirhir
fix(de): accept inf/NaN start bound in compact Range/RangeInclusive
5150ebe 
The compact-range serializer emits non-finite start bounds such as
`inf..2.0` and `NaN..=2.0`, but the `Range`/`RangeInclusive`
deserializers only took the compact-range path when the first byte was
recognised by `is_number_start` (digit / + / - / . / b). Because `inf`
and `NaN` start with a letter, those inputs failed to deserialize with
`ExpectedNamedStructLike`, breaking serialize -> deserialize round-trips.
The sibling `RangeFrom` branch already had the inf/NaN ident checks; this
applies the same checks to `Range`/`RangeInclusive`.
juntyr
juntyr reviewed Jun 26, 2026
View reviewed changes

@juntyr juntyr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @greymoth-jp for catching the issue and writing the fix!

Comment thread src/de/mod.rs Outdated
@mahirhir
mahirhir force-pushed the fix/range-inf-nan-start-bound branch from 56166da to 5150ebe Compare June 28, 2026 13:05
@mahirhir
refactor(de): move the compact-range number-start check into the parser
004d694 
The inf/NaN literal check now sits next to any_number as
check_any_number_start, so the Range/RangeInclusive and RangeFrom call
sites share one definition instead of repeating the same seven-way
condition.
@juntyr

juntyr commented Sep 5, 2026

Copy link
Copy Markdown
Member

Thanks @mahirhir, could you please also add a change log entry?

@mahirhir

mahirhir commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Added, as b485d55.

## Unreleased

### Bug Fixes

- Fixed round-tripping of compact ranges whose bounds are `inf` or `NaN` ([#605](https://github.com/ron-rs/ron/pull/605))

I put it under a new ### Bug Fixes heading rather than ### Format Changes, since nothing about the format changed here, only that the deserializer now accepts what the serializer already emitted. Say the word if you would rather it sat under the existing heading.

One thing worth flagging: my branch predates the #585 entry that is on master's Unreleased section now, so the merge may want a hand there. Happy to merge master in if that is easier for you than resolving it.

@mahirhir
docs(changelog): rebuild the entry on master's current file
d22bdb5 
The first attempt edited my branch's copy, which was four lines behind master
under Unreleased, so the file conflicted at exactly the line the entry goes on.
This writes master's bytes plus the one Bug Fixes entry, so the diff this PR
shows for CHANGELOG.md is those four lines and nothing else.
@mahirhir
Merge master into fix/range-inf-nan-start-bound
69c9467 
Only CHANGELOG.md conflicted: master added an Unreleased "Format Changes"
entry for ron-rs#585 on the same lines this branch used for its "Bug Fixes"
entry. Both are kept, with Format Changes first to match the order the
released sections use.

src/parse.rs merged without conflict. Master's work there is in
next_bytes_is_float and the escaped-string loop; this branch only adds
check_any_number_start next to any_number.
@mahirhir
docs(changelog): say start bound, which is what the fix covers
6e6bcf6 
The entry claimed both bounds. Only the start was broken: the guard was
is_number_start, which does not recognise inf or NaN, while the end bound
already went through any_number.
@mahirhir

mahirhir commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Merged master in (69c9467) and corrected the changelog line (6e6bcf6).

The conflict turned out to be only CHANGELOG.md — both sides had added a section under an ## Unreleased heading the base did not have. I rebuilt it from your bytes and put Bug Fixes after the #585 Format Changes bullet, matching how 0.9.0, 0.12.0 and 0.12.1 order those two.

src/parse.rs auto-merged. I checked rather than assumed: git diff upstream/master -- src/parse.rs is exactly my 18 lines around check_any_number_start, and your is_string_continuation_whitespace, the bounded .. search in next_bytes_is_float, the find(['"','\\']) loop and consume_string_continuation are all still there.

Full suite after the merge: 462 passed, 0 failed. 601_support_for_number_ranges 8, 607_quadratic_float_parsing 1, 609_quadratic_escaped_string_parsing 2, escape 12.

I also fixed my own changelog wording. It said "whose bounds are inf or NaN", which overstates it — only the start bound was broken, since the guard was is_number_start and the end already went through any_number. It now says start bound.

The two workflow runs sit at action_required, which I think needs your approval to run on a fork PR.

@juntyr
juntyr merged commit 7f765e9 into ron-rs:master Sep 6, 2026
10 checks passed
@LuisFigueroaG LuisFigueroaG mentioned this pull request Oct 27, 2026
Merged
1 task done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@juntyr juntyr juntyr approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mahirhir @juntyr