feat(parse): accept explicit + sign in e-notation exponent#248
feat(parse): accept explicit + sign in e-notation exponent#248thedavidmeister wants to merge 2 commits into
Conversation
`1e+2`, `1.0e+2`, `1E+2` etc. now parse identically to `1e2`. `1e+` with no digits still rejects with MalformedExponentDigits. Fixes rainlanguage/rainlang#214. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR extends ChangesPositive Exponent Sign Support
Estimated code review effort: 1 (Trivial) | ~5 minutes Related PRs: None identified. Suggested labels: enhancement, tests Suggested reviewers: None identified. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/parse/LibParseDecimalFloat.sol (1)
141-151: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winMixed
-/+exponent signs silently mis-parsed instead of rejected.The new
+check runs unconditionally after the-skip, without verifying that no negative sign was already consumed. For input like"1e-+2":
- Line 145 skips the
-, advancingcursorto point at+.- Line 148 then matches
+at that position, advancescursorpast it, and overwriteseStartto point at2— discarding the previously consumed-entirely.- The exponent parses as
+2instead of being rejected, silently dropping the sign that was present in the input.Previously (before this PR)
"1e-+2"failed withMalformedExponentDigitsbecause+is not a digit. Now it succeeds with an incorrect (wrong-sign) exponent value. This is a real regression for a decimal-float parser that presumably feeds downstream numeric/financial logic.🐛 Proposed fix: only accept `+` when no `-` was already consumed
uint256 eStart = cursor; - cursor = LibParseChar.skipMask(cursor, end, CMASK_NEGATIVE_SIGN); - // Skip an optional explicit positive sign (e.g. 1e+2). Advance - // eStart past it so the int parser only sees the digit string. - if (LibParseChar.isMask(cursor, end, CMASK_PLUS_SIGN) == 1) { - cursor++; - eStart = cursor; - } + uint256 cursorBeforeSign = cursor; + cursor = LibParseChar.skipMask(cursor, end, CMASK_NEGATIVE_SIGN); + // Skip an optional explicit positive sign (e.g. 1e+2), but only + // if a negative sign was not already consumed above. Advance + // eStart past it so the int parser only sees the digit string. + if (cursor == cursorBeforeSign && LibParseChar.isMask(cursor, end, CMASK_PLUS_SIGN) == 1) { + cursor++; + eStart = cursor; + }Please add test cases for
"1e-+2"and"0.0e-+1"assertingMalformedExponentDigits(or another rejection), to lock in the correct behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/parse/LibParseDecimalFloat.sol` around lines 141 - 151, The exponent-sign handling in LibParseDecimalFloat.parseDecimalFloat currently accepts a plus sign even after a minus has already been consumed, which lets mixed signs like 1e-+2 slip through. Update the exponent parsing logic around the eValue/cursor/eStart block so the optional CMASK_PLUS_SIGN is only recognized when no CMASK_NEGATIVE_SIGN was skipped, and otherwise leave the parser to fail with MalformedExponentDigits. Add regression tests covering 1e-+2 and 0.0e-+1 to ensure mixed signs are rejected.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/lib/parse/LibParseDecimalFloat.sol`:
- Around line 141-151: The exponent-sign handling in
LibParseDecimalFloat.parseDecimalFloat currently accepts a plus sign even after
a minus has already been consumed, which lets mixed signs like 1e-+2 slip
through. Update the exponent parsing logic around the eValue/cursor/eStart block
so the optional CMASK_PLUS_SIGN is only recognized when no CMASK_NEGATIVE_SIGN
was skipped, and otherwise leave the parser to fail with
MalformedExponentDigits. Add regression tests covering 1e-+2 and 0.0e-+1 to
ensure mixed signs are rejected.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 12c3e737-fa22-481d-a6a1-9aceddbfddd3
📒 Files selected for processing (2)
src/lib/parse/LibParseDecimalFloat.soltest/src/lib/parse/LibParseDecimalFloat.t.sol
Regenerate DecimalFloat.json artifact after adding explicit + sign support in e-notation exponent parsing. Co-Authored-By: Claude <noreply@anthropic.com>
1 participant
Summary
LibParseDecimalFloat.parseDecimalFloatInlinenow accepts an optional explicit+sign aftere/E(e.g.1e+2,1.0e+2), treating it identically to the implicit positive case (1e2)+with no following digits still rejects withMalformedExponentDigits(same as1e-)CMASK_PLUS_SIGNimport fromrain-stringAddresses rainlanguage/rainlang#214.
Deploy required: source change moves the
decimal-floatbytecode →testDeployAddress,testExpectedCodeHashDecimalFloat, andtestArtifactsCommittedare red pending amanual-sol-artifacts.yamlrun on this branch withsuite=decimal-float.Test plan
testParseLiteralDecimalFloatPositiveExponentSign— 9 positive casestestParseLiteralDecimalFloatPositiveENoDigits—1e+and0.0e+still rejecttestParseLiteralDecimalFloatExponents— 505 total tests pass (8 expected failures: 5 fork/RPC, 3 bytecode-pin pre-deploy)🤖 Generated with Claude Code
Summary by CodeRabbit
+sign in exponents, so values like1e+2are handled correctly.+but no exponent digits still fail with the expected error.