Skip to content

refactor(allocator): Vec construction methods take &GetAllocator#23755

Merged
graphite-app[bot] merged 1 commit into
mainfrom
om/06-23-refactor_allocator_vec_construction_methods_take_getallocator_
Jun 24, 2026
Merged

refactor(allocator): Vec construction methods take &GetAllocator#23755
graphite-app[bot] merged 1 commit into
mainfrom
om/06-23-refactor_allocator_vec_construction_methods_take_getallocator_

Conversation

@overlookmotel

@overlookmotel overlookmotel commented Jun 24, 2026

Copy link
Copy Markdown
Member

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.

// 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:

// 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:

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.

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.

@github-actions github-actions Bot added A-linter Area - Linter A-parser Area - Parser A-semantic Area - Semantic A-ast Area - AST A-formatter Area - Formatter A-allocator Area - Allocator labels Jun 24, 2026
@codspeed-hq

codspeed-hq Bot commented Jun 24, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 62 untouched benchmarks
⏩ 9 skipped benchmarks1


Comparing om/06-23-refactor_allocator_vec_construction_methods_take_getallocator_ (5201522) with main (417b506)2

Open in CodSpeed

Footnotes

  1. 9 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 main (5201522) during the generation of this report, so 417b506 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@overlookmotel overlookmotel marked this pull request as ready for review June 24, 2026 11:48
Copilot AI review requested due to automatic review settings June 24, 2026 11:48

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_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 Vec constructors to take &A where A: GetAllocator<'a> and update docs/tests accordingly.
  • Update many call sites to pass &context (where context: GetAllocator) or &allocator (where allocator: &Allocator) as required.
  • Add GetAllocator impl for PatternParser and adjust napi raw-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.

Comment thread crates/oxc_linter/src/lib.rs Outdated
@overlookmotel overlookmotel self-assigned this Jun 24, 2026
@overlookmotel overlookmotel force-pushed the om/06-23-refactor_allocator_vec_construction_methods_take_getallocator_ branch from 5650ee7 to 0294e4e Compare June 24, 2026 12:04
@graphite-app

graphite-app Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

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.
@graphite-app graphite-app Bot force-pushed the om/06-23-refactor_allocator_vec_construction_methods_take_getallocator_ branch from 0294e4e to 5201522 Compare June 24, 2026 12:13
graphite-app Bot pushed a commit that referenced this pull request Jun 24, 2026
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.
@graphite-app graphite-app Bot merged commit 5201522 into main Jun 24, 2026
40 checks passed
@graphite-app graphite-app Bot deleted the om/06-23-refactor_allocator_vec_construction_methods_take_getallocator_ branch June 24, 2026 12:18
graphite-app Bot pushed a commit that referenced this pull request Jun 25, 2026
Continuation of #23767, #23756, and #23755.

Make all methods that construct `Str`s and `Ident`s take `&A where A: GetAllocator<'a>` instead of `&'a Allocator`.
camc314 pushed a commit that referenced this pull request Jul 3, 2026
…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.
camc314 pushed a commit that referenced this pull request Jul 3, 2026
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.
camc314 pushed a commit that referenced this pull request Jul 3, 2026
Continuation of #23767, #23756, and #23755.

Make all methods that construct `Str`s and `Ident`s take `&A where A: GetAllocator<'a>` instead of `&'a Allocator`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-allocator Area - Allocator A-ast Area - AST A-formatter Area - Formatter A-linter Area - Linter A-parser Area - Parser A-semantic Area - Semantic

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants