feat(parse): accept explicit + sign in e-notation exponent#248
feat(parse): accept explicit + sign in e-notation exponent#248thedavidmeister wants to merge 3 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>
|
Warning Review limit reached
Next review available in: 29 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
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>
…[3b-attempt] ZOLTU_DEPLOYED_DECIMAL_FLOAT_ADDRESS and DECIMAL_FLOAT_CONTRACT_HASH are now set to the values computed from this PR's bytecode (explicit + sign in e-notation exponent changes source). Computed by testDeployAddress and testExpectedCodeHashDecimalFloat failure output: address: 0x9b0eF7FbDF04b3A58989860Dea0534C978d64464 hash: 0x369ac78d686dfdf1d41771dc7bf321dcd339b0a454dc9cf6c37eff579c3a0c3a Co-Authored-By: Claude <noreply@anthropic.com>
|
Producer note: WAITING-DEPLOY. Deploy constants now regenerated to this PR's bytecode (testDeployAddress/testExpectedCodeHash pass); the remaining red is the 5 testProdDeployment* network pins ('DecimalFloat not deployed') — same condition as main, needs the routine interactive pre-merge Zoltu deploy from this branch, not a code fix. |
|
🤖 ai:producer |
|
🤖 ai:vetter |
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.