Skip to content

refactor(ast): move AstKind::ty out of generated code#24390

Merged
graphite-app[bot] merged 1 commit into
mainfrom
om/07-11-refactor_ast_move_astkind_ty_out_of_generated_code
Jul 11, 2026
Merged

refactor(ast): move AstKind::ty out of generated code#24390
graphite-app[bot] merged 1 commit into
mainfrom
om/07-11-refactor_ast_move_astkind_ty_out_of_generated_code

Conversation

@overlookmotel

@overlookmotel overlookmotel commented Jul 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Follow-on after #24381. AstKind::ty is pure static code. It doesn't need to be generated. Move it out of the generated code.

@github-actions github-actions Bot added A-ast Area - AST A-ast-tools Area - AST tools labels Jul 11, 2026
@overlookmotel Graphite App

overlookmotel commented Jul 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent changes, fast-track this PR to the front of the merge queue

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@overlookmotel overlookmotel mentioned this pull request Jul 11, 2026
Merged
@overlookmotel
overlookmotel marked this pull request as ready for review July 11, 2026 18:00
Copilot AI review requested due to automatic review settings July 11, 2026 18:00
@overlookmotel overlookmotel self-assigned this Jul 11, 2026
Copilot AI reviewed Jul 11, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Refactors oxc_ast so AstKind::ty() lives in handwritten code (ast_kind_impl.rs) rather than in the generated generated/ast_kind.rs, aligning with the intent that ty() is pure static logic and doesn’t need codegen.

Changes:

  • Removed AstKind::ty() (and its NonNull import) from the AST kind generator output.
  • Removed the generated AstKind::ty() implementation from crates/oxc_ast/src/generated/ast_kind.rs.
  • Added the AstKind::ty() implementation to crates/oxc_ast/src/ast_kind_impl.rs.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.

File Description
tasks/ast_tools/src/generators/ast_kind.rs Stops generating AstKind::ty() and the NonNull import.
crates/oxc_ast/src/generated/ast_kind.rs Removes the generated ty() method and now only contains the remaining generated impls (e.g., node_id).
crates/oxc_ast/src/ast_kind_impl.rs Adds AstKind::ty() as a non-generated inherent method (including required imports).

@overlookmotel overlookmotel added the 0-merge Merge with Graphite Merge Queue label Jul 11, 2026
@overlookmotel Graphite App

overlookmotel commented Jul 11, 2026 •
edited by graphite-app Bot
Loading

Copy link
Copy Markdown
Member Author

Merge activity

@overlookmotel
refactor(ast): move AstKind::ty out of generated code (#24390)
19ad4a8 
Follow-on after #24381. `AstKind::ty` is pure static code. It doesn't need to be generated. Move it out of the generated code.
@graphite-app
graphite-app Bot force-pushed the om/07-11-refactor_ast_use_nonnull_for_ref_casting branch from 4aaaccc to 347c61b Compare July 11, 2026 18:03
@graphite-app
graphite-app Bot force-pushed the om/07-11-refactor_ast_move_astkind_ty_out_of_generated_code branch from bdd8770 to 19ad4a8 Compare July 11, 2026 18:04
@codspeed-hq

codspeed-hq Bot commented Jul 11, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 57 untouched benchmarks
⏩ 14 skipped benchmarks1

Comparing om/07-11-refactor_ast_move_astkind_ty_out_of_generated_code (bdd8770) with om/07-11-refactor_ast_use_nonnull_for_ref_casting (4aaaccc)2

Open in CodSpeed

Footnotes

  1. 14 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

  2. No successful run was found on om/07-11-refactor_ast_use_nonnull_for_ref_casting (a82a400) during the generation of this report, so b25646b was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

Base automatically changed from om/07-11-refactor_ast_use_nonnull_for_ref_casting to main July 11, 2026 18:07
@graphite-app
graphite-app Bot merged commit 19ad4a8 into main Jul 11, 2026
31 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label Jul 11, 2026
@graphite-app
graphite-app Bot deleted the om/07-11-refactor_ast_move_astkind_ty_out_of_generated_code branch July 11, 2026 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

@overlookmotel overlookmotel

Labels

A-ast Area - AST A-ast-tools Area - AST tools

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@overlookmotel