Remove parser modes - #8610
Conversation
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. |
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".
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #8610 +/- ##
==========================================
+ Coverage 77.38% 77.55% +0.17%
==========================================
Files 474 475 +1
Lines 63524 63851 +327
==========================================
+ Hits 49156 49518 +362
+ Misses 14368 14333 -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: |
|
@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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbf4a97d54
ℹ️ 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".
cristianoc
left a comment
There was a problem hiding this comment.
what happens in the bridge should stat in the bridge: ast should be restored and any encoding trick should be gone and not leaked in to the type checker
Apply the normalization approach proposed by @cristianoc in PR rescript-lang#8610. Keep source argument lists for printing and resolve semantic grouping after constructor disambiguation, without legacy PPX marker handling in the type checker or printer. Signed-off-by: Christoph Knittel <ck@cca.io>
cbf4a97 to
a6d5edc
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6d5edc346
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc3063c7aa
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ed6059854
ℹ️ 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>
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>
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>
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>
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>
6d16043 to
045c62d
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 045c62d6f4
ℹ️ 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>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 531a5fd63b
ℹ️ 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>
a7a9475 to
425e204
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 425e204de8
ℹ️ 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".
| match tuple_args arg with | ||
| | Some args when split_tuple -> List.map map args | ||
| | _ -> [map arg]) |
There was a problem hiding this comment.
Preserve attributes on marked constructor payload tuples
When a v0 PPX adds an attribute to the synthesized tuple for a marked multi-argument constructor or polymorphic-variant expression/pattern, this branch unwraps the tuple by mapping only its children, silently discarding the tuple's pexp_attributes or ppat_attributes. This breaks chained PPXs that communicate through payload attributes; as with the type-payload handling, keep an attributed tuple wrapper while consuming _res.constructor_args.
AGENTS.md reference: AGENTS.md:L37-L39
Useful? React with 👍 / 👎.
Motivation
The parser previously had two modes:
These produced different trees for printing and type checking. The printer needed to preserve constructor argument grouping and parentheses, while the type-checker representation collapsed those distinctions.
The modes also handled differences in string-literal representation. [#8606](#8606) unified that representation and was the first step toward removing the modes. Constructor argument grouping was the main remaining obstacle.
This PR replaces the two representations with one source-preserving parsetree, while retaining existing source and PPX compatibility.
Source syntax and semantic arity
Constructor expressions and patterns now carry located argument lists:
The same approach applies to polymorphic variant expressions, patterns, and type payload groups.
These lists describe what was written, not the constructor’s declared arity. For example, omitting locations:
The printer can preserve either spelling. Once the type checker resolves an ordinary constructor’s declaration, it normalizes the arguments accordingly: unpacking a tuple for a multi-argument constructor or packing multiple syntactic arguments into a single tuple payload.
Existing spellings such as
Some(x, y)andSome((x, y))therefore remain supported. This does not introduce stricter arity rules or require source migration.Normalization happens within the existing type-checking flow, without an additional AST pass. Ordinary constructors continue into the typedtree as semantic argument lists; polymorphic variants retain their optional single-payload representation. No typedtree or later-layer representation changes are needed.
Argument locations
Argument lists carry their own source span, including the surrounding parentheses. Consumers no longer need to reconstruct that span from individual arguments or encode it in attributes.
This simplifies printing and comment handling and fixes constructor signature help appearing in whitespace or comments between the constructor name and its arguments. Signature help now uses the argument-list boundaries.
Cleanup
The unified representation removes:
ParseForTypeCheckerandDefaultfor_printerparameters and the developer-only-typecheckerparser flagIt also consolidates constructor argument parsing and printing, makes AST helpers accept located argument lists directly, and confines legacy explicit-arity attribute handling to the AST0 compatibility boundary.
PPX compatibility
The frozen
parsetree0representation remains unchanged.The bridge encodes multiple syntactic arguments as a tuple with internal
_res.constructor_argsmetadata. Conversion back consumes that metadata and restores the argument list. A single tuple argument needs no marker.Unmarked tuples generated by existing PPXs remain supported: their semantic grouping is resolved during type checking. Legacy
explicit_arityandocaml.explicit_arityattributes are handled at the bridge boundary.Multi-argument locations survive through the encoded tuple. Where AST0 cannot represent a separate outer argument-list span for a single payload, conversion falls back to the payload’s location.
Testing
Coverage includes constructor and polymorphic variant expressions, patterns, and type payloads; tuple grouping compatibility; arity diagnostics; AST0 round trips and legacy PPX output; argument locations and parser recovery; printing and comment preservation; signature-help boundaries; and end-to-end compilation and runtime behavior.