fix: eliminate unused statements in certain scenarios#19536
Merged
alexander-akait merged 2 commits intowebpack:mainfrom May 20, 2025
hai-x:feat-dead-statement
Merged
fix: eliminate unused statements in certain scenarios#19536alexander-akait merged 2 commits intowebpack:mainfrom hai-x:feat-dead-statement
alexander-akait merged 2 commits intowebpack:mainfrom
hai-x:feat-dead-statement
Conversation
lib/ConstPlugin.js
Outdated
| return bool; | ||
| } | ||
| }); | ||
| parser.hooks.deadStatement.tap(PLUGIN_NAME, statement => { |
Member
There was a problem hiding this comment.
What about unusedStatement? dead is more a harsher word 😄
| const oldTerminated = parser.scope.terminated; | ||
| parser.scope.terminated = undefined; | ||
| parser.walkStatement(successExpression.fn.body); | ||
| parser.scope.terminated = oldTerminated; |
Member
Author
There was a problem hiding this comment.
We walk successExpressionArg and then walk errorExpressionArg in
In this test case
it will throw insuccessExpressionArg and parsing scope will be terminated which leads to failed.
To keep it short and to the point, show the input and output.
input:
it("should call error callback on exception thrown in loading module", function(done) {
require.ensure(['./throwing'], function(){
require('./throwing');
}, function(error) {
expect(error).toBeInstanceOf(Error);
expect(error.message).toBe('message');
done();
});
});
eliminated output
it("should call error callback on exception thrown in loading module", function(done) {
require.ensure(['./throwing'], function(){
require('./throwing');
}, function(error) {
expect(error).toBeInstanceOf(Error);
{}
{}
});
});
I think it's very very rare user case and i also add some test for it.
lib/ConstPlugin.js
Outdated
| const replacement = | ||
| declarations.length > 0 ? `{ var ${declarations.join(", ")}; }` : "{}"; | ||
| const dep = new ConstDependency( | ||
| replacement, |
Member
There was a problem hiding this comment.
Let's add a comment // removed by dead control flow\n${replacement} (maybe better) for debug reasons
CodSpeed Performance ReportMerging #19536 will degrade performances by 81.71%Comparing Summary
Benchmarks breakdown
|
Member
|
/lgtm |
alexander-akait
approved these changes
May 20, 2025
3ru
pushed a commit
to 3ru/webpack
that referenced
this pull request
Jul 1, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What kind of change does this PR introduce?
We have previously supported the elimination of unreachable (dead) branches in if statements in #6273.
In this PR, we adopt the same approach to remove some dead statements when tracked completely return/throw statement, as implemented in #14357.
And it also fixes #19514.
Did you add tests for your changes?
Yes
Does this PR introduce a breaking change?
No
What needs to be documented once your changes are merged?
No