Skip to content

Comments

refactor(ast, isolated_declarations, transformer): mark AstBuilder::copy as an unsafe function#4907

Merged
graphite-app[bot] merged 1 commit intomainfrom
08-14-refactor_ast_isolated_declarations_transformer_mark_astbuilder_copy_as_an_unsafe_function
Aug 15, 2024
Merged

refactor(ast, isolated_declarations, transformer): mark AstBuilder::copy as an unsafe function#4907
graphite-app[bot] merged 1 commit intomainfrom
08-14-refactor_ast_isolated_declarations_transformer_mark_astbuilder_copy_as_an_unsafe_function

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented Aug 14, 2024

AstBuilder::copy is completely unsound (#3483), and we need to remove it. Make it an unsafe function to discourage any further usage of it in meantime.

@github-actions github-actions bot added A-ast Area - AST A-transformer Area - Transformer / Transpiler A-isolated-declarations Isolated Declarations labels Aug 14, 2024
Copy link
Member Author

overlookmotel commented Aug 14, 2024

@overlookmotel overlookmotel marked this pull request as ready for review August 14, 2024 18:54
@overlookmotel
Copy link
Member Author

That's an hour of my life I'll never get back! 😭

@codspeed-hq
Copy link

codspeed-hq bot commented Aug 14, 2024

CodSpeed Performance Report

Merging #4907 will not alter performance

Comparing 08-14-refactor_ast_isolated_declarations_transformer_mark_astbuilder_copy_as_an_unsafe_function (1eb59d2) with main (46fb3cb)

Summary

✅ 29 untouched benchmarks

@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Aug 15, 2024
@graphite-app
Copy link
Contributor

graphite-app bot commented Aug 15, 2024

Merge activity

  • Aug 14, 10:56 PM EDT: The merge label 'merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Aug 14, 10:56 PM EDT: Boshen added this pull request to the Graphite merge queue.
  • Aug 14, 11:01 PM EDT: The Graphite merge queue couldn't merge this PR because it was not satisfying all requirements (Failed CI: 'Spell Check').
  • Aug 15, 10:15 AM EDT: The merge label 'merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Aug 15, 10:15 AM EDT: overlookmotel added this pull request to the Graphite merge queue.
  • Aug 15, 10:18 AM EDT: The Graphite merge queue couldn't merge this PR because it was not satisfying all requirements (Failed CI: 'Test (ubuntu-latest)', 'Test (macos-14)').
  • Aug 15, 11:23 AM EDT: The merge label 'merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Aug 15, 11:26 AM EDT: overlookmotel added this pull request to the Graphite merge queue.
  • Aug 15, 11:26 AM EDT: overlookmotel merged this pull request with the Graphite merge queue.

Boshen pushed a commit that referenced this pull request Aug 15, 2024
…copy` as an unsafe function (#4907)

`AstBuilder::copy` is completely unsound (#3483), and we need to remove it. Make it an `unsafe` function to discourage any further usage of it in meantime.
@Boshen Boshen force-pushed the 08-14-refactor_ast_isolated_declarations_transformer_mark_astbuilder_copy_as_an_unsafe_function branch from 77b36d7 to 7f54375 Compare August 15, 2024 03:00
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Aug 15, 2024
overlookmotel added a commit that referenced this pull request Aug 15, 2024
…copy` as an unsafe function (#4907)

`AstBuilder::copy` is completely unsound (#3483), and we need to remove it. Make it an `unsafe` function to discourage any further usage of it in meantime.
@overlookmotel overlookmotel force-pushed the 08-14-refactor_ast_isolated_declarations_transformer_mark_astbuilder_copy_as_an_unsafe_function branch from 5e8e448 to 6756644 Compare August 15, 2024 13:16
@overlookmotel overlookmotel changed the base branch from main to 08-15-fix_transformer_remove_an_ast.copy_from_nullishcoalescingoperator_transform August 15, 2024 13:16
@overlookmotel overlookmotel changed the base branch from 08-15-fix_transformer_remove_an_ast.copy_from_nullishcoalescingoperator_transform to graphite-base/4907 August 15, 2024 13:21
overlookmotel added a commit that referenced this pull request Aug 15, 2024
…copy` as an unsafe function (#4907)

`AstBuilder::copy` is completely unsound (#3483), and we need to remove it. Make it an `unsafe` function to discourage any further usage of it in meantime.
@overlookmotel overlookmotel force-pushed the 08-14-refactor_ast_isolated_declarations_transformer_mark_astbuilder_copy_as_an_unsafe_function branch from 6756644 to 229e38d Compare August 15, 2024 13:24
@overlookmotel overlookmotel changed the base branch from graphite-base/4907 to main August 15, 2024 13:25
@overlookmotel overlookmotel added the 0-merge Merge with Graphite Merge Queue label Aug 15, 2024
overlookmotel added a commit that referenced this pull request Aug 15, 2024
…copy` as an unsafe function (#4907)

`AstBuilder::copy` is completely unsound (#3483), and we need to remove it. Make it an `unsafe` function to discourage any further usage of it in meantime.
@overlookmotel overlookmotel force-pushed the 08-14-refactor_ast_isolated_declarations_transformer_mark_astbuilder_copy_as_an_unsafe_function branch from 229e38d to 1970059 Compare August 15, 2024 14:15
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Aug 15, 2024
…copy` as an unsafe function (#4907)

`AstBuilder::copy` is completely unsound (#3483), and we need to remove it. Make it an `unsafe` function to discourage any further usage of it in meantime.
@overlookmotel overlookmotel force-pushed the 08-14-refactor_ast_isolated_declarations_transformer_mark_astbuilder_copy_as_an_unsafe_function branch from 1970059 to 1eb59d2 Compare August 15, 2024 15:22
@overlookmotel overlookmotel added the 0-merge Merge with Graphite Merge Queue label Aug 15, 2024
@overlookmotel
Copy link
Member Author

Third time attempting to merge! Keeps failing each time due to changes on main. Third time lucky?

@graphite-app graphite-app bot merged commit 1eb59d2 into main Aug 15, 2024
@graphite-app graphite-app bot deleted the 08-14-refactor_ast_isolated_declarations_transformer_mark_astbuilder_copy_as_an_unsafe_function branch August 15, 2024 15:26
@oxc-bot oxc-bot mentioned this pull request Aug 18, 2024
Boshen added a commit that referenced this pull request Aug 18, 2024
## [0.24.3] - 2024-08-18

### Features

- d49fb16 oxc_codegen: Support generate range leading comments (#4898)
(IWANABETHATGUY)
- 80d0d1f semantic: Check for invalid interface heritage clauses (#4928)
(DonIsaac)
- 48821c0 semantic,syntax: Add SymbolFlags::ArrowFunction (#4946)
(DonIsaac)
- f1fcdde transformer: Support react fast refresh (#4587) (Dunqing)
- 0d79122 transformer: Support logical-assignment-operators plugin
(#4890) (Dunqing)
- ab1d08c transformer: Support `optional-catch-binding` plugin (#4885)
(Dunqing)
- 69da9fd transformer: Support nullish-coalescing-operator plugin
(#4884) (Dunqing)
- 3a66e58 transformer: Support exponentiation operator plugin (#4876)
(Dunqing)
- f88cbcd transformer: Add `BoundIdentifier::new_uid_in_current_scope`
method (#4903) (overlookmotel)
- 1e6d0fe transformer: Add methods to `BoundIdentifier` (#4897)
(overlookmotel)
- fd34640 traverse: Support `generate_uid_based_on_node` method in
`TraverseCtx` (#4940) (Dunqing)
- 72a37fc traverse: Support `clone_identifier_reference` method in
`TraverseCtx` (#4880) (Dunqing)

### Bug Fixes

- c0b26f4 ast: Do not include `scope_id` fields in JSON AST (#4858)
(overlookmotel)
- bbf9ec0 codegen: Add missing `declare` to `PropertyDefinition` (#4937)
(Boshen)
- f210cf7 codegen: Print `TSSatisfiesExpression` and
`TSInstantiationExpression` (#4936) (Boshen)
- 21f5762 codegen: Minify large numbers (#4889) (Boshen)
- e8de4bd codegen: Fix whitespace issue when minifying `x in new
Error()` (#4886) (Boshen)
- a226962 codegen: Print `TSNonNullExpression` (#4869) (Boshen)
- 3da33d3 codegen: Missing parenthesis for `PrivateInExpression` (#4865)
(Boshen)
- 1808529 codegen: Dedupe pure annotation comments (#4862)
(IWANABETHATGUY)
- d3bbc62 isolated-declarations: Declare modifier of PropertyDefinition
should not be retained (#4941) (Dunqing)
- 8e80f59 isolated_declarations: Class properties should still be lifted
from private constructors (#4934) (michaelm)
- b3ec9e5 isolated_declarations: Always emit module declarations that
perform augmentation (#4919) (michaelm)
- 0fb0b71 isolated_declarations: Always emit module declarations (#4911)
(michaelm)
- 4a16916 isolated_declarations: Support expando functions (#4910)
(michaelm)
- 508644a linter/tree-shaking: Correct the calculation of `>>`, `<<` and
`>>>` (#4932) (mysteryven)
- 46cb1c1 minifier: Handle `Object.definedPropert(exports` for
@babel/types/lib/index.js (#4933) (Boshen)
- 81fd637 minifier: Do not fold `0 && (module.exports = {})` for
`cjs-module-lexer` (#4878) (Boshen)
- 879a271 minifier: Do not join `require` calls for `cjs-module-lexer`
(#4875) (Boshen)
- 1bdde2c parser: Detect @flow in `/** @flow */ comment (#4861) (Boshen)
- 2476dce transformer: Remove an `ast.copy` from
`NullishCoalescingOperator` transform (#4913) (overlookmotel)
- 248a757 transformer/typescript: Typescript syntax within
`SimpleAssignmentTarget` with `MemberExpressions` is not stripped
(#4920) (Dunqing)

### Documentation

- 47c9552 ast, ast_macros, ast_tools: Better documentation for `Ast`
helper attributes. (#4856) (rzvxa)
- 0a01a47 semantic: Improve documentation (#4850) (DonIsaac)
- 9c700ed transformer: Add README including style guide (#4899)
(overlookmotel)

### Refactor

- a6967b3 allocator: Correct code comment (#4904) (overlookmotel)
- 90d0b2b allocator, ast, span, ast_tools: Use `allocator` as var name
for `Allocator` (#4900) (overlookmotel)
- 1eb59d2 ast, isolated_declarations, transformer: Mark
`AstBuilder::copy` as an unsafe function (#4907) (overlookmotel)
- 8e8fcd0 ast_tools: Rename `oxc_ast_codegen` to `oxc_ast_tools`.
(#4846) (rzvxa)
- 786bf07 index: Shorten code and correct comment (#4905)
(overlookmotel)
- ea1e64a semantic: Make SemanticBuilder opaque (#4851) (DonIsaac)
- 5fd1701 sourcemap: Lower the `msrv`. (#4873) (rzvxa)
- 48a1c32 syntax: Inline trivial bitflags methods (#4877)
(overlookmotel)
- 452187a transformer: Rename `BoundIdentifier::new_uid_in_root_scope`
(#4902) (overlookmotel)
- 707a01f transformer: Re-order `BoundIdentifier` methods (#4896)
(overlookmotel)
- 117dff2 transformer: Improve comments for `BoundIdentifier` helper
(#4895) (overlookmotel)

Co-authored-by: Boshen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

0-merge Merge with Graphite Merge Queue A-ast Area - AST A-isolated-declarations Isolated Declarations A-transformer Area - Transformer / Transpiler

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants