Eliminate Pjs_fn_make, Pjs_fn_make_unit, and unsafe_adjust_to_arity - #8570
Eliminate Pjs_fn_make, Pjs_fn_make_unit, and unsafe_adjust_to_arity#8570cristianoc wants to merge 1 commit into
Conversation
36801fd to
966cce5
Compare
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.
Append the PR link to the changelog entry
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 👍 / 👎.
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 62868 62758 -110
========================================================
- Hits 47669 47617 -52
+ 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: |
f36394c to
c3e20c1
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. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Cristiano Calcagno <cristianoc@users.noreply.github.com>
c3e20c1 to
4df8956
Compare
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).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