refactor(transformer, minifier): replace AstBuilder::move_xxxx methods with TakeIn trait

[Dunqing]

Pure refactor, TakeIn trait is exactly used for these cases, we don't need an indirect call for move_xxx.

[Dunqing]

• feat(ast)!: remove all AstBuilder::move_xxxx methods #10171
• refactor(transformer, minifier): replace AstBuilder::move_xxxx methods with TakeIn trait #10170 👈 (View in Graphite)
• feat(allocator): add TakeIn::take_in_box method #10169
• 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 Instrumentation Performance Report

Merging #10170 will create unknown performance changes

_{Comparing 04-02-refactor_transformer_minifier_replace_astbuilder_move_xxxx_methods_with_takein_trait (bcdbd38) with main (c198578)}

Summary

🆕 36 new benchmarks

Benchmarks breakdown

	Benchmark	BASE	HEAD	Change
🆕	codegen[checker.ts]	N/A	22.1 ms	N/A
🆕	codegen_sourcemap[checker.ts]	N/A	65.1 ms	N/A
🆕	formatter[antd.js]	N/A	7.9 ms	N/A
🆕	formatter[react.development.js]	N/A	43.5 µs	N/A
🆕	formatter[typescript.js]	N/A	7.2 ms	N/A
🆕	isolated-declarations[vue-id.ts]	N/A	58.2 ms	N/A
🆕	lexer[RadixUIAdoptionSection.jsx]	N/A	21.4 µs	N/A
🆕	lexer[antd.js]	N/A	24.8 ms	N/A
🆕	lexer[cal.com.tsx]	N/A	5.9 ms	N/A
🆕	lexer[checker.ts]	N/A	14.8 ms	N/A
🆕	lexer[pdf.mjs]	N/A	3.9 ms	N/A
🆕	linter[RadixUIAdoptionSection.jsx]	N/A	2.7 ms	N/A
🆕	linter[cal.com.tsx]	N/A	1.2 s	N/A
🆕	linter[checker.ts]	N/A	3 s	N/A
🆕	mangler[antd.js]	N/A	15.9 ms	N/A
🆕	mangler[react.development.js]	N/A	293.1 µs	N/A
🆕	mangler[typescript.js]	N/A	39.4 ms	N/A
🆕	minifier[antd.js]	N/A	166.3 ms	N/A
🆕	minifier[react.development.js]	N/A	1.9 ms	N/A
🆕	minifier[typescript.js]	N/A	293.3 ms	N/A
...	...	...	...	...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

[overlookmotel]

I did this in #9970 but then decided against it, for the reasons given in #10001. I think take_in makes it too easy to pull large types onto the stack, when you could likely find a way to use move_expression or move_statement instead (which are much cheaper).

I'm not dead set on that position if you strongly disagree, but that's my overall feeling.

[Boshen]

I did this in #9970 but then decided against it, for the reasons given in #10001. I think take_in makes it too easy to pull large types onto the stack, when you could likely find a way to use move_expression or move_statement instead (which are much cheaper).

I'm not dead set on that position if you strongly disagree, but that's my overall feeling.

I think we should have one way of doing things. Making decisions on TakeIn or move_xxx is really confusing ...

[graphite-app]

Merge activity

• Apr 4, 3:54 AM EDT: Boshen added this pull request to the Graphite merge queue.
• Apr 4, 4:12 AM EDT: Merged by the Graphite merge queue.

[overlookmotel]

@Dunqing Should we add an AstBuilder::take method, so you don't have to use TakeIn explicitly?

- declaration.take_in(self.ast.allocator)
+ self.ast.take(declaration)

I don't mind either way, just asking what you think.

[Dunqing]

@Dunqing Should we add an AstBuilder::take method, so you don't have to use TakeIn explicitly?

- declaration.take_in(self.ast.allocator)
+ self.ast.take(declaration)

I don't mind either way, just asking what you think.

Yes, this would be easier to use, I don't have opinion on this.

[overlookmotel]

OK, I opened an issue: #10314