-
-
Notifications
You must be signed in to change notification settings - Fork 658
feat(minifier): remove unused assignments for vars #13231
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
feat(minifier): remove unused assignments for vars #13231
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements support for removing unused assignments for var
declarations in the minifier. Previously, variables declared with var
did not have SymbolValue
populated, causing them to bail out during unused assignment removal optimization.
- Modified the inline module to populate
SymbolValue
forvar
declarations while avoiding constant value inlining - Updated test cases to verify that unused
var
assignments are now properly removed - Achieved improved minification results across multiple test files
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
crates/oxc_minifier/src/peephole/inline.rs | Modified init_symbol_value to populate symbol values for var declarations while skipping constant value inlining |
crates/oxc_minifier/src/peephole/remove_unused_expression.rs | Updated test cases to verify unused var assignment removal works correctly |
tasks/minsize/minsize.snap | Updated snapshot showing improved minification sizes across test files |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
CodSpeed Instrumentation Performance ReportMerging #13231 will degrade performances by 5.55%Comparing Summary
Benchmarks breakdown
Footnotes |
2141c18
to
dadfdc6
Compare
dadfdc6
to
93236d4
Compare
758139a
to
277b91e
Compare
277b91e
to
fc93c07
Compare
93236d4
to
19ab2c0
Compare
19ab2c0
to
b9a5d6e
Compare
fc93c07
to
da76589
Compare
Merge activity
|
I found that the temporary variable are kept. This change would remove them. Variables that were declared with `var` did not have `SymbolValue` populated and that caused those variables to bail out at this line. https://github.com/oxc-project/oxc/blob/2141c18540803218889aa3c3f1bca5f51cdb5daa/crates/oxc_minifier/src/peephole/remove_unused_expression.rs#L624-L626 I think TDZ needs to be considered when inlining the values, but does not affect removing the unused assignments.
da76589
to
0e804aa
Compare
b9a5d6e
to
8ca9909
Compare
I found that the temporary variable are kept. This change would remove them.
Variables that were declared with
var
did not haveSymbolValue
populated and that caused those variables to bail out at this line.oxc/crates/oxc_minifier/src/peephole/remove_unused_expression.rs
Lines 624 to 626 in 2141c18
I think TDZ needs to be considered when inlining the values, but does not affect removing the unused assignments.