feat(transformer/statement-injector): add a series of insert_*_before/after_current_statement methods

[Dunqing]

These methods allow inserting statements before or after the current statement without needing the statement address. And can reduce the address looking up logic like

oxc/crates/oxc_transformer/src/es2022/class_properties/class.rs

Lines 434 to 440 in c35f52b

	// Insert statements before/after class
	let stmt_address = match ctx.parent() {
	parent @ (Ancestor::ExportDefaultDeclarationDeclaration(_)
	| Ancestor::ExportNamedDeclarationDeclaration(_)) => parent.address(),
	// `Class` is always stored in a `Box`, so has a stable memory location
	_ => Address::from_ptr(class),
	};

This is also an alternative to oxc-project/backlog#140.

[Dunqing]

• feat(transformer/statement-injector): add an assertion to check if it still contains statements that don't inject #9063
• fix(transformer/decorator): decoration disappear due to incorrect statement address #9062
• feat(transformer/statement-injector): add a series of insert_*_before/after_current_statement methods #9061 👈 (View in Graphite)
• main

---

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.

[codspeed-hq]

CodSpeed Performance Report

Merging #9061 will not alter performance

_{Comparing 02-12-feat_transformer_statement-injector_add_a_series_of_insert___before_after_current_statement_methods (403e9ae) with main (66cf26f)}

Summary

✅ 33 untouched benchmarks

[overlookmotel]

I'm afraid I'm not convinced this is a good idea overall.

On the positive side, as you say, it removes common repeated code - great.

But on the negative side, it's doing extra work on every statement (very hot path) to update current_statement_address. For the majority of statements, no transform ever needs to know their address, so that work is wasted.

This PR is currently showing -1% on some of the transformer benchmarks. I believe that to make it correct, it'll require a stack (see comment below) which is more expensive, so the perf impact will increase further.

I wonder if there's another way to get the nice API without the perf impact?

How about insert_before_current_statement etc taking &TraverseCtx and they search up the ancestry chain to find the current statement? Then the work would only be done when it needs to be done, rather than eagerly? Do you think something like that could work?

[overlookmotel]

I don't think this can be correct. When exiting a statement, don't you need to restore the address of the parent statement? e.g.:

const x = () => {
  let y;
};

After exiting the statement let y, the current statement is const x = ... again.

I guess would need a stack to achieve this.

[Dunqing]

Ah, This big problem I've overlooked. Thanks for pointing it out

[Dunqing]

I wonder if there's another way to get the nice API without the perf impact?

How about insert_before_current_statement etc taking &TraverseCtx and they search up the ancestry chain to find the current statement? Then the work would only be done when it needs to be done, rather than eagerly? Do you think something like that could work?

This is feasible in most scenarios.

I added these APIs to solve a problem when we want to do some transformation in enter_statement or exit_statement and we want to insert a statement in a deeper function. In this case, we have to pass the address all the way to where it is truly needed. Taking &TraverseCtx cannot solve the problem because we are already at the statement.

[Dunqing]

Converted to draft as it is not ideal, and I changed another way to solve #9062, please review that PR, which blocks Rolldown from advancing the legacy decorator feature.

[overlookmotel]

@Dunqing #9062 is now merged, and from our discussion today, I think we agreed that these APIs don't solve our problems. So I'm closing. But feel free to re-open if I misunderstood.