refactor(allocator,formatter): consolidate borrow-passthrough into reusable alloc_cow_str#23528
refactor(allocator,formatter): consolidate borrow-passthrough into reusable alloc_cow_str#23528hyf0 wants to merge 1 commit into
Conversation
Merging this PR will not alter performance
Comparing 
Footnotes
|
There was a problem hiding this comment.
For oxc_allocator change, I'll leave review to others... (seems fine though), but a couple of things noticed:
- it's missing a
# Panicsdoc - uses
#[inline]instead of#[inline(always)]like the sibling?
For formatter part, if we're extracting this into oxc_allocator as a reusable helper, the same pattern elsewhere should be migrated too?
e.g. oxc_formatter_json already has a local arena_cow_str doing exactly this.
cea172c to
ed846f3
Compare
alloc_cow_str for the borrow-passthrough pattern
I split the pure improvement PR into #23534. I left the arena refactor change in this PR so we could focus on the question. |
ed846f3 to
b511b9b
Compare
|
Thanks @leaysgur! Addressed everything, and split the PR along your two points:
Left the |
leaysgur
left a comment
There was a problem hiding this comment.
LGTM for formatter changes. 👌🏻
…usable alloc_cow_str Adds Allocator::alloc_cow_str(&cow): returns a Cow's &str without copying when Cow::Borrowed, copying into the arena only for Cow::Owned. Crystallizes the borrow-passthrough pattern (string oxc-project#23465, number oxc-project#23512, bigint oxc-project#23534) into one documented, reusable helper instead of the inline match repeated at each site. Takes the Cow by shared reference so it also works where the Cow lives behind a &self borrow (string literals) and can't be moved out. Migrates every in-tree site to it: - oxc_formatter string / numeric / bigint literals - oxc_formatter_json: removes the crate-local arena_cow_str wrapper; call sites now use the shared helper directly (f.allocator().alloc_cow_str(&cow)), matching how the formatters already call f.allocator().alloc_str(...) directly with no per-crate wrapper. Behaviour- and allocation-neutral (pure consolidation); allocs snapshots unchanged.
b511b9b to
471de46
Compare
Closes #23528. (I will confirm this (should be lived in `oxc_allocator` side?) later, then refactor all callsites)
What
Introduces
Allocator::alloc_cow_str(&cow)— returns aCow's&strwithout copying when it isCow::Borrowed, copying into the arena only forCow::Owned. Crystallizes the borrow-passthrough pattern (string #23465, number #23512, bigint #23534) into one documented, reusable helper instead of an inlinematchrepeated at each site.It takes the
Cowby shared reference so it also works where theCowlives behind a&selfborrow (string literals) and can't be moved out — letting every in-tree site use it.Pure consolidation: behavior- and allocation-neutral (the bigint alloc win landed separately in #23534; allocation snapshots are unchanged here).
Migrated sites (all of them)
oxc_formatterstring literals (CleanedStringLiteralText::fmt)oxc_formatternumeric literals (CleanedNumberLiteralText::fmt)oxc_formatterbigint literalsoxc_formatter_json: removed the crate-localarena_cow_strwrapper; its call sites now use the shared helper directly (f.allocator().alloc_cow_str(&cow)) — matching how both formatters already callf.allocator().alloc_str(...)directly, with no per-crate wrapper.Addresses review (@leaysgur)
# Panicsdoc.#[inline(always)](+#[expect(clippy::inline_always)]) to match the siblingalloc_str.oxc_formatter_json) — and every remaining inline occurrence of the pattern.Prepared with AI assistance.