Skip to content

Comments

fix: correctly parse i128::MIN from text Ion#1018

Merged
almann merged 1 commit intomainfrom
i128-bug
Jan 6, 2026
Merged

fix: correctly parse i128::MIN from text Ion#1018
almann merged 1 commit intomainfrom
i128-bug

Conversation

@almann
Copy link
Contributor

@almann almann commented Jan 5, 2026

The text Ion parser rejected i128::MIN (-170141183460469231731687303715884105728) as out of range, even though it is a valid i128 value. This happened because the parser stripped the sign, parsed the magnitude as i128, then negated. Since the magnitude of i128::MIN is i128::MAX + 1, parsing it as i128 failed.

Fixed by parsing the magnitude as u128 with proper range validation, then using wrapping_neg() to correctly convert to the negative i128 value.

Added boundary value tests for i32, i64, and i128 min/max values.

Add read_ints_overflow test to verify the parser correctly rejects integers outside the i128 range:

  • i128::MAX + 1 (positive overflow)
  • i128::MIN - 1 (negative overflow)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

* fix: correctly parse `i128::MIN` from text Ion

The text Ion parser rejected `i128::MIN` (`-170141183460469231731687303715884105728`)
as out of range, even though it is a valid `i128` value. This happened because the
parser stripped the sign, parsed the magnitude as `i128`, then negated. Since the
magnitude of `i128::MIN` is `i128::MAX + 1`, parsing it as `i128` failed.

Fixed by parsing the magnitude as `u128` with proper range validation, then using
`wrapping_neg()` to correctly convert to the negative `i128` value.

Added boundary value tests for `i32`, `i64`, and `i128` min/max values.

* test: add overflow tests for `i128` boundary parsing

Add `read_ints_overflow` test to verify the parser correctly rejects
integers outside the `i128` range:
- `i128::MAX + 1` (positive overflow)
- `i128::MIN - 1` (negative overflow)
@almann almann requested a review from jobarr-amzn January 5, 2026 15:20
@codecov
Copy link

codecov bot commented Jan 5, 2026

Codecov Report

❌ Patch coverage is 94.23077% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.36%. Comparing base (bbcd84c) to head (aa2cc4c).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/lazy/text/matched.rs 94.23% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1018      +/-   ##
==========================================
+ Coverage   78.33%   78.36%   +0.02%     
==========================================
  Files         138      138              
  Lines       34188    34232      +44     
  Branches    34188    34232      +44     
==========================================
+ Hits        26781    26825      +44     
+ Misses       5345     5344       -1     
- Partials     2062     2063       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@almann almann merged commit d18d2bc into main Jan 6, 2026
36 checks passed
@jobarr-amzn jobarr-amzn deleted the i128-bug branch January 6, 2026 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants