Make the typed layers n-ary: Tarrow params and Texp_function params - #8568
Conversation
5cdc8bc to
f18adb7
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: 5cdc8bc959
ℹ️ 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".
f18adb7 to
4c9e95f
Compare
There was a problem hiding this comment.
Declared error-message improvement: the count is now explicitly about unlabelled arguments ("takes 2 unlabelled arguments, but is called with just 1") instead of a raw total that miscounted labeled/mixed calls.
There was a problem hiding this comment.
Declared fix: the missing-arguments list now prints in source order (~b, ~c, ~d); the old order was an artifact of walking the curried chain inside-out.
There was a problem hiding this comment.
Declared genType fix: _4 → d — real parameter names are recovered after a defaulted parameter (the old gather-until-arity walk lost names past the default).
There was a problem hiding this comment.
Same mechanism as LabeledFun.gen.tsx: the positional placeholder _2 is replaced by the parameter's actual (compiler-introduced) name for the unit parameter — faithful name recovery, if less pretty.
There was a problem hiding this comment.
The declared *opt_<label>* naming in the one unprettified case: $staropt$star$1 → $staropt_dir$star. Pure renaming of optional-parameter internals; no structural change. This and uncurried_default.args.mjs are the only two .mjs files that change in this PR — everything else is byte-identical, as the commit message claims.
There was a problem hiding this comment.
This is the headline bug fix, visible in the JS: x's default is now computed in the outer function's body before the inner closure is returned, instead of being deferred to the inner application. The fixture gained a comment pinning exactly this, so any regression shows up as a snapshot diff. The xOpt/zOpt names are the same renaming as mario_game.mjs, prettified.
There was a problem hiding this comment.
Exactly six removed lines, all DeadOptionalArgs.addReferences … argNames: argNamesMaybe: with both lists empty — the declared "spurious empty optional-argument references" reanalyze no longer emits. No dead/live verdicts change; nothing is added.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8568 +/- ##
==========================================
- Coverage 75.84% 75.82% -0.02%
==========================================
Files 475 475
Lines 62835 62904 +69
==========================================
+ Hits 47655 47699 +44
- Misses 15180 15205 +25
🚀 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: |
4c9e95f to
eed5fe7
Compare
|
Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-8568 |
eed5fe7 to
63409eb
Compare
| @@ -544,15 +544,12 @@ module Find_functions_called = struct | |||
| {super with Tast_mapper.expr} | |||
|
|
|||
| let find_callees (expression : Typedtree.expression) = | |||
There was a problem hiding this comment.
Can we not remove this function?
There was a problem hiding this comment.
Done in 0196f60 — removed Find_functions_called entirely along with the full dead cascade it anchored: the never-firing progress-function arm in Extend_function_table.traverse_expr (its guard intersected an always-empty set), the now-unused ~progress_functions/~value_bindings_table parameters, and the always-empty callees field of value_bindings_table (triple → pair). Net −60 lines. The reanalyze suite, including the termination-analysis corpus, passes with zero expectation changes — confirming none of it was reachable.
There was a problem hiding this comment.
Update — your question turned out to be exactly the right one, with the opposite resolution. Digging into the history: this was never intentionally dead code. The collection guard was tightened to Texp_function {arity = None} in c37be1e (Dec 2024, "Remove difference on termination analysis tests") — a shape uncurried code never produces — which silently disabled the progress-function extension and introduced 3 false positives that the termination corpus has been carrying since (functions whose progress flows through un-annotated helpers like parseInt). The stub here was a faithful translation of that disabled state. Now restored in d980cec instead of deleted: the n-ary node has no arity to gate on, so callees are collected for every function binding — the corpus's false positives are discharged (15 → 12 issues) and parseInt & co. are back in the function table with their +Parser.next progress summaries. Declared in the PR description and CHANGELOG.
63409eb to
0196f60
Compare
Types.Tarrow carries a parameter list (Tarrow of arg list * type_expr);
Texp_function carries typed parameters {fp_lbl; fp_param; fp_pat;
fp_partial} and a body; Ttyp_arrow and Otyp_arrow follow. The arity
annotation and its int-option phantom state are gone from the compiler.
Type relations compare parameters pairwise; a length mismatch is
structural incompatibility (which also makes mcomp's arrow verdict
sound: arrows of different lengths can never unify). filter_arrow
becomes filter_arrow_n. type_function types all parameters against one
arrow, checking expected labels up front to preserve the dedicated
Abstract_wrong_label diagnostics; optional-parameter defaults desugar
to uniquified *opt_<label>* bindings stacked at the head of the body.
type_application is a single parameters-to-arguments matching loop
preserving the legacy commutation, optional auto-fill, eta-expansion
placeholder, and error-selection behavior.
translcore's push_defaults is deleted (defaults now sit in the body by
construction) and transl_function walks the parameter list, keeping the
active-pattern split. Downstream, the gather-until-arity walkers in
gentype and the outcome printer, reanalyze's two arity-corrective
helpers, and typedecl's structural arity fallback are all deleted.
The cmi and cmt magic numbers are bumped to Caml1999I023/Caml1999T023.
Generated JavaScript is byte-identical across the test suite except:
- a bug fix: defaults of optional parameters in curried functions are
now computed when their own parameter group is applied ((~x=d, y) =>
(~z=d, w) => ... no longer defers x's default to the inner
application); pinned by the uncurried_default.args snapshot;
- optional-parameter internals are named *opt_<label>* instead of
*opt* in the one unprettified case (mario_game).
Error-message improvements: method arity mismatches report unlabelled
argument counts precisely, and missing-argument lists print in source
order. Reanalyze no longer emits spurious empty optional-argument
references; genType recovers real parameter names after defaulted
parameters.
Restore the termination analysis' progress-function extension: collecting
a binding's callees had been gated (since c37be1e) on arity-less
function nodes, which uncurried code never produces and which no longer
exist at all - effectively disabling the mechanism that adds helpers
calling @progress functions to the function table, and producing false
positives for functions whose progress flows through such helpers. The
n-ary node has no arity to gate on; collect for every function binding.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Cristiano Calcagno <cristianoc@users.noreply.github.com>
0196f60 to
d980cec
Compare
2 participants
Part of the n-ary functions series tracked in #8557 (item 5; stacked on #8566 via base branch only — each PR merges independently).
What
Types.Tarrowcarries a parameter list (Tarrow of arg list * type_expr);Texp_functioncarries typed parameters{fp_lbl; fp_param; fp_pat; fp_partial}and a body;Ttyp_arrowandOtyp_arrowfollow. The arity annotation and itsint optionphantom state are gone from the compiler.Type relations compare parameters pairwise; a length mismatch is structural incompatibility (which also makes mcomp's arrow verdict sound: arrows of different lengths can never unify).
filter_arrowbecomesfilter_arrow_n.type_functiontypes all parameters against one arrow, checking expected labels up front to preserve the dedicatedAbstract_wrong_labeldiagnostics; optional-parameter defaults desugar to uniquified*opt_<label>*bindings stacked at the head of the body.type_applicationis a single parameters-to-arguments matching loop preserving the legacy commutation, optional auto-fill, eta-expansion placeholder, and error-selection behavior.translcore's
push_defaultsis deleted (defaults now sit in the body by construction) andtransl_functionwalks the parameter list, keeping the active-pattern split. Downstream, the gather-until-arity walkers in gentype and the outcome printer, reanalyze's two arity-corrective helpers, and typedecl's structural arity fallback are all deleted.The cmi and cmt magic numbers are bumped to
Caml1999I023/Caml1999T023— clean builds required; cmt-consuming tools rebuilt in lockstep in-tree.Generated JavaScript
Byte-identical across the test suite except:
(~x=d, y) => (~z=d, w) => ...no longer defersx's default to the inner application); pinned by theuncurried_default.argssnapshot.*opt_<label>*instead of*opt*in the one unprettified case (mario_game).Tooling
Termination analysis restored (pre-existing bug, fixed here since this PR must touch the code anyway): collecting a binding's callees had been gated since 2024 (
c37be1e8a) on arity-less function nodes, which uncurried code never produces — effectively disabling the mechanism that adds helpers calling@progressfunctions to the function table. The n-ary node has no arity to gate on; collection now runs for every function binding, discharging 3 false positives in the termination corpus (15 → 12 reported issues).Method arity mismatches report unlabelled argument counts precisely, and missing-argument lists print in source order. Reanalyze no longer emits spurious empty optional-argument references; genType recovers real parameter names after defaulted parameters.
🤖 Generated with Claude Code