Eliminate Pjs_fn_make, Pjs_fn_make_unit, and unsafe_adjust_to_arity - #8570
Open
cristianoc wants to merge 1 commit into
Open
Eliminate Pjs_fn_make, Pjs_fn_make_unit, and unsafe_adjust_to_arity#8570cristianoc wants to merge 1 commit into
cristianoc wants to merge 1 commit into
Conversation
cristianoc
force-pushed
the
codex/nary-pjs-fn-make
branch
from
August 20, 2026 07:08
36801fd to
966cce5
Compare
![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: 36801fdbc0
ℹ️ 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".
| - Sync the platform npm package's compiler binaries (`packages/@rescript/<platform>/bin`) via dune promotion on every `dune build`, instead of Makefile/CI copy steps that only ran when make did: a plain `dune build` can no longer leave `cli/*.js` and the test harnesses running a stale compiler. https://github.com/rescript-lang/rescript/pull/8560 | ||
| - Remove unused compiler IR definitions, modules, helpers, error variants, and Typedtree fields. https://github.com/rescript-lang/rescript/pull/8551 https://github.com/rescript-lang/rescript/pull/8555 | ||
| - Cleanups enabled by structural arity: remove the unreachable `Too_many_arguments` error and the `?in_function` threading through the type checker that existed only to decorate it; remove the dead `function$`-vs-arrow unification bridge, `Ctype.arity`, and the unused parsetree arity helpers; deduplicate the analysis arrow-flattening helpers. https://github.com/rescript-lang/rescript/pull/8569 | ||
| - Eliminate the `Pjs_fn_make`/`Pjs_fn_make_unit` arity-adjustment primitives and the `unsafe_adjust_to_arity` machinery: with structural arity, functions are constructed at their final arity, so the enforcement layer (and the active-pattern currying split it compensated for) is deleted. Generated code improves: no adapter closures for patterns on mutable fields, better constant propagation and name preservation, and recursive modules whose members are plain functions compile statically without the runtime bootstrap. |
There was a problem hiding this comment.
This newly added user-facing changelog entry ends without its PR URL, although repository policy requires entries under the current unreleased version to end with the corresponding PR link. Append that link so the release note remains traceable to the change.
AGENTS.md reference: AGENTS.md:L138-L138
Useful? React with 👍 / 👎.
cristianoc
force-pushed
the
codex/nary-pjs-fn-make
branch
from
August 20, 2026 07:24
966cce5 to
f36394c
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## codex/nary-dead-code #8570 +/- ##
========================================================
+ Coverage 75.82% 75.87% +0.05%
========================================================
Files 474 474
Lines 62870 62768 -102
========================================================
- Hits 47671 47627 -44
+ Misses 15199 15141 -58
🚀 New features to boost your workflow:
|
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
cristianoc
force-pushed
the
codex/nary-pjs-fn-make
branch
from
August 20, 2026 07:49
f36394c to
c3e20c1
Compare
cristianoc
force-pushed
the
codex/nary-pjs-fn-make
branch
from
August 20, 2026 08:15
c3e20c1 to
4df8956
Compare
cristianoc
force-pushed
the
codex/nary-pjs-fn-make
branch
from
August 20, 2026 09:16
4df8956 to
cbc902e
Compare
With structural arity, "function of arity n" is a construction invariant rather than a goal state: translcore builds every Lfunction with exactly the parameters its type declares. The arity-enforcement layer therefore disappears: - Function literals are emitted directly; the Pjs_fn_make wrapper that every function passed through was resolved as a no-op by lam_pass_alpha_conversion, but only *after* deep_flatten, simplify_exits and simplify_alias had run with the function hidden inside an Lprim, acting as an accidental optimization barrier. - Pjs_fn_make_unit was a one-bit metadata channel: its entire effect was setting one_unit_arg so js_exp_make drops the unit parameter. translcore now sets the attribute directly, gated on the parameter pattern binding no identifiers (a () or _ pattern) - a more principled test than the alpha pass's check that the parameter was named "param". - The active-pattern currying split in transl_function is deleted. It preserved pattern-effect timing across curried application steps, which no longer exist: total applications supply all arguments at once and explicit partial application eta-defers the entire call. The old output proves the point - the split's closures were immediately applied by the arity adapter, so only the allocations are gone (see mutable_uncurry_test). - The I<N> unboxed-record producer (the @this method-callback encoding) is removed: the general Record_unboxed translation already returns the single field unboxed, and the wrapped value is a literal of matching arity. With no producers left, both primitive constructors and every consumer arm are deleted, including the 230-line unsafe_adjust_to_arity (its only callers were the two Pjs_fn_make resolution sites). On recursive modules: removing the wrapper lets the static recursive-module compilation path see module members that are plain functions, replacing the Primitive_module.init/update bootstrap with hoisted function declarations. This is safe because the static path's own applicability check now sees the functions it was designed to check - the wrapper was hiding them, pessimizing compilation - and the bootstrap demonstrably remains for members that are not plain functions (rec_module_test keeps its lazy/value cases dynamic). Verified: stdlib byte-identical; full test suite green; JS output changes limited to removed adapter closures, removed no-op module bootstraps, better name preservation, and constant propagation. Keep beta-residue let chains (immutable blocks and aliases feeding a final apply) local in Lam_pass_deep_flatten instead of hoisting them into the enclosing group: with the wrapper gone, beta reduction happens before flatten2, and hoisting the argument bindings to toplevel put them beyond Lam_pass_lets_dce's reach (a_recursive_type kept a named intermediate that master inlined; the snapshot pins the restored form). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Cristiano Calcagno <cristianoc@users.noreply.github.com>
cristianoc
force-pushed
the
codex/nary-pjs-fn-make
branch
from
August 20, 2026 10:13
cbc902e to
b7d4b07
Compare
7 tasks
cknitt
approved these changes
Aug 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of the n-ary functions series tracked in #8557 (item 7; stacked via base branch only — each PR merges independently).
What
With structural arity, "function of arity n" is a construction invariant rather than a goal state: translcore builds every
Lfunctionwith exactly the parameters its type declares. The arity-enforcement layer therefore disappears:Pjs_fn_makewrapper that every function passed through was resolved as a no-op bylam_pass_alpha_conversion, but only afterdeep_flatten,simplify_exitsandsimplify_aliashad run with the function hidden inside anLprim, acting as an accidental optimization barrier.Pjs_fn_make_unitwas a one-bit metadata channel: its entire effect was settingone_unit_argsojs_exp_makedrops the unit parameter. translcore now sets the attribute directly, gated on the parameter pattern binding no identifiers (a()or_pattern) — a more principled test than the alpha pass's check that the parameter was named"param".transl_functionis deleted. It preserved pattern-effect timing across curried application steps, which no longer exist: total applications supply all arguments at once and explicit partial application eta-defers the entire call. The old output proves the point — the split's closures were immediately applied by the arity adapter, so only the allocations are gone (seemutable_uncurry_test).I<N>unboxed-record producer (the@thismethod-callback encoding) is removed: the generalRecord_unboxedtranslation already returns the single field unboxed, and the wrapped value is a literal of matching arity.With no producers left, both primitive constructors and every consumer arm are deleted, including the 230-line
unsafe_adjust_to_arity(its only callers were the twoPjs_fn_makeresolution sites).Recursive modules
Removing the wrapper lets the static recursive-module compilation path see module members that are plain functions, replacing the
Primitive_module.init/updatebootstrap with hoisted function declarations. This is safe because the static path's own applicability check now sees the functions it was designed to check — the wrapper was hiding them, pessimizing compilation — and the bootstrap demonstrably remains for members that are not plain functions (rec_module_testkeeps its lazy/value cases dynamic).Review notes
-debug-ir). A follow-up in this PR keeps the beta-residue let chains local inLam_pass_deep_flattensoLam_pass_lets_dcecan still substitute single-use arguments into the call — corpus impact: exactly one snapshot (a_recursive_type.mjs), restored to master's inline form.Lam_beta_reduce(reproducible on master, visible inbs_set_int_test.mjs's checked-in output); fixed separately in Fix argument evaluation order under function inlining #8572.Verification
Stdlib byte-identical; full test suite green; JS output changes limited to removed adapter closures, removed no-op module bootstraps, better name preservation, and constant propagation.
🤖 Generated with Claude Code