N-ary functions: make arity structural across the compiler - #8557
N-ary functions: make arity structural across the compiler#8557cristianoc wants to merge 3 commits into
Conversation
7d0341c to
8c402ae
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #8557 +/- ##
==========================================
+ Coverage 75.89% 75.95% +0.05%
==========================================
Files 474 474
Lines 62772 62901 +129
==========================================
+ Hits 47640 47774 +134
+ Misses 15132 15127 -5
🚀 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: |
|
Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-8557 |
|
@cknitt tried to put the entire overview together, in this draft PR, before splitting into individual PRs. |
It's already good that CI is green. I can also try to test against a large company project of ours tomorrow. |
|
Starting to extract the first couple of commits, which are just low risk bug fixes, into PRs. |
Done, seeing a single change in compiler output, and that is just the variable name:
for a function parameter |
| }); | ||
| }; | ||
|
|
||
| let non_terminate = g(x); |
There was a problem hiding this comment.
Lost some optimization / inlining here.
There was a problem hiding this comment.
Investigated with per-pass IR dumps (-debug-ir): no work was lost — same beta reduction, different residue shape. Removing Pjs_fn_make moves beta reduction of the immediately applied lambda from the alpha-conversion round to simplify_alias round 1, i.e. before flatten2 — which then hoisted the argument binding to toplevel, beyond Lam_pass_lets_dce's reach (previously the binding stayed local and got substituted). Now fixed at the root in the #8570 commit: Lam_pass_deep_flatten keeps beta-residue let chains local, and this snapshot is restored to exactly master's inline form (let non_terminate = g({TAG: \"A\", _0: g})), pinned by the checked-in JS. Corpus impact of that change: this one file only.
| ]); | ||
|
|
||
| if (!$eq$tilde(sort(u), [ | ||
| let x = sort(u); |
There was a problem hiding this comment.
This one turned out to be an inlining gain with an important catch. The local \"=~" operator was previously invisible to simplify_alias in round 1 (its definition sat behind Pjs_fn_make), and by round 3 — after the wrapper resolves — its body had been rewritten to reference the signature coercion's internal idents, failing the closed-over-exports condition that gates inlining of exported functions. With the wrapper gone the decision happens in round 1, where the body still references only the exported Int_array block: closed, so both call sites inline (one call eliminated each). The let-bound arguments are the standard beta residue for non-substitutable (application) arguments — cost-free after JIT. The catch: digging into why those lets looked odd exposed a pre-existing bug — Lam_beta_reduce stacked the bindings in reverse parameter order, evaluating the last argument first (reproducible on master; visible in bs_set_int_test.mjs's checked-in output). Fixed in #8572, after which this file's bindings come out in source order.
263febd to
a43dd97
Compare
19f6137 to
49ff009
Compare
49ff009 to
c2a8f61
Compare
270640e to
63be65d
Compare
def62af to
4fce94c
Compare
5a11692 to
7d8319a
Compare
7d8319a to
f2a157a
Compare
f36394c to
c3e20c1
Compare
6ce10b8 to
e3a45ec
Compare
4df8956 to
cbc902e
Compare
bcde52f to
d7292aa
Compare
cbc902e to
b7d4b07
Compare
d7292aa to
e771d82
Compare
b7d4b07 to
3b5dadd
Compare
e771d82 to
401166f
Compare
3b5dadd to
0fce1b4
Compare
401166f to
89c5319
Compare
89c5319 to
1713694
Compare
Replace the Pexp_newtype wrapper chains that the parser built for (type t, x) => ... arrow syntax with a structural field on the function node: Pexp_fun.newtypes carries each newtype name with its own attributes, hoisted in front of the value parameters as before. Pexp_newtype remains solely as the desugaring of [let f: type a. ...] annotations and for PPX-authored trees. Fidelity fixes visible in the formatter: - Attributes keep their association with their type parameter group: (@attr type t, x, @attr2 type s, y) round-trips as written instead of printing @attr @attr2 on the function. - Comments written next to a type parameter travel with it to the hoisted group instead of migrating onto the following value parameter. - Attributes written in front of the arrow now live on the function node, so built-in attribute processing (e.g. @this) sees them on type-first functions; previously they sat inert on the wrapper node. Typing follows the upstream OCaml 5.x design: the newtype machinery is extracted into a reusable type_newtype helper (mirroring OCaml's helper of the same name) and the function case peels one newtype at a time, mimicking the typing of the former wrapper chain; the typedtree output is bit-identical to before. The v0 PPX bridge expands the field back into a wrapper chain around Function$: each wrapper carries its own newtype's attributes, and the outermost wrapper separates function-node attributes from the first newtype's attributes with an internal _res.newtype_attrs marker (no marker means node attributes only, matching the historical wire). Newtype-free programs are wire byte-identical; for functions with newtypes the deltas are confined to wrapper-node locations and, for the rare attributed groups, per-wrapper attribute placement. Identity-PPX round-trips are AST-exact, verified against the previous compiler. Also: jsx_v4 and bs_builtin_ppx now carry newtypes (and their attributes) through their function rebuilds instead of dropping them, the sexp AST debugger emits the field, and dead parser plumbing (fundef param attrs/p_pos, arrow_start_pos, make_newtypes ~attrs) is removed. Signed-Off-By: Cristiano Calcagno <cristianoc@users.noreply.github.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace the desugared encoding of [let f: type a. t = e] - a Ptyp_poly
pattern constraint plus a Pexp_newtype chain over a Pexp_constraint,
with the type stored twice and no AST invariant ensuring that the copies
agree - with a structural field on the binding:
pvb_constraint: {pvc_newtypes: string loc list; pvc_type: core_type}
Only the [type a.] form uses the field; plain constraints and explicit
polymorphic annotations keep their existing representation. The type is
stored once, and [varify_constructors] now runs in exactly one place,
inside the type checker.
With functions already carrying their locally abstract type parameters in
Pexp_fun.newtypes, this removes the last place where the parser constructs
Pexp_newtype. Delete the constructor from the current parsetree, along with
the Texp_newtype exp_extra, which had no consumer beyond no-op iterators and
the debug printer. The CMT magic number is bumped to Caml1999T024; the CMI
format is unchanged.
Type checking follows the same design as the function case (and OCaml
5.x): type_let introduces the locally abstract types into scope via
type_newtype, types the body against the constraint, and unifies with the
pattern's polymorphic type. This preserves the semantics of the former
desugaring.
The frozen v0 PPX bridge expands the field back into the historical
wrapper-chain encoding and recognizes well-formed instances of that
encoding on the way in, verified by unit tests. A v0 Pexp_newtype chain
that cannot be represented - such as one that does not enclose ReScript's
Function$ encoding, or one whose structure was changed by a PPX - now
becomes a located ocaml.error extension with an explicit message. This is
the only intentional reduction in accepted v0 PPX output.
Formatter bug fix covered by syntax fixtures: a trailing comment between
the constraint type and [=] is no longer dropped. An end-to-end GADT test
checks that refinement still works with the new binding field.
Signed-Off-By: Cristiano Calcagno <cristianoc@users.noreply.github.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Function labels in Outcometree were strings, with optionality encoded by a leading question mark. That forced printers to decode the spelling and left downstream consumers without the label structure already known by the type system. Store Noloc.arg_label directly on Otyp_arrow, update both printers to match it exhaustively, and remove the unproduced Octy_arrow constructor. The doc generator previously walked Types.type_expr independently and flattened every reachable constructor into one list. Nested arrows became outer parameters, tuples and type variables disappeared, labels and optionality were lost, and non-function values acquired fabricated zero-parameter signatures. Build details from the normalized Outcometree instead: parameters retain their metadata, constructors, variables, tuples, and functions form recursive nodes, uncommon forms remain visible through a rendered fallback, and only top-level arrows receive signature details. Update the published RescriptTools.Docgen types and snapshots for the intentionally breaking JSON shape, and correct the implementation's stale alias tag to match the signature tag declared by its interface. The documentation site drops value details before publishing its data, but third-party consumers of rescript-tools doc need the changelog warning. Focused fixtures cover labeled and optional parameters, generic variables, callbacks, tuple returns, returned functions, fallback rendering, and non-function values. Compiler, tools, analysis, syntax, roundtrip, and full test suites remain green. Signed-off-by: Cristiano Calcagno <cristianoc@users.noreply.github.com>
1713694 to
17d8b22
Compare
Note
Stack tracking PR. All ten items of the series are now extracted — review nothing here. Items 1–7 have landed on master (#8559, #8561, #8563, #8566, #8568, #8569, #8570); items 8–10 are in review as the native stack #8574 → #8575 → #8576, which this PR's diff mirrors exactly (GitHub does not allow retargeting a PR to a base that empties it). The diff shrinks as those PRs merge; once all three land this PR will be closed as complete, remaining as the series index.
Functions are n-ary, arity is structural
This branch completes the transition started by uncurried-by-default: functions and arrow types are now n-ary at every level of the compiler — parsetree, typedtree, and
Types— and a function's arity is the length of its parameter list rather than anint optionannotation on the head of a curried chain. Locally abstract types follow the same principle: a function's newtypes are a field on the function node, andlet f: type a. t = eis a structural field on the binding, replacing the wrapper-node encodings.Why. The curried encoding survived only in the frontend, inherited from OCaml, and every layer paid for it: arity lived in three places that could disagree (the term, the type, and the backend's
.cmj), and some 30 hand-rolled "walk the chain until the arity marker" loops existed across the compiler, genType, reanalyze, and the editor tooling. Where they disagreed, the results were real bugs (see below). With structural arity, "declared arity = runtime arity" holds by construction, and the machinery that existed to enforce, compensate for, or work around the old encoding is deleted rather than maintained.Benefits
Soundness and correctness fixes (each found by the refactor, each pinned by a test):
:>coercion ignored arity, so a curriedint => int => intcould satisfy(int, int) => int; a first-class use of such a value then miscompiled (typedint, runtime closure). Now a compile error with a dedicated message.(~x=d, y) => (~z=d, w) => ...deferredx's default to the inner application).~x: int => string(unparenthesized) printed identically to(~x: int) => stringbut did not unify with it.@res.asyncattributes, dropped arrow/awaitattributes (one crashing the formatter with a stack overflow), lost JSX closing tags,assert falseon PPX-emitted OCaml-stylefunction.Formatter fidelity (from the structural newtypes):
typeparameter group:(@attr type t, x, @attr2 type s, y)round-trips as written instead of printing@attr @attr2on the function.type a.constraint and=is no longer dropped.Correct docgen output: the structured
detailproduced byrescript-tools docwas corrupt — nested arrows flattened into extra parameters, labels and optionality were discarded, tuples and type variables vanished and shifted the parameter/return split, and non-functions got fabricated zero-parameter signatures. Details are now built from the same normalized outcome tree as the printed signature: parameters carrylabel/optional, types are recursive nodes (constructor/variable/tuple/function/rendered), and only functions get signature details. This is a breaking change to the publishedRescriptTools.DocgenJSON schema (see Risks).Better generated code:
(param => {...})(a)(b)); these are gone (seemutable_uncurry_test.mjs).Primitive_module.init/updateruntime bootstrap (seerec_module_test.mjs). The bootstrap remains where it is load-bearing.Pjs_fn_makewrapper acted as an accidental optimization barrier: early Lambda passes saw anLprimwhere a function was. With it gone, user variable names survive more often and constants propagate (e.g.param_0/param_1become the user'su/v).$staropt_dir$starinstead of$staropt$star$1) in the rare unprettified case.Better error messages: arity mismatches report precise unlabelled-argument counts; missing-argument lists print in source order; the confusing "This labeled function is applied to arguments in an order different from other calls" restriction is gone (labels commute for inferred functions too, soundly).
Less compiler, with test coverage added. Deleted outright: the parsetree arity annotation and
ast_uncurried.ml;push_defaults; thePjs_fn_make/Pjs_fn_make_unitprimitives and the 230-lineunsafe_adjust_to_arity; the gather-until-arity walkers in genType (×2), reanalyze (×2), the outcome printer, and the editor tooling; the unreachableToo_many_argumentserror and its?in_functionplumbing; the parser/printer mirrored@as-arity hacks; thePexp_newtype/Texp_newtypewrapper encoding, the parser'swrap_type_annotationdouble-type dance, and the'?'-in-string label smuggling inOtyp_arrow(plus the deadOcty_arrow).Better tooling output: signature help no longer includes the opening paren in the first parameter's range; genType recovers real parameter names after defaulted parameters; reanalyze stops emitting spurious empty optional-argument references.
Risks
detailJSON emitted byrescript-tools docand the publishedRescriptTools.Docgentypes changed shape (the old shape was unusable — see Benefits). The documentation site does not consumedetail; third-party consumers must adapt.Pexp_newtyperemoved from the current parsetree, a v0 locally-abstract-type wrapper that the bridge cannot represent (e.g. PPX-synthesizedfun (type a) ->with no arity wrapper, or atype a.molecule a PPX perturbed) becomes a locatedocaml.errorextension with an explicit message, instead of passing through. Compiler-produced shapes round-trip exactly (unit-tested, including the diagnostic).@this this => async arg => ...now means what it says (a method returning an async function) instead of the old chain-walk absorbing the nested lambda's parameter into the method. Relatedly, an attribute written in front of a type-first arrow (@this (type t, x) => ...) now lands on the function node and takes effect; it previously sat inert on a wrapper node.I023, cmt magic isT024; clean builds are required, and cmt-consuming tools must be rebuilt in lockstep (all in-tree consumers are updated here).type_functionandtype_applicationin typecore andtransl_functionin translcore. Mitigations: generated JS is byte-identical across the stdlib and the test corpus except for the deliberate improvements listed above; the full suites (syntax round-trip, super_errors, build tests, gentype, analysis, tools, ounit) pass at every commit; an adversarial corpus covers label commutation, optional inference, partial application, and the reject-side of every closed soundness hole._res.arrow_node_attrsand_res.newtype_attrsmarkers appear when a node's attribute split must survive the single v0 attribute slot;Has_arityNnow always equals the arrow-chain length (previously not true for@as-phantom externals); and the synthesized newtype/constraint wrapper nodes carry slightly different location values than the old parser produced (structure and attributes are exact; verified by loading both wires through the same frontend).make test-rewatchis red on master itself (the vendoredsuryuses the removedJsnamespace) — unrelated to this branch.PR series
Each commit builds and passes the full suite independently. Extraction proceeds bottom-up; as each PR below merges, this PR's base moves down the stack and its diff shrinks accordingly. (Item numbers are stable — cross-references like "rides with 5" refer to them.)
Merged:
Enforce function arity in type inclusion and coercion #8559 — Enforce function arity in inclusion, type equality, and coercion (the soundness fix)
Harden the Parsetree0 PPX bridge #8561 — Harden the Parsetree0 PPX bridge and add a round-trip corpus (bug fixes + the safety net for 4)
Record written arrow arity before external lowering #8563 — Record written arrow arity before external lowering (removes the
@asarity fudge and its printer compensation)Make functions and arrow types n-ary in the parsetree #8566 — Make functions and arrow types n-ary in the parsetree (typed layers untouched)
Make the typed layers n-ary: Tarrow params and Texp_function params #8568 — Make the typed layers n-ary (
Tarrow/Texp_functionparams; cmt+cmi bump; downstream tools adapt in lockstep)Remove dead code enabled by structural arity #8569 — Remove dead code enabled by structural arity (also deletes
Ast_compatible)Eliminate Pjs_fn_make, Pjs_fn_make_unit, and unsafe_adjust_to_arity #8570 — Eliminate
Pjs_fn_make,Pjs_fn_make_unit, andunsafe_adjust_to_arity(contains the recursive-module rationale)In review (native stack #8574 → #8575 → #8576, lands bottom-up):
Pexp_newtype/Texp_newtype, bumps cmt toT024, contains the PPX-surface narrowing)Otyp_arrowlabels + the docgen schema change)Of the items originally deferred here: docgen precision and structured
Otyp_arrowlabels landed as commit 10; per-parameter newtypes resolved into commits 8–9 after design review (front-hoisting is intentional normalization, so newtypes became structural fields rather than positional parameters, mirroring what OCaml 5.1/5.2 did withPvc_constraintandtype_newtype); optionality-as-a-parameter-field was analyzed and declined — the churn outweighs the payoff, and the v0 wire keepsOptionallabels regardless.🤖 Generated with Claude Code