Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #479 +/- ##
==========================================
+ Coverage 89.99% 90.31% +0.32%
==========================================
Files 7 7
Lines 1519 1528 +9
==========================================
+ Hits 1367 1380 +13
+ Misses 152 148 -4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Thanks for the clear writeup and for jumping on a first PR the same day. DiagnosisWith Always running the integer lexer, and only taking the float path for actual floats / specials, is the right fix and the right layer.
|
|
Thanks for your fast feedback! I reworked my PR without modifying the I tried a few different approaches to handle all the specific cases covered by the tests, and I believe the current version should work correctly. I also removed all the trailing spaces I had added previously, and I believe all the comments should now be clear and up to date. If everything looks good to you, I'm happy with the current version. Otherwise, I'm happy to make any further changes if needed. |
quinnj
left a comment
There was a problem hiding this comment.
[Reviewed by AI] Rechecked head 527057f. The original typed-integer failure is now covered by merged #480; current master 3e4cb97 passes the original struct case and Int64/UInt64/Int128 limit probes. This branch conflicts with current master in src/lazy.jl and test/parse.jl. Its full local suite also hits two signed-zero expectations that fail on the original base with the same dependencies. The new regression below is independent of those existing failures.
| isfloat = !(UInt8('0') <= b <= UInt8('9')) | ||
| end | ||
| end | ||
| if !isfloat |
There was a problem hiding this comment.
[Reviewed by AI] P2: Keep configured numeric special tokens ahead of integer parsing. For example, JSON.parse("0"; allownan=true, nan="0") and JSON.parse("1"; allownan=true, inf="1") returned NaN/Inf on the original base; this branch returns integers 0/1 because the new integer path bypasses @check_special. Both baseline controls pass with matched dependencies. Current master already preserves these overrides through #480, so this logic should not replace that implementation.
Summary
Root Cause
When using allownan, any number was parse as Float, and then convert as Int if needed. But the parsing as Float64 was rounded from 2e63 -1 to 2e63.
Notice: it's my first PR, so I may have done some mistakes. I ran all tests locally, and it was working.
Fixes #478