Repository navigation
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #8689 +/- ##
==========================================
+ Coverage 81.97% 81.99% +0.01%
==========================================
Files 456 456
Lines 65671 65681 +10
==========================================
+ Hits 53836 53854 +18
+ Misses 11835 11827 -8
🚀 New features to boost your workflow:
|
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
|
Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-8689 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24646c5cb8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
24646c5 to
08056a9
Compare
Changelog🚀 New Feature
Semantic Diff9 commits · Full page ✏️ Enable selective cross-module inlining for Option helpersbb8a422 · cknitt · Sep 28, 2026, 06:04 UTC · edited
✏️ Inline additional Option helpers across modulese33d56d · cknitt · Sep 28, 2026, 07:46 UTC · edited Option's filter, mapOr, getOr, orElse, isSome and isNone use ✏️ Document cross-module inlining featurecd458ae · cknitt · Sep 28, 2026, 07:49 UTC · edited Add the changelog fragment for ✏️ Refresh Option inlining snapshots after clean buildd44363e · cknitt · Sep 28, 2026, 07:55 UTC · edited A clean build of the tests inlines the Option helpers in more places than the incremental build the snapshots were made with. ✏️ Track CMJ changes for cross-module inlining008acbd · cknitt · Sep 28, 2026, 09:19 UTC · edited Rewatch now compares the .cmj itself instead of a separate .cmj.inline digest: exported JavaScript metadata, inline bodies included, can change without changing the .cmi, and callers compiled against the old .cmj are then stale. ✏️ Read CMJ digest header for incremental rebuildsb08d5b8 · cknitt · Sep 28, 2026, 09:53 UTC · edited The compiler writes the digest of a .cmj's contents in its first 16 bytes, so Rewatch can read that instead of hashing the whole file. Recompile dependents of a changed CMJ in the OCaml build systemb2066b0 · cknitt + Claude · Oct 27, 2026, 07:15 UTC
Publishing a byte-different CMJ now marks dependents dirty like a changed CMI, and the startup freshness check compares a dependent's CMT with the later of its dependency's CMI and CMJ. Review
Treat catch binders as bound in Lam_closure.free_variables01e3b54 · cknitt + Claude · Oct 29, 2026, 09:51 UTC Answers the Codex finding that an |
|
@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". |
5899ffe to
1dbd977
Compare
Why:
Calls to small stdlib helpers such as `Option.map` stay calls into
`Stdlib_Option` unless the whole build enables cross-module
optimization. `@inline(crossModule)` marks a closed function whose body
the .cmj exports, so other modules inline its calls as they would a
local function. Option's forEach, map, flatMap, filter, mapOr, getOr,
orElse, isSome and isNone use it.
JS modified (Option helper calls):
Example:
let bign = BigInt.fromFloat(Number.MAX_VALUE)->Option.getOr(0n)
Before (js):
let bign = Stdlib_Option.getOr(Stdlib_BigInt.fromFloat(Number.MAX_VALUE), 0n);
After (js):
let opt = Stdlib_BigInt.fromFloat(Number.MAX_VALUE);
let bign = opt !== undefined ? Primitive_option.valFromOption(opt) : 0n;
Diagnostic modified (@inline payload):
Example:
@inline(sometimes)
let f = x => x
Before (diagnostic):
illegal payload for attribute @inline.
It must be either empty, 'always' or 'never'
After (diagnostic):
illegal payload for attribute @inline.
It must be empty, 'always', 'never' or 'crossModule'
An async or directed function, or one capturing local values, is rejected
with an error. The build system recompiles dependents when a module's .cmj
changes, not only its .cmi, since an inline body can change without its
type: during a build when the published .cmj differs, and on startup when
a dependency's .cmi or .cmj is newer than the dependent's .cmt.
Review: Does every implementation change now recompile dependents? Only
those whose .cmj bytes change.
Review: Why does Lam_closure.free_variables change? Its `Ltrywith` case
never bound the exception, so a function whose `catch` reads it counted as
capturing a local value and was rejected.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kbf3gKsWykY6KH4QSJ9Wrs
Signed-off-by: Christoph Knittel <ck@cca.io>
1dbd977 to
ccb2418
Compare
Changelog🚀 New Feature
Semantic Diff1 commit · Full page ✨ Inline functions marked
|
|
| Benchmark | Base | Head | Change |
|---|---|---|---|
build/Namespace/edit |
7,943,503 | 8,063,811 | +1.51% |
build/no-op |
9,304,489 | 9,385,663 | +0.87% |
packages/@rescript/runtime/Stdlib_Option.res (inputs changed) |
762,255 | 768,899 | +0.87% |
build/leaf-edit |
13,196,287 | 13,277,494 | +0.62% |
packages/@rescript/runtime/Stdlib_Nullable.res (inputs changed) |
331,825 | 332,616 | +0.24% |
packages/@rescript/runtime/Stdlib_Null.res (inputs changed) |
334,290 | 335,081 | +0.24% |
build/shared-edit |
72,914,622 | 73,018,460 | +0.14% |
build/Namespace/clean |
188,699,272 | 188,902,117 | +0.11% |
(inputs changed): this pull request changes runtime or Belt sources, so these benchmarks compile different code than on the base, and their changes don't count as regressions.
Instructions
In total: 9,211,172,326 → 9,211,182,891 (±0.00%).
None of the 33 benchmarks changed by 0.1% or more.
Output size
In total: 25,843,198 → 25,845,339 (+0.01%).
| Benchmark | Base | Head | Change |
|---|---|---|---|
output/runtime/*.cmj (inputs changed) |
19,973 | 20,758 | +3.93% |
(inputs changed): this pull request changes runtime or Belt sources, so these benchmarks compile different code than on the base, and their changes don't count as regressions.
Pull request ccb2418 compared with base 00e3f40 in this run.
Add
@inline(crossModule)to opt individual functions into cross-module inlining, and use it for selectedOptionhelpers.Inlining copies a function’s body into its callers. If that body changes but its type does not, the caller must still be rebuilt; checking only the
.cmitype signature would leave the old body in the generated JavaScript.The compiler stores exported inline bodies in the existing
.cmjfile, alongside other JavaScript compilation metadata. Rewatch now compares the digest already stored in that file’s header before and after compiling a module. If it changes, Rewatch rebuilds dependents. This keeps implementation details out of.cmiand requires no new artifact. A.cmjchange unrelated to inlining may also cause a dependent rebuild.