Skip to content

refactor(allocator): Box::new_in take &GetAllocator#23756

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

refactor(allocator): Box::new_in take &GetAllocator#23756
graphite-app[bot] merged 1 commit into
mainfrom
om/06-23-refactor_allocator_box_new_in_take_getallocator_

Conversation

@overlookmotel

@overlookmotel overlookmotel commented Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

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.

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

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

@overlookmotel Graphite App

overlookmotel commented Jun 24, 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 Jun 24, 2026
Merged
@github-actions github-actions Bot added A-minifier Area - Minifier A-ast Area - AST A-ast-tools Area - AST tools A-allocator Area - Allocator labels Jun 24, 2026
@codspeed-hq

codspeed-hq Bot commented Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 62 untouched benchmarks
⏩ 9 skipped benchmarks1

Comparing om/06-23-refactor_allocator_box_new_in_take_getallocator_ (163ed51) 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 (163ed51) 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:53
Copilot AI review requested due to automatic review settings June 24, 2026 11:53
Copilot AI reviewed Jun 24, 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

Updates oxc_allocator::Box::new_in to accept a generic allocator accessor (&A where A: GetAllocator<'a>) instead of requiring a direct &'a Allocator, aligning Box construction ergonomics with the earlier Vec changes and unblocking upcoming AstBuilder API work (#23043).

Changes:

  • Refactors Box::new_in to take &GetAllocator and allocates via allocator.allocator().alloc(value).
  • Updates codegen and call sites to pass allocator accessors directly (e.g. self, ctx) or use &&Allocator when the only available value is &Allocator.
  • Adjusts docs/tests and supporting helper code (FromIn, TakeIn, CloneIn, arena Vec constructors) to the new API.

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tasks/ast_tools/src/generators/ast_builder.rs Updates generator to emit ArenaBox::new_in(..., &self) for alloc methods.
crates/oxc_regular_expression/src/parser/pattern_parser/pattern_parser_impl.rs Switches allocations to pass self (as a GetAllocator implementor).
crates/oxc_react_compiler/src/convert_ast_reverse.rs Updates a boxed allocation to pass &&Allocator (&self.allocator).
crates/oxc_minifier/src/peephole/replace_known_methods.rs Uses ctx as the allocator accessor for regex pattern boxing.
crates/oxc_ast/src/generated/ast_builder.rs Regenerates alloc methods to pass &self into ArenaBox::new_in.
crates/oxc_ast/src/ast_builder_impl.rs Updates AstBuilder::alloc to call ArenaBox::new_in(value, &self).
crates/oxc_allocator/src/vec.rs Adjusts internal Box::new_in usage to the new accessor-based signature.
crates/oxc_allocator/src/take_in.rs Updates take_in_box and Dummy for Box to pass &&Allocator.
crates/oxc_allocator/src/convert.rs Updates FromIn impls to pass &&Allocator (and adds a lint expectation).
crates/oxc_allocator/src/clone_in.rs Updates cloning helpers to pass &&Allocator.
crates/oxc_allocator/src/boxed.rs Implements the new Box::new_in<A: GetAllocator> signature and updates docs/tests accordingly.

Comment thread crates/oxc_allocator/src/convert.rs
@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
@overlookmotel overlookmotel force-pushed the om/06-23-refactor_allocator_box_new_in_take_getallocator_ branch from 2b69778 to 54bd54b Compare June 24, 2026 12:04
@graphite-app

graphite-app Bot commented Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Merge activity

@overlookmotel
refactor(allocator): Box::new_in take &GetAllocator (#23756)
163ed51 
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 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 graphite-app Bot requested a review from camc314 as a code owner June 24, 2026 12:13
@graphite-app graphite-app Bot force-pushed the om/06-23-refactor_allocator_box_new_in_take_getallocator_ branch from 54bd54b to 163ed51 Compare June 24, 2026 12:13
Base automatically changed from om/06-23-refactor_allocator_vec_construction_methods_take_getallocator_ to main June 24, 2026 12:18
@graphite-app graphite-app Bot merged commit 163ed51 into main Jun 24, 2026
42 checks passed
@graphite-app graphite-app Bot deleted the om/06-23-refactor_allocator_box_new_in_take_getallocator_ branch June 24, 2026 12:19
This was referenced Jun 24, 2026
Merged
Merged
graphite-app Bot pushed a commit that referenced this pull request Jun 25, 2026
@overlookmotel
feat(str)!: Str and Ident methods take &GetAllocator (#23781)
88f4455 
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
@overlookmotel @camc314
refactor(allocator): Box::new_in take &GetAllocator (#23756)
7b4e494 
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
@overlookmotel @camc314
feat(str)!: Str and Ident methods take &GetAllocator (#23781)
108ae34 
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

Reviewers

Copilot code review Copilot Copilot left review comments
@Dunqing Dunqing Awaiting requested review from Dunqing Dunqing is a code owner
@leaysgur leaysgur Awaiting requested review from leaysgur leaysgur is a code owner
@camc314 camc314 Awaiting requested review from camc314

Assignees

@overlookmotel overlookmotel

Labels

A-allocator Area - Allocator A-ast Area - AST A-ast-tools Area - AST tools A-minifier Area - Minifier

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@overlookmotel