Make functions and arrow types n-ary in the parsetree - #8566
Conversation
dfbd748 to
270640e
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: dfbd748cd2
ℹ️ 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".
There was a problem hiding this comment.
Two intended changes here. The range shifts (e.g. [0, 7] → [4, 7]) are the first parameter's highlight no longer swallowing the opening paren, and the parameter list in the debug header no longer renders a stray ( and ~ prefixes. The new makeAdder case at the bottom pins the other fix: signature help no longer walks into the result type, so a function of type (int, int) => int => int lists two parameters, not three.
There was a problem hiding this comment.
Only debug-trace lines change here: the old curried encoding created one nested Pexp_fun node per parameter and the iterator logged a "Found expr" line for each; the n-ary node is visited once. No completion results change in this file.
There was a problem hiding this comment.
Same as CompletionInferValues.res.txt: one synthetic chain-node debug line gone, no result changes.
There was a problem hiding this comment.
New regression case (see the fixture comment): with the cursor in the second parameter's pattern, the first parameter is no longer pattern-completed, so its comma recovery can't shadow the correct result — completion now targets Blah's second payload (bool).
There was a problem hiding this comment.
No dead/live verdicts change. The location diffs are record-label declarations now pointing at the label itself (e.g. 7:15 → 8:11), and the Root (annotated) listing reorders because the underlying iteration is now over one parameter list instead of a nested chain.
There was a problem hiding this comment.
These parsing dumps print the recovered AST in ML syntax, so the n-ary node renders as fun [arity:3]a b c -> instead of a chain of single-parameter funs. Same reason for the other parsing/**/expected files in this PR — the dump format follows the representation; the parsed programs are unchanged.
There was a problem hiding this comment.
Besides the n-ary dump flattening, error-recovery functions now carry an explicit [arity:1] where they previously had none — recovery goes through the same non-empty-params Exp.fun_ as regular parsing now (one of the three latent empty-params paths fixed in this PR).
There was a problem hiding this comment.
The one deliberate generated-JS change in this PR: @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. The group-boundary ambiguity that allowed the old reading is exactly what the n-ary representation removes.
Replace the curried one-parameter-per-node encoding with n-ary nodes:
Ptyp_arrow of {params: arg list; ret: core_type}
Pexp_fun of {params: fun_param list; body: expression; async: bool}
where fun_param carries per-parameter attributes, label, default, and
pattern. The arity annotation is gone from the parsetree: a function's
arity is List.length params, unrepresentable wrong. ast_uncurried.ml is
deleted; Ast_helper.Typ.arrow and Exp.fun_ are list-first and assert
non-empty parameter lists.
The typed layers are unchanged: typetexp folds the params list into the
existing curried Tarrow/Ttyp_arrow chains (Some arity on the head, None
inside), and typecore peels parameters one at a time, reproducing the
legacy per-level type_function calls; synthesized rest-functions carry
an internal #res.fun_rest attribute consumed immediately on re-entry.
The Parsetree0 bridge re-curries on the way out (byte-identical wire
format for external PPXes) and gathers Has_arityN / res.arity groups
back into one node on the way in; bare PPX-fabricated v0 funs decode as
one-parameter functions instead of the old, mostly unusable arity-None
encoding.
Attribute contract: in-parens parameter attributes stay on the
patterns (as before); arrow-level attributes (@attr (a, b) => ...) live
on the function node; p_attrs is populated only by the PPX bridge.
Printing is byte-identical across the syntax test corpus; generated
JavaScript is byte-identical across the test suite except
UncurriedExternals.res, where `@this this => async arg => ...` now
honors the written nesting (a method returning an async function)
instead of absorbing the nested lambda's parameter into the method -
the group-boundary ambiguity this representation removes. Signature
help no longer includes the opening paren in the first parameter's
highlight range, and completion debug traces lose their synthetic
chain-node lines.
Also fixes three latent Typ.arrow-on-empty-params paths (zero-argument
externals, @deriving(accessors) zero-argument constructors in
signatures, parser error recovery) that the old arrows helper silently
absorbed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Cristiano Calcagno <cristianoc@users.noreply.github.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #8566 +/- ##
==========================================
+ Coverage 75.83% 75.86% +0.03%
==========================================
Files 476 475 -1
Lines 62715 62835 +120
==========================================
+ Hits 47560 47672 +112
- Misses 15155 15163 +8
🚀 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: |
270640e to
63be65d
Compare
|
Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-8566 |
cknitt
left a comment
cknitt
left a comment
There was a problem hiding this comment.
Looks good as far as I can tell! 👍
I also retested against a large company project - compiled fine without any output changes.
2 participants
Part of the n-ary functions series tracked in #8557 (item 4; follows the merged #8559, #8561, #8563).
What
Replace the curried one-parameter-per-node encoding in the parsetree with n-ary nodes:
where
fun_paramcarries per-parameter attributes, label, default, and pattern. The arity annotation is gone from the parsetree: a function's arity isList.length params, unrepresentable wrong.ast_uncurried.mlis deleted;Ast_helper.Typ.arrowandExp.fun_are list-first and assert non-empty parameter lists.Contract
Tarrow/Ttyp_arrowchains (Somearity on the head,Noneinside), and typecore peels parameters one at a time, reproducing the legacy per-leveltype_functioncalls; synthesized rest-functions carry an internal#res.fun_restattribute consumed immediately on re-entry.Has_arityN/res.aritygroups back into one node on the way in; bare PPX-fabricated v0 funs decode as one-parameter functions instead of the old, mostly unusable arity-Noneencoding.UncurriedExternals.res, where@this this => async arg => ...now honors the written nesting (a method returning an async function) instead of absorbing the nested lambda's parameter into the method — the group-boundary ambiguity this representation removes.@attr (a, b) => ...) live on the function node;p_attrsis populated only by the PPX bridge.Tooling fixes included (each pinned by a test)
let f: (a, b) => c => d, help onf(...)lists two parameters, not three.(Some(x), Blah(a, <cursor>)) => ...now completesBlah's payload).Also fixes three latent
Typ.arrow-on-empty-params paths (zero-argument externals,@deriving(accessors)zero-argument constructors in signatures, parser error recovery) that the old arrows helper silently absorbed.🤖 Generated with Claude Code