Skip to content

Allow labeled arguments in any order for inferred function types - #8547

Merged
cristianoc merged 1 commit into
masterfrom
remove-label-commutation-lock
Aug 14, 2026
Merged

Allow labeled arguments in any order for inferred function types#8547
cristianoc merged 1 commit into
masterfrom
remove-label-commutation-lock

Conversation

@cristianoc

Copy link
Copy Markdown
Collaborator

Removes the legacy commutable flag from Tarrow, together with its plumbing (commu_repr, set_commu, copy_commu, the Ccommu undo-log entry) and the Incoherent_label_order error.

Previously, arrows created while inferring a function's type from its call sites were marked Cunknown, and the argument-commuting loop in type_args refused to run on them. So a lambda parameter g first called as g(~a=1, ~b=2) could not later be called as g(~b=3, ~a=4):

let f = g => (g(~a=1, ~b=2), g(~b=3, ~a=4))
// error: This labeled function is applied to arguments
// in an order different from other calls. ...

Now labeled arguments commute at every call site regardless of whether the function type was declared or inferred.

Why this is sound

ReScript compiles labeled arguments positionally, in the order the parameters appear in the function's type. Reordering stays safe because:

  1. Each function value has exactly one arrow order — the first call site fixes the inferred arrow order (and arity, via the existing force_tvar path).
  2. Unification never commutes labelsunify3 still requires same_arg_label position by position, so any concrete function passed in must have been defined with its parameters in exactly the inferred order. Passing (~b, ~a) => … where (~a, ~b) => 'r was inferred remains a type error.
  3. Every call site normalizes to the type's ordertype_args walks the type's arrows and picks the matching syntactic argument by name, so the generated JS always passes arguments in parameter order (g(~b=3, ~a=4) compiles to g(4, 3)).

The commutable flag guarded principality (the inferred type of g depends on which call is typed first), not soundness — and this inference path already isn't principal in ReScript, since the arity itself is fixed by whichever total application is typed first. The lock was protecting a property the language had already given up, at the cost of a confusing error.

🤖 Generated with Claude Code

Remove the legacy commutable flag from Tarrow, together with its
plumbing (commu_repr, set_commu, copy_commu, the Ccommu undo-log
entry) and the Incoherent_label_order error. Labeled arguments now
commute at every call site regardless of whether the function type
was declared or inferred; unification still requires labels in
matching order, so call sites and callees always agree on the
positional argument order in the generated JavaScript.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Cristiano Calcagno <cristianoc@users.noreply.github.com>
@cristianoc
cristianoc force-pushed the remove-label-commutation-lock branch from c7dc72e to a7c4c90 Compare August 14, 2026 10:59
@cristianoc
cristianoc requested review from cknitt and zth August 14, 2026 11:00

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c7dc72ed5c

ℹ️ 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".

Comment thread CHANGELOG.md Outdated
@codecov

codecov Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.00000% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.83%. Comparing base (11a25f4) to head (a7c4c90).

Files with missing lines Patch % Lines
compiler/ml/ctype.ml 61.53% 5 Missing ⚠️
analysis/src/type_utils.ml 63.63% 4 Missing ⚠️
compiler/ml/printtyp.ml 50.00% 2 Missing ⚠️
compiler/ml/record_type_spread.ml 0.00% 2 Missing ⚠️
compiler/ml/typecore.ml 88.88% 2 Missing ⚠️
compiler/ml/typedecl.ml 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8547      +/-   ##
==========================================
- Coverage   74.83%   74.83%   -0.01%     
==========================================
  Files         477      477              
  Lines       63313    63284      -29     
==========================================
- Hits        47382    47359      -23     
+ Misses      15931    15925       -6     
Files with missing lines Coverage Δ
analysis/reanalyze/src/dead_optional_args.ml 92.00% <100.00%> (ø)
analysis/src/completion_back_end.ml 81.72% <100.00%> (ø)
analysis/src/completion_jsx.ml 75.00% <100.00%> (ø)
analysis/src/create_interface.ml 86.95% <100.00%> (ø)
analysis/src/shared.ml 67.44% <100.00%> (ø)
compiler/gentype/translate_type_expr_from_types.ml 89.09% <100.00%> (ø)
compiler/ml/btype.ml 82.44% <100.00%> (-0.08%) ⬇️
compiler/ml/translcore.ml 89.67% <100.00%> (ø)
compiler/ml/typeopt.ml 78.72% <100.00%> (ø)
compiler/ml/types.ml 66.66% <ø> (ø)
... and 8 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pkg-pr-new

pkg-pr-new Bot commented Aug 14, 2026

Copy link
Copy Markdown

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript@8547

@rescript/darwin-arm64

npm i https://pkg.pr.new/@rescript/darwin-arm64@8547

@rescript/darwin-x64

npm i https://pkg.pr.new/@rescript/darwin-x64@8547

@rescript/linux-arm64

npm i https://pkg.pr.new/@rescript/linux-arm64@8547

@rescript/linux-x64

npm i https://pkg.pr.new/@rescript/linux-x64@8547

@rescript/runtime

npm i https://pkg.pr.new/@rescript/runtime@8547

@rescript/win32-x64

npm i https://pkg.pr.new/@rescript/win32-x64@8547

commit: a7c4c90

@github-actions

Copy link
Copy Markdown

@cristianoc
cristianoc merged commit d96662e into master Aug 14, 2026
29 checks passed
@cristianoc
cristianoc deleted the remove-label-commutation-lock branch August 14, 2026 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants