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

refactor(transformer): exponentiation operator: convert to match #6123

Conversation

overlookmotel
Copy link
Collaborator

@overlookmotel overlookmotel commented Sep 27, 2024

In exponentiation operator transform, the first branch handling ** was "falling through" to also run the 2nd branch which handles **=. The first branch replaces ** with Math.pow, so it can never match on the 2nd branch.

Convert it to a match instead, so it only executes one branch or the other.

Copy link

graphite-app bot commented Sep 27, 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-transformer Area - Transformer / Transpiler label Sep 27, 2024
Copy link
Collaborator Author

overlookmotel commented Sep 27, 2024

@overlookmotel overlookmotel marked this pull request as ready for review September 27, 2024 18:49
Copy link

codspeed-hq bot commented Sep 27, 2024

CodSpeed Performance Report

Merging #6123 will not alter performance

Comparing 09-27-refactor_transformer_exponentiation_operator_convert_to_match (07fe45b) with main (4387845)

Summary

✅ 29 untouched benchmarks

Copy link

graphite-app bot commented Sep 27, 2024

Merge activity

In exponentiation operator transform, the first branch handling `**` was "falling through" to also run the 2nd branch which handles `**=`. The first branch replaces `**` with `Math.pow`, so it can never match on the 2nd branch.

Convert it to a `match` instead, so it only executes one branch or the other.
@overlookmotel overlookmotel force-pushed the 09-27-refactor_transformer_exponentiation_operator_convert_to_match branch from 1700bab to 07fe45b Compare September 27, 2024 19:22
@overlookmotel overlookmotel removed the request for review from Dunqing September 27, 2024 19:22
@overlookmotel
Copy link
Collaborator Author

I didn't mean to merge this. I added merge label to PR above it in the stack. Oops. But I think it's OK...

@graphite-app graphite-app bot merged commit 07fe45b into main Sep 27, 2024
26 checks passed
@graphite-app graphite-app bot deleted the 09-27-refactor_transformer_exponentiation_operator_convert_to_match branch September 27, 2024 19:26
Boshen added a commit that referenced this pull request Sep 28, 2024
## [0.30.4] - 2024-09-28

### Bug Fixes

- 8582ae3 codegen: Missing parentheses if there is a pure comment before
a NewExpression as a ComputedMemberExpression's callee (#6105) (Dunqing)
- fd6798f parser: Remove unintended `pub Kind` (#6109) (Boshen)
- 6f98aad sourcemap: Align sourcemap type with Rollup (#6133) (Boshen)
- 64d4756 transformer: Fix debug assertion in `Stack` (#6106)
(overlookmotel)

### Performance

- 05852a0 codegen: Do not check whether there are annotation comments or
not if we don't preserve annotation comments (#6107) (Dunqing)

### Documentation

- 26a273a oxc-transform: Update README (Boshen)
- e2c5baf transformer: Fix formatting of README (#6111) (overlookmotel)

### Refactor

- 2090fce semantic: Fix lint warning in nightly (#6110) (overlookmotel)
- 7bc3988 transformer: Remove dead code (#6124) (overlookmotel)
- 07fe45b transformer: Exponentiation operator: convert to match (#6123)
(overlookmotel)
- 4387845 transformer: Share `TypeScriptOptions` with ref not `Rc`
(#6121) (overlookmotel)
- 09e41c2 transformer: Share `TransformCtx` with ref not `Rc` (#6118)
(overlookmotel)
- 58fd6eb transformer: Pre-allocate more stack space (#6095)
(overlookmotel)
- 9ac80bd transformer: Add wrapper around `NonNull` (#6115)
(overlookmotel)
- c50500e transformer: Move common stack functionality into
`StackCommon` trait (#6114) (overlookmotel)
- 9839059 transformer: Simplify `StackCapacity` trait (#6113)
(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-transformer Area - Transformer / Transpiler
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant