Skip to content

refactor(ast)!: remove unnecessary AstBuilder::alloc_* methods#10267

Merged
graphite-app[bot] merged 1 commit intomainfrom
04-06-refactor_ast_remove_unnecessary_astbuilder_alloc__methods
Apr 6, 2025
Merged

refactor(ast)!: remove unnecessary AstBuilder::alloc_* methods#10267
graphite-app[bot] merged 1 commit intomainfrom
04-06-refactor_ast_remove_unnecessary_astbuilder_alloc__methods

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented Apr 6, 2025

Various AST types are never boxed e.g. Box<BindingPattern> does not appear anywhere in AST.

Remove AstBuilder::alloc_* methods for these types. They're unnecessary, as you never need to construct a Box<T> for these types.

This removes 600 lines from ast_builder.rs.

Copy link
Member Author

overlookmotel commented Apr 6, 2025


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 hot fixes, skip the queue and merge this PR next

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.

@github-actions github-actions bot added A-ast Area - AST A-ast-tools Area - AST tools C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior labels Apr 6, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Apr 6, 2025

CodSpeed Instrumentation Performance Report

Merging #10267 will degrade performances by 3.42%

Comparing 04-06-refactor_ast_remove_unnecessary_astbuilder_alloc__methods (5ad41c9) with main (aaafdbf)

Summary

⚡ 1 improvements
❌ 1 regressions
✅ 34 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
formatter[antd.js] 7.9 ms 7.6 ms +3.9%
formatter[typescript.js] 7.2 ms 7.4 ms -3.42%

@overlookmotel overlookmotel marked this pull request as ready for review April 6, 2025 13:54
@overlookmotel overlookmotel requested a review from Boshen April 6, 2025 13:55
@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Apr 6, 2025
Copy link
Member

Boshen commented Apr 6, 2025

Merge activity

)

Various AST types are never boxed e.g. `Box<BindingPattern>` does not appear anywhere in AST.

Remove `AstBuilder::alloc_*` methods for these types. They're unnecessary, as you never need to construct a `Box<T>` for these types.

This removes 600 lines from `ast_builder.rs`.
@graphite-app graphite-app bot force-pushed the 04-06-feat_ast_tools_record_typeid_of_containers_on_types branch from 52e65e1 to 5b47a9f Compare April 6, 2025 14:57
@graphite-app graphite-app bot force-pushed the 04-06-refactor_ast_remove_unnecessary_astbuilder_alloc__methods branch from 3f97971 to 5ad41c9 Compare April 6, 2025 14:58
Base automatically changed from 04-06-feat_ast_tools_record_typeid_of_containers_on_types to main April 6, 2025 15:05
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Apr 6, 2025
@graphite-app graphite-app bot merged commit 5ad41c9 into main Apr 6, 2025
27 checks passed
@graphite-app graphite-app bot deleted the 04-06-refactor_ast_remove_unnecessary_astbuilder_alloc__methods branch April 6, 2025 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ast Area - AST A-ast-tools Area - AST tools C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants