Repository navigation
Fix the sign of @inline bigint constants - #8732
Conversation
Bigint_utils.parse_bigint returns whether the literal is positive, but Ast_external_mk.inline_bigint bound that flag as `negative`, so the stored inline constant had its sign inverted. Translation passed the flag straight through as Lambda's positive `sign`, inverting it back, so generated JavaScript was correct; signatures and error messages printed `@inline(-12n)` for `@inline(12n)`. Store the real `negative` flag and convert it to Lambda's sign in translcore. Signed-off-by: Christoph Knittel <christoph@knittel.cc> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
c02bbfe to
eac1e3d
Compare
Bigint_utils.parse_bigint, Asttypes.Const_bigint, Lambda.Const_bigint and Js_op.bigint_lit all record whether a bigint is positive. The inline constant was the only one recording `negative`, which is how its sign got inverted. Use `positive` there too, so the value passes through the frontend and translation unchanged. Signed-off-by: Christoph Knittel <christoph@knittel.cc> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #8732 +/- ##
==========================================
+ Coverage 79.79% 79.81% +0.02%
==========================================
Files 464 464
Lines 63134 63134
==========================================
+ Hits 50377 50391 +14
+ Misses 12757 12743 -14
🚀 New features to boost your workflow:
|
|
Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-8732 |
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Bigint_utils.parse_bigintreturns whether a literal is positive, butAst_external_mk.inline_bigintbound that flag asnegative, so every@inlinebigint constant was stored with its sign inverted.Translation passed the flag straight into Lambda's
Const_bigint, whose flag means positive, which inverted it back. So generated JavaScript was correct, but signatures and error messages printed the wrong sign:This renames the field to
positive, matchingBigint_utils.parse_bigint,Asttypes.Const_bigint,Lambda.Const_bigintandJs_op.bigint_lit, which all record whether a bigint is positive. The flag now passes through the frontend and translation unchanged.Found while looking into structural
@inlineconstants (#8624).Tests
tests/tests/src/inline_const.res: a functor whose parameter declares@inline(12n)and@inline(-5n). A functor parameter is currently the only way to reach codegen with a bigint inline constant, since@inline let x = 12nisn't supported in implementations yet. This pins that the JS output keeps12n/-5n.super_errors/inline_bigint_signature_mismatch.res: pins the printed sign of both constants in a signature mismatch.🤖 Generated with Claude Code