Skip to content

Record written arrow arity before external lowering - #8563

Merged
cristianoc merged 1 commit into
codex/harden-parsetree0-bridgefrom
codex/parsed-arrow-arity
Aug 19, 2026
Merged

Record written arrow arity before external lowering#8563
cristianoc merged 1 commit into
codex/harden-parsetree0-bridgefrom
codex/parsed-arrow-arity

Conversation

@cristianoc

Copy link
Copy Markdown
Collaborator

Summary

  • record an arrow type's written parameter count directly in the parsetree, including phantom @as(...) _ parameters on externals
  • remove the parser-side decrement and matching printer compensation
  • give bare labeled arrow types the same arity as their parenthesized spelling

Rationale

External lowering already removes phantom parameters and rebuilds the function type with its effective call arity. Encoding that lowered arity in the parser made the surface parsetree context-dependent and required the printer to reverse the adjustment. Keeping the written arity makes the parser representation consistent while preserving generated JavaScript.

Bare labeled arrow types previously printed like their parenthesized equivalent but carried different arity metadata and therefore did not unify with it.

Validation

  • make test-syntax
  • make test
  • parser snapshots cover the bare labeled spelling
  • existing external fixtures retain byte-identical generated JavaScript

@cristianoc
cristianoc force-pushed the codex/parsed-arrow-arity branch 2 times, most recently from 86a3008 to e45b706 Compare August 18, 2026 12:56
@cristianoc
cristianoc marked this pull request as ready for review August 18, 2026 13:01
@cristianoc
cristianoc requested a review from cknitt August 18, 2026 13:02

@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: e45b706218

ℹ️ 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 compiler/syntax/src/res_core.ml
@codecov

codecov Bot commented Aug 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 75.83%. Comparing base (cbd02f1) to head (f138a72).

Files with missing lines Patch % Lines
compiler/syntax/src/res_core.ml 85.71% 1 Missing ⚠️
Additional details and impacted files
@@                        Coverage Diff                         @@
##           codex/harden-parsetree0-bridge    #8563      +/-   ##
==================================================================
- Coverage                           75.84%   75.83%   -0.01%     
==================================================================
  Files                                 476      476              
  Lines                               62729    62715      -14     
==================================================================
- Hits                                47575    47560      -15     
- Misses                              15154    15155       +1     
Files with missing lines Coverage Δ
compiler/syntax/src/res_parsetree_viewer.ml 90.48% <ø> (-0.13%) ⬇️
compiler/syntax/src/res_core.ml 91.53% <85.71%> (-0.05%) ⬇️
🚀 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 18, 2026

Copy link
Copy Markdown

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript-lang/rescript@8563

@rescript/belt

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/belt@8563

@rescript/darwin-arm64

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

@rescript/darwin-x64

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

@rescript/linux-arm64

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

@rescript/linux-x64

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

@rescript/runtime

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/runtime@8563

@rescript/win32-x64

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

commit: f138a72

@cknitt cknitt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice to get rid of the arity workaround for @as(json...)! 🎉

You may want to address the Codex comment before merging.

@cristianoc
cristianoc force-pushed the codex/parsed-arrow-arity branch from e45b706 to cf7ad01 Compare August 19, 2026 07:32
External arrow parsing used to subtract labelled phantom @as(...) _ parameters from the head arity, while the printer added them back. No downstream consumer needs that parser-level encoding: external lowering removes phantom parameters and rebuilds the type with the effective call arity.

Record the written parameter count in the parsetree instead and remove the parser and printer compensation. Also assign arity 1 to bare labelled arrow types so they agree with their parenthesized spelling.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Cristiano Calcagno <cristianoc@users.noreply.github.com>
@cristianoc
cristianoc merged commit 110534a into master Aug 19, 2026
29 of 30 checks passed
@cristianoc
cristianoc deleted the codex/parsed-arrow-arity branch August 19, 2026 09:21
@cristianoc
cristianoc restored the codex/parsed-arrow-arity branch August 19, 2026 10:00
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