Skip to content

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented Oct 23, 2024

Follow-up after #6805.

TraverseCtx::generate_binding take an Atom instead of a CompactStr. If it's an existing var name (which it must be, otherwise we'd be using TraverseCtx::generate_uid), an Atom will already exist in the arena for this symbol name, so we'd be better off reusing it, rather than writing the same Atom into the arena again.

@graphite-app
Copy link
Contributor

graphite-app bot commented Oct 23, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “0-merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

Copy link
Member Author

overlookmotel commented Oct 23, 2024

@github-actions github-actions bot added A-transformer Area - Transformer / Transpiler C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior labels Oct 23, 2024
@overlookmotel overlookmotel marked this pull request as ready for review October 23, 2024 16:09
@overlookmotel overlookmotel requested a review from Dunqing October 23, 2024 16:15
@codspeed-hq
Copy link

codspeed-hq bot commented Oct 23, 2024

CodSpeed Performance Report

Merging #6830 will not alter performance

Comparing 10-23-refactor_traverse_traversectx_generate_binding_take_an_atom_ (9b33fca) with main (2aa763c)

Summary

✅ 30 untouched benchmarks

@graphite-app
Copy link
Contributor

graphite-app bot commented Oct 24, 2024

Merge activity

…6830)

Follow-up after #6805.

`TraverseCtx::generate_binding` take an `Atom` instead of a `CompactStr`. If it's an existing var name (which it must be, otherwise we'd be using `TraverseCtx::generate_uid`), an `Atom` will already exist in the arena for this symbol name, so we'd be better off reusing it, rather than writing the same `Atom` into the arena again.
@Dunqing Dunqing force-pushed the 10-23-refactor_traverse_traversectx_generate_binding_take_an_atom_ branch from 9b33fca to 60f487a Compare October 24, 2024 02:47
@graphite-app graphite-app bot merged commit 60f487a into main Oct 24, 2024
@graphite-app graphite-app bot deleted the 10-23-refactor_traverse_traversectx_generate_binding_take_an_atom_ branch October 24, 2024 02:51
Boshen pushed a commit that referenced this pull request Oct 24, 2024
Follow-up after #6830.

Now that `generate_binding` doesn't need access to `AstBuilder`, it can move into `TraverseScoping`.

Occasionally this is helpful when you need to borrows 2 parts of `TraverseCtx` simultaneously. e.g.:

```rs
// This fails to compile because we try to mutably borrow `TraverseCtx`
// while it's already borrowed
if let Ancestor::ProgramBody(body) = ctx.parent() {
    //                               ^^^ immutable borrow
    let binding = ctx.generate_binding(name, scope_id, flags);
    //            ^^^ mutable borrow
}

// Borrow-checker allows this because we borrow fields of `TraverseCtx` separately
if let Ancestor::ProgramBody(body) = ctx.ancestry.parent() {
    let binding = ctx.scoping.generate_binding(name, scope_id, flags);
}
```
Boshen added a commit that referenced this pull request Oct 26, 2024
## [0.34.0] - 2024-10-26

- 4618aa2 transformer: [**BREAKING**] Rename `TransformerOptions::react`
to `jsx` (#6888) (Boshen)

- 90c786c regular_expression: [**BREAKING**] Support ES2025 Duplicated
named capture groups (#6847) (leaysgur)

- 67a7bde napi/parser: [**BREAKING**] Add typings to napi/parser (#6796)
(ottomated)

### Features

- 1145341 ast_tools: Output typescript to a separate package (#6755)
(ottomated)
- 4429754 ecmascript: Constant eval `null` to number (#6879) (Boshen)
- fd57e00 ecmascript: Add abstract_relational_comparison to dce (#6846)
(Boshen)
- 8bcaf59 minifier: Late peeophole optimization (#6882) (Boshen)
- 860cbca minifier: Implement folding simple arrow fns (#6875) (camc314)
- c26020e minifier: Implement folding String.prototype.replaceAll
(#6871) (camc314)
- 50744f3 minifier: Implement folding String.prototype.replace (#6870)
(camc314)
- fccf82e minifier: Implement folding `substring` string fns (#6869)
(camc314)
- e6a5a1b minifier: Implement folding `charCodeAt` string fns (#6475)
(camc314)
- 0d0bb17 transformer: Complete the async-to-generator plugin (#6658)
(Dunqing)
- 419343b traverse: Implement `GetAddress` for `Ancestor` (#6877)
(overlookmotel)

### Bug Fixes

- a47c70e minifier: Fix remaining runtime bugs (#6855) (Boshen)
- 686727f minifier: Reference read has side effect (#6851) (Boshen)
- c658d93 minifier: Keep template literals with expressions (#6849)
(Boshen)
- 4dc5e51 transformer: Only run typescript plugin for typescript source
(#6889) (Boshen)
- 076f5c3 transformer/typescript: Retain ExportNamedDeclaration without
specifiers and declaration (#6848) (Dunqing)
- b075982 types: Change @oxc/types package name (#6874) (ottomated)

### Documentation

- 6eeb0e6 ast: Mention typescript-eslint, field ordering and shape
(#6863) (Boshen)
- 99e3b32 napi: Remove JSON.parse from example (#6836) (ottomated)

### Refactor

- adb5039 allocator: Add `impl GetAddress for Address` (#6891)
(overlookmotel)
- 3e7507f ast_tools: Reduce macro usage (#6895) (overlookmotel)
- 423d54c rust: Remove the annoying `clippy::wildcard_imports` (#6860)
(Boshen)
- 2d95009 transformer: Implement `Debug` on `StatementInjector` internal
types (#6886) (overlookmotel)
- c383c34 transformer: Make `StatementInjectorStore` methods generic
over `GetAddress` (#6885) (overlookmotel)
- 1f29523 transformer: Rename ReactJsx to Jsx (#6883) (Boshen)
- 333b758 transformer: `StatementInjectorStore` methods take
`&Statement` as target (#6858) (overlookmotel)
- c19996c transformer: Add `StatementInjectorStore::insert_many_before`
method (#6857) (overlookmotel)
- 7339dde transformer: `StatementInjectorStore::insert_many_after` take
an iterator (#6856) (overlookmotel)
- 4348eae transformer/typescript: Re-order visitor methods (#6864)
(overlookmotel)
- 3a56d59 transformer/typescript: Insert assignments after super by
`StatementInjector` (#6654) (Dunqing)
- a366fae traverse: Rename
`TraverseScoping::generate_binding_in_current_scope` (#6832)
(overlookmotel)
- 3b99fe6 traverse: Move `generate_binding` to `TraverseScoping` (#6831)
(overlookmotel)
- 60f487a traverse: `TraverseCtx::generate_binding` take an `Atom`
(#6830) (overlookmotel)

### Styling

- 262b2ed ast: Move crate doc comment to top of file (#6890)
(overlookmotel)

---------

Co-authored-by: Boshen <[email protected]>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Orenbek pushed a commit to Orenbek/oxc that referenced this pull request Oct 28, 2024
## [0.34.0] - 2024-10-26

- 4618aa2 transformer: [**BREAKING**] Rename `TransformerOptions::react`
to `jsx` (oxc-project#6888) (Boshen)

- 90c786c regular_expression: [**BREAKING**] Support ES2025 Duplicated
named capture groups (oxc-project#6847) (leaysgur)

- 67a7bde napi/parser: [**BREAKING**] Add typings to napi/parser (oxc-project#6796)
(ottomated)

### Features

- 1145341 ast_tools: Output typescript to a separate package (oxc-project#6755)
(ottomated)
- 4429754 ecmascript: Constant eval `null` to number (oxc-project#6879) (Boshen)
- fd57e00 ecmascript: Add abstract_relational_comparison to dce (oxc-project#6846)
(Boshen)
- 8bcaf59 minifier: Late peeophole optimization (oxc-project#6882) (Boshen)
- 860cbca minifier: Implement folding simple arrow fns (oxc-project#6875) (camc314)
- c26020e minifier: Implement folding String.prototype.replaceAll
(oxc-project#6871) (camc314)
- 50744f3 minifier: Implement folding String.prototype.replace (oxc-project#6870)
(camc314)
- fccf82e minifier: Implement folding `substring` string fns (oxc-project#6869)
(camc314)
- e6a5a1b minifier: Implement folding `charCodeAt` string fns (oxc-project#6475)
(camc314)
- 0d0bb17 transformer: Complete the async-to-generator plugin (oxc-project#6658)
(Dunqing)
- 419343b traverse: Implement `GetAddress` for `Ancestor` (oxc-project#6877)
(overlookmotel)

### Bug Fixes

- a47c70e minifier: Fix remaining runtime bugs (oxc-project#6855) (Boshen)
- 686727f minifier: Reference read has side effect (oxc-project#6851) (Boshen)
- c658d93 minifier: Keep template literals with expressions (oxc-project#6849)
(Boshen)
- 4dc5e51 transformer: Only run typescript plugin for typescript source
(oxc-project#6889) (Boshen)
- 076f5c3 transformer/typescript: Retain ExportNamedDeclaration without
specifiers and declaration (oxc-project#6848) (Dunqing)
- b075982 types: Change @oxc/types package name (oxc-project#6874) (ottomated)

### Documentation

- 6eeb0e6 ast: Mention typescript-eslint, field ordering and shape
(oxc-project#6863) (Boshen)
- 99e3b32 napi: Remove JSON.parse from example (oxc-project#6836) (ottomated)

### Refactor

- adb5039 allocator: Add `impl GetAddress for Address` (oxc-project#6891)
(overlookmotel)
- 3e7507f ast_tools: Reduce macro usage (oxc-project#6895) (overlookmotel)
- 423d54c rust: Remove the annoying `clippy::wildcard_imports` (oxc-project#6860)
(Boshen)
- 2d95009 transformer: Implement `Debug` on `StatementInjector` internal
types (oxc-project#6886) (overlookmotel)
- c383c34 transformer: Make `StatementInjectorStore` methods generic
over `GetAddress` (oxc-project#6885) (overlookmotel)
- 1f29523 transformer: Rename ReactJsx to Jsx (oxc-project#6883) (Boshen)
- 333b758 transformer: `StatementInjectorStore` methods take
`&Statement` as target (oxc-project#6858) (overlookmotel)
- c19996c transformer: Add `StatementInjectorStore::insert_many_before`
method (oxc-project#6857) (overlookmotel)
- 7339dde transformer: `StatementInjectorStore::insert_many_after` take
an iterator (oxc-project#6856) (overlookmotel)
- 4348eae transformer/typescript: Re-order visitor methods (oxc-project#6864)
(overlookmotel)
- 3a56d59 transformer/typescript: Insert assignments after super by
`StatementInjector` (oxc-project#6654) (Dunqing)
- a366fae traverse: Rename
`TraverseScoping::generate_binding_in_current_scope` (oxc-project#6832)
(overlookmotel)
- 3b99fe6 traverse: Move `generate_binding` to `TraverseScoping` (oxc-project#6831)
(overlookmotel)
- 60f487a traverse: `TraverseCtx::generate_binding` take an `Atom`
(oxc-project#6830) (overlookmotel)

### Styling

- 262b2ed ast: Move crate doc comment to top of file (oxc-project#6890)
(overlookmotel)

---------

Co-authored-by: Boshen <[email protected]>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-transformer Area - Transformer / Transpiler C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants