refactor(allocator): Vec construction methods take &GetAllocator#23755
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via 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. |
Merging this PR will not alter performance
Comparing Footnotes
|
There was a problem hiding this comment.
Pull request overview
Refactors oxc_allocator::Vec construction APIs (new_in, with_capacity_in, from_iter_in, from_array_in, etc.) to accept &impl GetAllocator<'a> instead of &'a Allocator, enabling callers to pass common context types (e.g. parser/traverse/formatter contexts) directly. The PR updates call sites across the workspace to match the new signatures (often requiring &&Allocator when only an &Allocator is available).
Changes:
- Generalize arena
Vecconstructors to take&A where A: GetAllocator<'a>and update docs/tests accordingly. - Update many call sites to pass
&context(wherecontext: GetAllocator) or&allocator(whereallocator: &Allocator) as required. - Add
GetAllocatorimpl forPatternParserand adjustnapiraw-transfer code to pass allocator references correctly.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| napi/parser/src/raw_transfer.rs | Adjust allocator handling and update downstream calls to match new GetAllocator-based APIs. |
| napi/parser/src/raw_transfer_types.rs | Update arena vec allocations to pass &&Allocator where needed. |
| crates/oxc_syntax/src/module_record.rs | Update ModuleRecord::new arena vec initializations for new constructor signatures. |
| crates/oxc_semantic/src/scoping.rs | Update arena vec creation sites to pass &&Allocator and keep behavior unchanged. |
| crates/oxc_regular_expression/src/parser/pattern_parser/pattern_parser_impl.rs | Switch to ArenaVec::new_in(self) and add GetAllocator impl for PatternParser. |
| crates/oxc_react_compiler/src/lib.rs | Update comment preservation arena vec allocation call for new constructor signature. |
| crates/oxc_parser/src/module_record.rs | Update module record builder arena vec usage for new constructor signature. |
| crates/oxc_parser/src/lib.rs | Use ArenaVec::new_in(&self.ast) in panic path, leveraging AstBuilder: GetAllocator. |
| crates/oxc_parser/src/lexer/trivia_builder.rs | Update comments vec initialization to pass &&Allocator. |
| crates/oxc_parser/src/lexer/mod.rs | Update token vec initialization/replacement to pass &&Allocator. |
| crates/oxc_mangler/src/lib.rs | Update multiple arena vec allocations to pass &&Allocator. |
| crates/oxc_linter/src/rules/react/exhaustive_deps.rs | Use &ast_builder as allocator accessor for ArenaVec::from_iter_in. |
| crates/oxc_linter/src/rules/import/consistent_type_specifier_style.rs | Use &ast_builder as allocator accessor for generated import specifiers vecs. |
| crates/oxc_linter/src/lib.rs | Update allocator usage for external rules path; introduces a guard-lifetime bug that must be fixed. |
| crates/oxc_formatter/src/print/jsx/child_list.rs | Use formatter (f) as allocator accessor and update &&Allocator call sites. |
| crates/oxc_formatter/src/print/call_like_expression/arguments.rs | Use formatter (f) as allocator accessor for from_array_in. |
| crates/oxc_formatter/src/ir_transform/sort_imports/mod.rs | Update with_capacity_in to pass &&Allocator. |
| crates/oxc_formatter/src/ast_nodes/node.rs | Update ArenaVec::new_in call to pass &&Allocator. |
| crates/oxc_formatter_json/src/parse.rs | Update mem::replace(..., ArenaVec::new_in(...)) patterns to pass &&Allocator. |
| crates/oxc_formatter_core/src/builders.rs | Use formatter (f) as allocator accessor for from_iter_in. |
| crates/oxc_formatter_core/src/buffer.rs | Use FormatState as allocator accessor where possible; update remaining sites to &&Allocator. |
| crates/oxc_ast/src/ast_builder_impl.rs | Route AstBuilder convenience methods through ArenaVec constructors using &self (GetAllocator). |
| crates/oxc_allocator/src/vec.rs | Core API change: make constructors accept &impl GetAllocator<'a>; update examples/tests accordingly. |
| crates/oxc_allocator/src/take_in.rs | Update dummy Vec construction to pass &&Allocator. |
| crates/oxc_allocator/src/clone_in.rs | Update internal vec allocations to pass &&Allocator; adjust tests accordingly. |
| crates/oxc_allocator/src/boxed.rs | Update tests to pass &&Allocator via let allocator = &allocator;. |
| crates/oxc_allocator/src/allocator.rs | Update docs to reflect the new &&Allocator calling pattern. |
5650ee7 to
0294e4e
Compare
Merge activity
|
…23755) Part of #23043. `Vec::new_in`, `Vec::with_capacity_in`, `Vec::from_array_in` etc previously took an `&'a Allocator`. Instead make them take an `&A where A: GetAllocator<'a>`. This makes them more flexible - you can pass a reference to any type which implements `GetAllocator` e.g. `ParserImpl`, `TraverseCtx` etc. ```rust // Before (`ctx` here is `&mut TraverseCtx`) let vec = ArenaVec::new_in(ctx.ast.allocator); // After let vec = ArenaVec::new_in(ctx); ``` The main motivation is to make them workable replacements for what are currently methods on `AstBuilder`, but will be removed in the new `AstBuilder`: ```rust // Before (`ctx` here is `&mut TraverseCtx`) let vec = ctx.ast.vec(); // After let vec = ArenaVec::new_in(ctx); // Instead of let vec = ArenaVec::new_in(ctx.ast.allocator); ``` This is still longer code than the original, which is not ideal, but it's as short as can make them. The main downside of this change is that when calling these methods with an `&Allocator`, you have to double reference: ```rust let allocator: &'a Allocator = get_allocator_somehow(); // Passing `&&Allocator` let vec = ArenaVec::new_in(&allocator); ``` This has no perf impact - these methods are inlined, and compiler collapses the double-reference like it wasn't there, but it's just ugly. Unfortunately, there's no way to implement `GetAllocator` so it'll accept both `&Allocator` and `&mut TraverseCtx`, and the latter is the much more common use case.
0294e4e to
5201522
Compare
Part of #23043. Same as #23755, but for `Box`. `Box::new_in` previously took an `&'a Allocator`. Instead make it take an `&A where A: GetAllocator<'a>`. This makes it more flexible - you can pass a reference to any type which implements `GetAllocator` e.g. `ParserImpl`, `TraverseCtx` etc. ```rust // Before (`ctx` here is `&mut TraverseCtx`) let boxed = Box::new_in(value, ctx.ast.allocator); // After let boxed = Box::new_in(value, ctx); ``` The main motivation is to make it a workable replacement for `AstBuilder::alloc`, which will be removed in the new `AstBuilder`: ```rust // Before (`ctx` here is `&mut TraverseCtx`) let boxed = ctx.ast.alloc(value); // After let boxed = ArenaBox::new_in(value, ctx); // Instead of let boxed = ArenaBox::new_in(value, ctx.ast.allocator); ``` This has the same advantages and disadvantages as `Vec` methods - see #23755.
…23755) Part of #23043. `Vec::new_in`, `Vec::with_capacity_in`, `Vec::from_array_in` etc previously took an `&'a Allocator`. Instead make them take an `&A where A: GetAllocator<'a>`. This makes them more flexible - you can pass a reference to any type which implements `GetAllocator` e.g. `ParserImpl`, `TraverseCtx` etc. ```rust // Before (`ctx` here is `&mut TraverseCtx`) let vec = ArenaVec::new_in(ctx.ast.allocator); // After let vec = ArenaVec::new_in(ctx); ``` The main motivation is to make them workable replacements for what are currently methods on `AstBuilder`, but will be removed in the new `AstBuilder`: ```rust // Before (`ctx` here is `&mut TraverseCtx`) let vec = ctx.ast.vec(); // After let vec = ArenaVec::new_in(ctx); // Instead of let vec = ArenaVec::new_in(ctx.ast.allocator); ``` This is still longer code than the original, which is not ideal, but it's as short as can make them. The main downside of this change is that when calling these methods with an `&Allocator`, you have to double reference: ```rust let allocator: &'a Allocator = get_allocator_somehow(); // Passing `&&Allocator` let vec = ArenaVec::new_in(&allocator); ``` This has no perf impact - these methods are inlined, and compiler collapses the double-reference like it wasn't there, but it's just ugly. Unfortunately, there's no way to implement `GetAllocator` so it'll accept both `&Allocator` and `&mut TraverseCtx`, and the latter is the much more common use case.
Part of #23043. Same as #23755, but for `Box`. `Box::new_in` previously took an `&'a Allocator`. Instead make it take an `&A where A: GetAllocator<'a>`. This makes it more flexible - you can pass a reference to any type which implements `GetAllocator` e.g. `ParserImpl`, `TraverseCtx` etc. ```rust // Before (`ctx` here is `&mut TraverseCtx`) let boxed = Box::new_in(value, ctx.ast.allocator); // After let boxed = Box::new_in(value, ctx); ``` The main motivation is to make it a workable replacement for `AstBuilder::alloc`, which will be removed in the new `AstBuilder`: ```rust // Before (`ctx` here is `&mut TraverseCtx`) let boxed = ctx.ast.alloc(value); // After let boxed = ArenaBox::new_in(value, ctx); // Instead of let boxed = ArenaBox::new_in(value, ctx.ast.allocator); ``` This has the same advantages and disadvantages as `Vec` methods - see #23755.

Part of #23043.
Vec::new_in,Vec::with_capacity_in,Vec::from_array_inetc previously took an&'a Allocator. Instead make them take an&A where A: GetAllocator<'a>.This makes them more flexible - you can pass a reference to any type which implements
GetAllocatore.g.ParserImpl,TraverseCtxetc.The main motivation is to make them workable replacements for what are currently methods on
AstBuilder, but will be removed in the newAstBuilder:This is still longer code than the original, which is not ideal, but it's as short as can make them.
The main downside of this change is that when calling these methods with an
&Allocator, you have to double reference:This has no perf impact - these methods are inlined, and compiler collapses the double-reference like it wasn't there, but it's just ugly. Unfortunately, there's no way to implement
GetAllocatorso it'll accept both&Allocatorand&mut TraverseCtx, and the latter is the much more common use case.