Fix #e prefix for decimal numbers to produce exact Rationals#7
Conversation
This commit modifies the number parser to correctly handle the `#e` exactness prefix when applied to decimal numbers (e.g. `#e1.223`) or scientific notation (e.g. `#e1.2e2`). Previously, these were parsed as inexact floating point numbers, violating R7RS which states that `#e` must force exactness. Changes: - Added `parseDecimalAsExact` helper in `src/core/interpreter/reader/number_parser.js` to parse decimal strings into `Rational` or `BigInt` using BigInt arithmetic. - Updated `parsePrefixedNumber` to use this new helper when `exactness` is 'exact'. - Added error handling to throw `SchemeReadError` for `#e+inf.0` / `#e+nan.0`, as these cannot be exact. - Added comprehensive unit tests in `tests/core/interpreter/reader/number_parser_tests.js`. Verified with: - New unit tests passing. - Chibi compliance tests passing (including numeric syntax). - Full regression suite passing. Co-authored-by: mark-friedman <1743690+mark-friedman@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
@jules, parsePrefixedNumber is a pretty long function. Can it be reasonably split up and/or refactored?
There was a problem hiding this comment.
I have refactored parsePrefixedNumber by splitting it into smaller, more focused helper functions: parsePrefixes, parseRationalWithRadix, parseComplexWithRadix, and parseRealWithRadix. This should improve readability and maintainability significantly.
mark-friedman
left a comment
mark-friedman
left a comment
There was a problem hiding this comment.
See my comment about parsePrefixedNumber
- Split `parsePrefixedNumber` into smaller helper functions: - `parsePrefixes` - `parseRationalWithRadix` - `parseComplexWithRadix` - `parseRealWithRadix` - This addresses code review feedback regarding function length and complexity. - Verified with unit tests. Co-authored-by: mark-friedman <1743690+mark-friedman@users.noreply.github.com>
1 participant
Fixes Issue #6 by implementing exact decimal parsing for numbers with the
#eprefix.When a number like
#e1.223is encountered, it is now parsed into the exact Rational1223/1000(orBigIntif the result is an integer like#e1.2e2->120), instead of an inexact float.This aligns behavior with R7RS Section 6.2.4. It also enforces that
#ecannot be used with infinities or NaNs, raising aSchemeReadError.PR created automatically by Jules for task 1281184460849524705 started by @mark-friedman