Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(codegen): converts line comment to block comment if it is a PURE comment #6356

Conversation

Dunqing
Copy link
Member

@Dunqing Dunqing commented Oct 8, 2024

The last part of fixing https://github.com/rolldown/rolldown/pull/2375/files#r1789011257

In the following case, the pure comment was written by //

const Component = // #__PURE__
React.forwardRef((props, ref) => {});

The printed code looks like this

const Component = 
// #__PURE__ React.forwardRef((props, ref) => {});

As you can see, it is broken because the code also commented, so we need to replace // with /* */

Copy link

graphite-app bot commented Oct 8, 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.

@github-actions github-actions bot added the A-codegen Area - Code Generation label Oct 8, 2024
Copy link
Member Author

Dunqing commented Oct 8, 2024

Copy link

codspeed-hq bot commented Oct 8, 2024

CodSpeed Performance Report

Merging #6356 will not alter performance

Comparing 10-08-fix_codegen_converts_line_comment_to_block_comment_if_it_is_a_pure_comment (84b2d07) with main (62d45bf)

Summary

✅ 29 untouched benchmarks

@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Oct 8, 2024 — with Graphite App
Copy link

graphite-app bot commented Oct 8, 2024

Merge activity

…` comment (#6356)

The last part of fixing https://github.com/rolldown/rolldown/pull/2375/files#r1789011257

In the following case, the pure comment was written by `//`
```ts
const Component = // #__PURE__
React.forwardRef((props, ref) => {});
```

The printed code looks like this
```ts
const Component =
// #__PURE__ React.forwardRef((props, ref) => {});
```

As you can see, it is broken because the code also commented, so we need to replace `//` with `/* */`
@Boshen Boshen force-pushed the 10-08-fix_parser_should_regard_comments_where_after_as_leading_comments_next_token branch from d1c680b to 1380d8b Compare October 8, 2024 05:42
@Boshen Boshen force-pushed the 10-08-fix_codegen_converts_line_comment_to_block_comment_if_it_is_a_pure_comment branch from 3c5c6b6 to 84b2d07 Compare October 8, 2024 05:43
Base automatically changed from 10-08-fix_parser_should_regard_comments_where_after_as_leading_comments_next_token to main October 8, 2024 05:47
@graphite-app graphite-app bot merged commit 84b2d07 into main Oct 8, 2024
27 checks passed
@graphite-app graphite-app bot deleted the 10-08-fix_codegen_converts_line_comment_to_block_comment_if_it_is_a_pure_comment branch October 8, 2024 05:48
@oxc-bot oxc-bot mentioned this pull request Oct 8, 2024
Boshen added a commit that referenced this pull request Oct 8, 2024
## [0.31.0] - 2024-10-08

- 01b878e parser: [**BREAKING**] Use `BindingIdentifier` for `namespace`
declaration names (#6003) (DonIsaac)

- 95ca01c cfg: [**BREAKING**] Make BasicBlock::unreachable private
(#6321) (DonIsaac)

- 020bb80 codegen: [**BREAKING**] Change to `CodegenReturn::code` and
`CodegenReturn::map` (#6310) (Boshen)

- 409dffc traverse: [**BREAKING**] `generate_uid` return a
`BoundIdentifier` (#6294) (overlookmotel)

- 5a73a66 regular_expression: [**BREAKING**] Simplify public APIs
(#6262) (leaysgur)

- 32d972e parser: [**BREAKING**] Treat unambiguous files containing TS
export assignments as modules (#6253) (overlookmotel)

- 4f6bc79 transformer: [**BREAKING**] Remove `source_type` param from
`Transformer::new` (#6251) (overlookmotel)

- afc3ccb napi/transform: [**BREAKING**] Rename
`TransformOptions::react` to `jsx`. (#6211) (Boshen)

- 82ab689 transformer,minifier: [**BREAKING**] Move define and inject
plugin from minifier to transformer (#6199) (Boshen)

### Features

- fa4d505 cfg: Derive more base traits for CFG blocks (#6320) (DonIsaac)
- 14275b1 cfg: Color-code edges in CFG dot diagrams (#6314) (DonIsaac)
- 7566c2d data_structures: Add `as_slice` + `as_mut_slice` methods to
stacks (#6216) (overlookmotel)
- c3c3447 data_structures: Add `oxc_data_structures` crate; add stack
(#6206) (Boshen)
- e304e8c minifier: Minify exponential arithmetic operation. (#6281)
(7086cmd)
- f9ae70c minifier: Minify basic arithmetic calculations. (#6280)
(7086cmd)
- 4008afe minifier: Fold array and object constructors (#6257)
(camchenry)
- 115ccc9 minifier: Bitwise not in exceeded value. (#6235) (7086cmd)
- ee6c850 minifier: Scaffold peephole replace known methods. (#6245)
(7086cmd)
- c32af57 minifier: Fold demical bitwise not for bigint. (#6233)
(7086cmd)
- 23b6464 minifier: Fold true / false comparison. (#6225) (7086cmd)
- 585ccda minifier: Support subtraction assignment. (#6214) (7086cmd)
- cca0034 minifier: Handle positive `NaN` and `Infinity`. (#6207)
(7086cmd)
- dac8f09 minifier: Minify unary plus negation. (#6203) (7086cmd)
- 3b79e1b minifier: Evaluate bigint in fold constant (#6178) (Boshen)
- abd3a9f napi/transform: Perform dce after define plugin (#6312)
(Boshen)
- a0ccc26 napi/transform: Add `lang` option to change source type
(#6309) (Boshen)
- f98e12c napi/transform: Add inject plugin (#6250) (Boshen)
- 291891e napi/transform: Add `define` option (#6212) (Boshen)
- 51a78d5 napi/transform: Rename all mention of React to Jsx; remove
mention of `Binding` (#6198) (Boshen)
- 2f888ed oxc: Add napi transform options (#6268) (Boshen)
- 8729755 oxc,napi/transform: Napi/transform use oxc compiler pipeline
(#6298) (Boshen)
- f6e42b6 sourcemap: Add support for sourcemap debug IDs (#6221) (Tim
Fish)
- 9e62396 syntax_operations: Add crate `oxc_syntax_operations` (#6202)
(Boshen)
- cf20f3a transformer: Exponentiation transform: support private fields
(#6345) (overlookmotel)

### Bug Fixes

- 84b2d07 codegen: Converts line comment to block comment if it is a
`PURE` comment (#6356) (Dunqing)
- e9eeae0 isolated-declarations: False positive for function with a type
asserted parameters (#6181) (Dunqing)
- d953a6b minifier: Correct the reference link (#6283) (dalaoshu)
- 37cbabb minifier: Should not handle the strict operation for bool
comparison. (#6261) (7086cmd)
- e29c067 minifier: Handle exceeded shifts. (#6237) (7086cmd)
- 294da86 napi/transform: Fix index.d.ts (Boshen)
- 9736aa0 oxc_transformer: Define `import.meta` and `import.meta.*`
(#6277) (IWANABETHATGUY)
- 6159560 parser: String `ImportSpecifier`s for type imports (#6352)
(DonIsaac)
- 1380d8b parser: Should regard comments where after `=` as leading
comments of next token (#6355) (Dunqing)
- 2bcd12a transformer: Exponentiation transform: fix reference flags
(#6330) (overlookmotel)
- 28cbfa7 transformer: Exponentiation transform: fix temp var names
(#6329) (overlookmotel)
- 3a4bcc7 transformer: Exponentiation transform: fix temp var names
(#6318) (overlookmotel)
- ccb7bdc transformer: Exponentiation transform: do not replace object
when private property (#6313) (overlookmotel)
- 56d50cf transformer: Exponentiation transform: do not assume `Math` is
not a local var (#6302) (overlookmotel)
- bd81c51 transformer: Exponentiation transform: fix duplicate symbols
(#6300) (overlookmotel)
- 06797b6 transformer: Logical assignment operator transform: fix
reference IDs (#6289) (overlookmotel)
- 4b42047 transformer: Fix memory leak in `ReplaceGlobalDefines` (#6224)
(overlookmotel)
- a28926f transformer: Fix inserting `require` with `front` option
(#6188) (overlookmotel)
- b92fe84 transformer: `NonEmptyStack::push` write value before updating
cursor (#6169) (overlookmotel)

### Performance

- 5db9b30 allocator: Use lower bound of size hint when creating Vecs
from an iterator (#6194) (DonIsaac)
- 788e444 transformer: Parse options from comments only once (#6152)
(overlookmotel)
- da2b2a4 transformer: Look up `SymbolId` for `require` only once
(#6192) (overlookmotel)
- 40bd919 transformer: Faster parsing JSX pragmas from comments (#6151)
(overlookmotel)

### Documentation

- eb1d0b8 transformer: Exponentiation transform: update doc comments
(#6315) (overlookmotel)
- c7636d7 traverse: Remove erroneous doc comment (#6328) (overlookmotel)

### Refactor

- f7d1136 allocator: Remove unnecessary `Vec` impl (#6213)
(overlookmotel)
- 40932f7 cfg: Use IndexVec for storing basic blocks (#6323) (DonIsaac)
- a1e0d30 cfg: Add type alias for Graph (#6322) (DonIsaac)
- 7672793 cfg: Move block data types to separate file (#6319) (DonIsaac)
- cc57541 data_structures: `NonEmptyStack::len` hint that `len` is never
0 (#6220) (overlookmotel)
- 147a5d5 data_structures: Remove `is_empty` methods for non-empty
stacks (#6219) (overlookmotel)
- 61805fd data_structures: Add debug assertion to `SparseStack` (#6218)
(overlookmotel)
- dbfa0bc data_structures: Add `len` method to `StackCommon` trait
(#6215) (overlookmotel)
- ac5a23f minifier: Use ctx.ast.vec instead of Vec::new. (#6331)
(7086cmd)
- 1cee207 minifier: Some boilerplate work for PeepholeFoldConstants
(#6054) (Boshen)
- 5b5daec napi: Use vitest (#6307) (Boshen)
- 58a8615 napi/transform: Remove context (#6306) (Boshen)
- 099ff3a napi/transform: Remove "Binding" from types; fix type error
(#6260) (Boshen)
- 54c1c53 napi/transform: Remove a call on `TransformOptions::clone`
(#6210) (Boshen)
- aa0dbb6 oxc: Add `napi` feature, change napi parser to use `oxc` crate
(#6265) (Boshen)
- 3b53dd4 parser: Provide better error messages for `const` modifiers on
class elements (#6353) (DonIsaac)
- acab777 regular_expression: Misc fixes (#6234) (leaysgur)
- bdd9e92 semantic: Rename vars from `ast_node_id` to `node_id` (#6304)
(overlookmotel)
- d110700 semantic: Dereference IDs as quickly as possible (#6303)
(overlookmotel)
- 03bc041 syntax: Remove some unsafe code creating IDs (#6324)
(overlookmotel)
- bd5fb5a transformer: Exponentiation transform: rename methods (#6344)
(overlookmotel)
- 4aa4e6b transformer: Exponentiation transform: do not wrap in
`SequenceExpression` if not needed (#6343) (overlookmotel)
- a15235a transformer: Exponentiation transform: no cloning (#6338)
(overlookmotel)
- 7d93b25 transformer: Exponentiation transform: split into 2 paths
(#6316) (overlookmotel)
- 15cc8af transformer: Exponentiation transform: break up into functions
(#6301) (overlookmotel)
- 7f5a94b transformer: Use `Option::get_or_insert_with` (#6299)
(overlookmotel)
- 0cea6e9 transformer: Exponentiation transform: reduce identifier
cloning (#6297) (overlookmotel)
- ac7a3ed transformer: Logical assignment transform: reduce identifier
cloning (#6296) (overlookmotel)
- 527f7c8 transformer: Nullish coalescing transform: no cloning
identifier references (#6295) (overlookmotel)
- 7b62966 transformer: Move `BoundIdentifier` into `oxc_traverse` crate
(#6293) (overlookmotel)
- c7fbf68 transformer: Logical assignment operator transform: no cloning
identifier references (#6290) (overlookmotel)
- f0a74ca transformer: Prefer `create_bound_reference_id` to
`create_reference_id` (#6282) (overlookmotel)
- ba3e85b transformer: Fix spelling (#6279) (overlookmotel)
- bc757c8 transformer: Move functionality of common transforms into
stores (#6243) (overlookmotel)
- 1c31932 transformer: Rename var in `VarDeclarations` common transform
(#6242) (overlookmotel)
- 0400ff9 transformer: `VarDeclarations` common transform: check if at
top level with `ctx.parent()` (#6231) (overlookmotel)
- 235cdba transformer: Use AstBuilder instance from TraverseCtx (#6209)
(overlookmotel)
- a7ed29e transformer: Insert `import` statement or `require` depending
on source type (#6191) (overlookmotel)
- 4c63f0e transformer: Rename methods (#6190) (overlookmotel)
- 900cb46 transformer: Convert `ModuleImports` into common transform
(#6186) (overlookmotel)
- 00e2802 transformer: Introduce `TopLevelStatements` common transform
(#6185) (overlookmotel)
- 70d4c56 transformer: Rename `VarDeclarationsStore` methods (#6184)
(overlookmotel)
- 81be545 transformer: Export `var_declarations` module from `common`
module (#6183) (overlookmotel)
- 02fedf5 transformer: Shorten import (#6180) (overlookmotel)
- f2ac584 transformer: Use TraverseCtx's ast in ModuleImports (#6175)
(Dunqing)
- 21b08ba transformer: Shared `VarDeclarations` (#6170) (overlookmotel)
- 0dd9a2e traverse: Add helper methods to `BoundIdentifier` (#6341)
(overlookmotel)
- c0e2fef traverse: Function to get var name from node (#6317)
(overlookmotel)
- adc5381 traverse: `TraverseAncestry` use `NonEmptyStack` (#6217)
(overlookmotel)

### Testing

- 964d71e minifier: Add arithmetic tests for fold constants. (#6269)
(7086cmd)
- fcb4651 minifier: Enable null comparison with bigint. (#6252)
(7086cmd)
- d4f2ee9 transformer: Tidy up transform checker (#6287) (overlookmotel)
- 0f5afd7 transformer: Transform checker output symbol name for
mismatches (#6286) (overlookmotel)

---------

Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
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
0-merge Merge with Graphite Merge Queue A-codegen Area - Code Generation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants