Remove parser modes by encoding constructor arity explicitly - #8610
Remove parser modes by encoding constructor arity explicitly#8610cknitt wants to merge 13 commits into
Conversation
Signed-off-by: Christoph Knittel <ck@cca.io>
Signed-off-by: Christoph Knittel <ck@cca.io>
Signed-off-by: Christoph Knittel <ck@cca.io>
Signed-off-by: Christoph Knittel <ck@cca.io>
Signed-off-by: Christoph Knittel <ck@cca.io>
Signed-off-by: Christoph Knittel <ck@cca.io>
Signed-off-by: Christoph Knittel <ck@cca.io>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Signed-off-by: Christoph Knittel <ck@cca.io>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba681ec574
ℹ️ 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".
Signed-off-by: Christoph Knittel <ck@cca.io>
Signed-off-by: Christoph Knittel <ck@cca.io>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #8610 +/- ##
==========================================
+ Coverage 77.32% 77.43% +0.10%
==========================================
Files 467 467
Lines 63342 63491 +149
==========================================
+ Hits 48982 49166 +184
+ Misses 14360 14325 -35
🚀 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: |
Signed-off-by: Christoph Knittel <ck@cca.io>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b374049de4
ℹ️ 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".
Signed-off-by: Christoph Knittel <ck@cca.io>
Signed-off-by: Christoph Knittel <ck@cca.io>
Motivation
The ReScript parser previously supported two modes:
These modes allowed the parser to produce different trees depending on whether its output was intended for the type checker or the printer.
In particular, the type-checker representation collapsed the distinction between multiple constructor arguments and a tuple passed as a single constructor argument. The printer needed a different, source-preserving representation so it could retain the extra parentheses.
The parser mode was also used for differences in string-literal representation. #8606 normalized string literals across the compiler and was the first step toward removing the mode entirely. Constructor arity was the main remaining reason for it.
This change
Constructor arity is now represented explicitly in the parsetree:
The same approach is used for polymorphic variant expressions, patterns, and type payload groups.
For example:
Their applications have unambiguous parsetree representations:
The type checker can therefore compare the number of syntactic arguments directly with the constructor's declared arity.
This also means that a constructor with one tuple payload must use the corresponding nested parentheses:
Writing
Some(x, y)now correctly reports an arity mismatch.Alignment with subsequent compiler layers
The new representation is more closely aligned with subsequent compiler layers. Nominal constructors are already represented as argument lists in the typedtree, so the type checker no longer needs to infer their arity by inspecting tuple nodes.
Polymorphic variants continue to be normalized to their semantic single-payload representation when entering the type checker. Their source-level arity remains explicit in the parsetree for printing and tooling.
There is therefore no need to change the typedtree or later compiler representations.
Cleanup enabled by the new representation
With both source shape and constructor arity available in one parsetree, this PR removes:
ParseForTypeCheckerandDefaultfrom the parserfor_printerargument throughout parser APIs and their callers-typecheckerparser flagIt also:
ocaml.explicit_arityhandling to the AST0 compatibility boundaryPPX compatibility
The frozen
parsetree0representation remains unchanged.When converting multiple constructor arguments to AST0, the bridge encodes them as a tuple and attaches internal
_res.constructor_argsmetadata. Conversion back uses that metadata to restore the argument list and removes the internal attribute before returning to the current parsetree.This preserves the distinction between multiple arguments and a single tuple argument across PPX round trips without requiring changes to the frozen PPX-facing AST.
Breaking change
Code that relied on the previous ambiguity must add explicit tuple parentheses.
For example:
must become:
when the constructor has one tuple payload.
Testing
The change includes coverage for: