Repository navigation
Conversation
Why:
`f(a, ...)` put its callee and arguments inside the function it
creates, so they ran on every call, and an `await` in them ended up in
a non-async arrow, a JavaScript syntax error.
They are now bound outside that function, as the optional-argument path
already did. Variables, constants, closures, external values and module
paths stay in place, so such a function can still be inlined. Fixes #8932.
JS modified (partial application):
Example:
let add = (a, b, c) => a + b + c
let f = async p => {
let h = add(await p, ...)
h(1, 2) + h(3, 4)
}
Before (js):
async function f(p) {
let h = (extra, extra$1) => add(await p, extra, extra$1);
return h(1, 2) + h(3, 4) | 0;
}
After (js):
async function f(p) {
let arg = await p;
return add(arg, 1, 2) + add(arg, 3, 4) | 0;
}
Review: Is evaluating arguments once a behaviour change? Yes, for
arguments with side effects (earger_curry_test shows one). They now run
once, where the application is, like the arguments of any other call.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fc7LEuyEfynjaKCVEBPmTF
Signed-off-by: Christoph Knittel <ck@cca.io>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8936 +/- ##
==========================================
+ Coverage 81.13% 81.14% +0.01%
==========================================
Files 456 457 +1
Lines 63754 63805 +51
==========================================
+ Hits 51728 51776 +48
- Misses 12026 12029 +3
🚀 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-8936 |
|
@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. |
![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: 4882045df7
ℹ️ 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".
Each evaluation of a closure is a new one, and an external value can be reassigned or be a getter, so neither may be re-evaluated on each call. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fc7LEuyEfynjaKCVEBPmTF Signed-off-by: Christoph Knittel <ck@cca.io>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be72a815b0
ℹ️ 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".
Now that `f(a, ...)` evaluates its callee and arguments once, the
function it creates shares them, so treating every partial application as
nonexpansive let `first([], ...)` return one array at two element types.
Typing modified (value restriction for partial application):
Example:
let first = (x, _y) => x
let h = first([], ...)
let ints: array<int> = h(0)
let strings: array<string> = h(0)
Before (rule):
───────────────────────── (nonexp-partial)
nonexpansive(`f(a₁, …, aₙ, ...)`)
After (rule):
nonexpansive(f) nonexpansive(a₁) … nonexpansive(aₙ)
───────────────────────────────────────────────────── (nonexp-partial)
nonexpansive(`f(a₁, …, aₙ, ...)`)
After (diagnostic):
This has type: array<int>
But it's expected to have type: array<string>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fc7LEuyEfynjaKCVEBPmTF
Signed-off-by: Christoph Knittel <ck@cca.io>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fc7LEuyEfynjaKCVEBPmTF Signed-off-by: Christoph Knittel <ck@cca.io>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ecb2c66788
ℹ️ 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".
A constant block such as a tuple is emitted as a new JavaScript value on each evaluation, so only constants that allocate nothing stay in place. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fc7LEuyEfynjaKCVEBPmTF Signed-off-by: Christoph Knittel <ck@cca.io>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 885afb2c07
ℹ️ 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".
The callee and allocating arguments are strict bindings around the generated function, callee first; variables and constants stay inside. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fc7LEuyEfynjaKCVEBPmTF Signed-off-by: Christoph Knittel <ck@cca.io>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ff19a3d27
ℹ️ 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".
| known one that can be inlined. A closure or a constant block does not: | ||
| each evaluation is a new one. Nor does an external value, which can be | ||
| reassigned or be a getter. *) | ||
| | Lconst c when not (Lambda.const_is_allocating c) -> lam |
There was a problem hiding this comment.
When a supplied argument is a nested option such as Some(None), translation produces Lconst (Const_some (Const_js_undefined ...)); Lambda.const_is_allocating returns false because it merely recurses through Const_some, while lam_compile_const.translate_some emits Caml_option.some(undefined), which allocates a fresh object. This exemption therefore leaves the allocation inside the wrapper, so let h = first(Some(None), ...); h(0) === h(0) is false even though the argument should be evaluated once. Unlike the previously fixed Const_block case, this fresh case contains no Const_block, so the predicate or hoisting logic must also recognize runtime-allocating optional constants.
Useful? React with 👍 / 👎.
This branch has not been deployed
1 participant
Fixes #8932.
Partial application
f(a, ...)put its callee and arguments inside the function it creates. They were evaluated on every call of that function, and anawaitin them ended up in a non-async arrow:The callee and arguments are now bound outside the new function, as the optional-argument path (
build_apply) already did; the two paths share the helper. Only variables, constants that allocate nothing and module paths such asBelt.Map.fromArraystay in place, because they read the same on every call; without that, a partial application likeN.fromArray(~id=module(IntCmp), ...)would no longer be inlined at its call sites. Closures, constant blocks such as tuples, and external values are bound too: each evaluation of a closure or block is a new one, and an external value can be reassigned or be a getter.This is a behaviour change for arguments with side effects, which now run once, where the application is, like the arguments of any other call. Generated output changes in five test files:
earger_curry_test: the side effects in the arguments run once, at application.test_demo: the calleeg1(3, 4)is evaluated once.bs_array_test: an array literal argument is allocated once instead of on every call, and the calls are now inlined.inline_regression_test: a closure argument becomes a top-level function, created once.PartialApplicationNoRuntimeCurry: the external valuehookis read once, intoarg; the calls ofaddHookPartialare no longer inlined.Because the function a partial application creates now shares its callee and arguments, it is generalized only when they are values:
let h = first([], ...)gets a weak type, aslet h = first([])would, whilefirst(1, ...)stays polymorphic.It is also a prerequisite for generator functions (#8934): a
yieldin a partial application would be moved into the closure the same way.Test:
tests/tests/src/partial_application_eval_test.rescovers single evaluation of arguments, of the callee, of a closure argument and a constant tuple argument (identity) and of an external getter, left-to-right evaluation order, and an awaited argument with the partial application used twice, and that a partial application of values stays polymorphic.super_errors/fixtures/partial_application_value_restriction.reschecks thatfirst([], ...)is not generalized.tests/ounit_tests/ounit_partial_application_tests.mlchecks the Lambda: strict bindings around the generated function with the callee first, and which arguments stay inside.🤖 Generated with Claude Code
https://claude.ai/code/session_01Fc7LEuyEfynjaKCVEBPmTF