Migrate typed tree to use AST with locs for fn arguments #7873
Conversation
…s in the typed tree as well
| innerFunctionDefinition.kind | ||
| |> List.map (fun (entry : Kind.entry) -> | ||
| ( Asttypes.Noloc.Labelled entry.label, | ||
| ( Asttypes.Labelled {txt = entry.label; loc = Location.none}, |
There was a problem hiding this comment.
I've assumed that tracking the loc is unimportant here since reanalyze worked well before this refactor, and didn't have locs then.
| if labelDecl.ld_optional then Asttypes.Noloc.Optional lblName | ||
| else Labelled lblName | ||
| if labelDecl.ld_optional then | ||
| Asttypes.Optional {txt = lblName; loc = Location.none} | ||
| else Asttypes.Labelled {txt = lblName; loc = Location.none} |
There was a problem hiding this comment.
Same here, I've assumed tracking the loc is not important here. We could however trivially add labelDecl.ld_loc here if we wanted to.
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the function argument handling in the typed tree representation from using location-stripped argument labels (Noloc.arg_label) to using AST nodes with locations (arg_label). This change aligns the typed tree with the parse tree representation and lays groundwork for future refactors.
Key changes:
- Replace
Noloc.arg_labelwitharg_labelthroughout the typed tree - Update pattern matching from string-based to record-based field access
- Remove the
to_nolocconversion function calls
Reviewed Changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| compiler/ml/typetexp.ml | Remove conversion to noloc for function argument labels |
| compiler/ml/types.mli | Update type definition to use located arg_label |
| compiler/ml/types.ml | Update type definition to use located arg_label |
| compiler/ml/typedtree.mli | Update function and argument type definitions |
| compiler/ml/typedtree.ml | Update function and argument type definitions |
| compiler/ml/typecore.mli | Update error type definitions |
| compiler/ml/typecore.ml | Update error handling and pattern matching |
| compiler/ml/printtyped.ml | Update argument label printing |
| compiler/ml/printtyp.mli | Update function signature |
| compiler/ml/printtyp.ml | Update label string conversion |
| compiler/ml/ctype.mli | Update filter_arrow signature |
| compiler/ml/ctype.ml | Update type comparison functions |
| compiler/ml/btype.mli | Consolidate utility functions |
| compiler/ml/btype.ml | Remove duplicate utility functions |
| compiler/ml/asttypes.ml | Remove Noloc-specific comparison function |
| compiler/gentype/*.ml | Update gentype translation functions |
| analysis/src/*.ml | Update analysis functions for new label format |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
I guess |
This migrates fn arguments to use the AST nodes with locations in the typed tree as well. It was previously just used for the parsetree.
Lays the foundations for some other refactors as well.