NotificationsYou must be signed in to change notification settings
Fork471
Star7.2k

Migrate typed tree to use AST with locs for fn arguments#7873

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

zth merged 3 commits intomasterfromarg-types

Oct 6, 2025

Merged

Migrate typed tree to use AST with locs for fn arguments#7873

zth merged 3 commits intomasterfromarg-types

Oct 6, 2025

Conversation

Copy link

Member

zth commentedSep 11, 2025•
edited
Loading

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.

zth commented

Sep 11, 2025

View reviewed changes

analysis/reanalyze/src/Arnold.ml

		innerFunctionDefinition.kind
		\|>List.map (fun (entry: Kind.entry) ->
		(Asttypes.Noloc.Labelled entry.label,
		(Asttypes.Labelled{txt=entry.label; loc=Location.none},

Copy link

MemberAuthor

Choose a reason for hiding this comment

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

I've assumed that tracking the loc is unimportant here since reanalyze worked well before this refactor, and didn't have locs then.

zth commented

Sep 11, 2025

View reviewed changes

analysis/src/CreateInterface.ml

Comment on lines -176 to +178

		if labelDecl.ld_optionalthenAsttypes.Noloc.Optional lblName
		elseLabelled lblName
		if labelDecl.ld_optionalthen
		Asttypes.Optional {txt= lblName; loc=Location.none}
		elseAsttypes.Labelled {txt= lblName; loc=Location.none}

Copy link

MemberAuthor

Choose a reason for hiding this comment

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

Same here, I've assumed tracking the loc is not important here. We could however trivially addlabelDecl.ld_loc here if we wanted to.

Copy link

pkg-pr-newbot commentedSep 11, 2025•
edited
Loading

Open in StackBlitz

rescript

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

@rescript/darwin-arm64

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

@rescript/darwin-x64

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

@rescript/linux-arm64

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

@rescript/linux-x64

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

@rescript/runtime

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

@rescript/win32-x64

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

commit:9649c71

zth marked this pull request as ready for review

September 11, 2025 09:12

zth requested review fromCopilot andcristianoc

September 11, 2025 09:12

CopilotAI reviewed

Sep 11, 2025

View reviewed changes

Copy link

Contributor

CopilotAI left a comment

Choose a reason for hiding this comment

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

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:

ReplaceNoloc.arg_label witharg_label throughout the typed tree
Update pattern matching from string-based to record-based field access
Remove theto_noloc conversion 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