Skip to content

Commit bca3d40

Browse files
hai-xhai-x
authored
fix: eliminate unused statements in certain scenarios (#19536)
1 parent 356c5d1 commit bca3d40

File tree

16 files changed

+335
-65
lines changed

16 files changed

+335
-65
lines changed

‎lib/ConstPlugin.js‎

Lines changed: 61 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -179,53 +179,21 @@ class ConstPlugin {
179179
? statement.alternate
180180
: statement.consequent;
181181
if (branchToRemove) {
182-
// Before removing the dead branch, the hoisted declarations
183-
// must be collected.
184-
//
185-
// Given the following code:
186-
//
187-
// if (true) f() else g()
188-
// if (false) {
189-
// function f() {}
190-
// const g = function g() {}
191-
// if (someTest) {
192-
// let a = 1
193-
// var x, {y, z} = obj
194-
// }
195-
// } else {
196-
// …
197-
// }
198-
//
199-
// the generated code is:
200-
//
201-
// if (true) f() else {}
202-
// if (false) {
203-
// var f, x, y, z; (in loose mode)
204-
// var x, y, z; (in strict mode)
205-
// } else {
206-
// …
207-
// }
208-
//
209-
// NOTE: When code runs in strict mode, `var` declarations
210-
// are hoisted but `function` declarations don't.
211-
//
212-
const declarations = parser.scope.isStrict
213-
? getHoistedDeclarations(branchToRemove, false)
214-
: getHoistedDeclarations(branchToRemove, true);
215-
const replacement =
216-
declarations.length > 0
217-
? `{ var ${declarations.join(", ")}; }`
218-
: "{}";
219-
const dep = new ConstDependency(
220-
replacement,
221-
/** @type {Range} */ (branchToRemove.range)
222-
);
223-
dep.loc = /** @type {SourceLocation} */ (branchToRemove.loc);
224-
parser.state.module.addPresentationalDependency(dep);
182+
this.eliminateUnusedStatement(parser, branchToRemove);
225183
}
226184
return bool;
227185
}
228186
});
187+
parser.hooks.unusedStatement.tap(PLUGIN_NAME, statement => {
188+
if (
189+
parser.scope.isAsmJs ||
190+
// Check top level scope here again
191+
parser.scope.topLevelScope === true
192+
)
193+
return;
194+
this.eliminateUnusedStatement(parser, statement);
195+
return true;
196+
});
229197
parser.hooks.expressionConditionalOperator.tap(
230198
PLUGIN_NAME,
231199
expression => {
@@ -534,6 +502,56 @@ class ConstPlugin {
534502
}
535503
);
536504
}
505+
506+
/**
507+
* Eliminate an unused statement.
508+
* @param {JavascriptParser} parser the parser
509+
* @param {Statement} statement the statement to remove
510+
* @returns {void}
511+
*/
512+
eliminateUnusedStatement(parser, statement) {
513+
// Before removing the unused branch, the hoisted declarations
514+
// must be collected.
515+
//
516+
// Given the following code:
517+
//
518+
// if (true) f() else g()
519+
// if (false) {
520+
// function f() {}
521+
// const g = function g() {}
522+
// if (someTest) {
523+
// let a = 1
524+
// var x, {y, z} = obj
525+
// }
526+
// } else {
527+
// …
528+
// }
529+
//
530+
// the generated code is:
531+
//
532+
// if (true) f() else {}
533+
// if (false) {
534+
// var f, x, y, z; (in loose mode)
535+
// var x, y, z; (in strict mode)
536+
// } else {
537+
// …
538+
// }
539+
//
540+
// NOTE: When code runs in strict mode, `var` declarations
541+
// are hoisted but `function` declarations don't.
542+
//
543+
const declarations = parser.scope.isStrict
544+
? getHoistedDeclarations(statement, false)
545+
: getHoistedDeclarations(statement, true);
546+
const replacement =
547+
declarations.length > 0 ? `{ var ${declarations.join(", ")}; }` : "{}";
548+
const dep = new ConstDependency(
549+
`// removed by dead control flow\n${replacement}`,
550+
/** @type {Range} */ (statement.range)
551+
);
552+
dep.loc = /** @type {SourceLocation} */ (statement.loc);
553+
parser.state.module.addPresentationalDependency(dep);
554+
}
537555
}
538556

539557
module.exports = ConstPlugin;

‎lib/dependencies/RequireEnsureDependenciesBlockParserPlugin.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,10 @@ module.exports = class RequireEnsureDependenciesBlockParserPlugin {
109109
}
110110
if (successExpression) {
111111
if (successExpression.fn.body.type === "BlockStatement") {
112+
// Opt-out of Dead Control Flow detection for this block
113+
const oldTerminated = parser.scope.terminated;
112114
parser.walkStatement(successExpression.fn.body);
115+
parser.scope.terminated = oldTerminated;
113116
} else {
114117
parser.walkExpression(successExpression.fn.body);
115118
}

‎lib/javascript/JavascriptParser.js‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -578,7 +578,9 @@ class JavascriptParser extends Parser {
578578
/** @type {SyncBailHook<[ThrowStatement | ReturnStatement], boolean | void>} */
579579
terminate: new SyncBailHook(["statement"]),
580580
/** @type {SyncBailHook<[Program, Comment[]], boolean | void>} */
581-
finish: new SyncBailHook(["ast", "comments"])
581+
finish: new SyncBailHook(["ast", "comments"]),
582+
/** @type {SyncBailHook<[Statement], boolean | void>} */
583+
unusedStatement: new SyncBailHook(["statement"])
582584
});
583585
this.sourceType = sourceType;
584586
/** @type {ScopeInfo} */
@@ -1939,8 +1941,13 @@ class JavascriptParser extends Parser {
19391941
for (let index = 0, len = statements.length; index < len; index++) {
19401942
const statement = statements[index];
19411943

1942-
if (onlyFunctionDeclaration && statement.type !== "FunctionDeclaration")
1944+
if (
1945+
onlyFunctionDeclaration &&
1946+
statement.type !== "FunctionDeclaration" &&
1947+
this.hooks.unusedStatement.call(/** @type {Statement} */ (statement))
1948+
) {
19431949
continue;
1950+
}
19441951

19451952
this.walkStatement(statement);
19461953

‎test/configCases/parsing/dead-code-elimination-require-ensure/foo.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export default "foo"

‎test/configCases/parsing/dead-code-elimination-require-ensure/index.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
it("should compile and work", done => {
2+
require.ensure(
3+
["./foo"],
4+
() => {
5+
throw new Error("error");
6+
},
7+
() => {
8+
import("./foo").then(m => {
9+
expect(m.default).toBe("foo");
10+
done();
11+
});
12+
}
13+
);
14+
});

‎test/configCases/parsing/dead-code-elimination-require-ensure/webpack.config.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
/** @type {import("../../../../").Configuration} */
2+
module.exports = {
3+
optimization: {
4+
minimize: false
5+
}
6+
};

‎test/configCases/parsing/dead-code-elimination/esm1.js‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
it("Should eliminate hoisted function in ESM because of default strict mode", () => {
2+
expect(() => {
3+
fnDecl;
4+
}).toThrow();
5+
try {
6+
throw new Error();
7+
} catch (e) {
8+
return;
9+
}
10+
{
11+
function fnDecl() {
12+
expect(true).toBe(true);
13+
}
14+
}
15+
expect(true).toBe(false);
16+
});
17+
18+
export default "esm1";

‎test/configCases/parsing/dead-code-elimination/esm2.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export default "esm2";

‎test/configCases/parsing/dead-code-elimination/index.js‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
const esm1 = require("./esm1");
2+
3+
expect(esm1.default).toBe("esm1");
4+
5+
it("Shouldn't eliminate hoisted function in current block scope", () => {
6+
expect(() => {
7+
funcDecl;
8+
}).not.toThrow();
9+
10+
funcDecl();
11+
if (true) return;
12+
function funcDecl() {}
13+
import("./esm3").then(() => {
14+
expect(true).toBe(false);
15+
});
16+
});
17+
18+
it("Shouldn't eliminate hoisted variable", () => {
19+
expect(() => {
20+
varDecl;
21+
}).not.toThrow();
22+
try {
23+
throw new Error();
24+
} catch (e) {
25+
return;
26+
}
27+
var varDecl = "foo";
28+
expect(true).toBe(false);
29+
import("./esm3").then(() => {
30+
expect(true).toBe(false);
31+
});
32+
});
33+
34+
it("Shouldn't eliminate hoisted variable (more complex). From test/configCases/parsing/issue-4857/index.js", () => {
35+
expect(() => {
36+
a;
37+
b;
38+
c;
39+
d;
40+
e;
41+
f;
42+
g;
43+
h;
44+
i;
45+
j;
46+
k;
47+
l;
48+
m;
49+
n;
50+
o;
51+
}).not.toThrow();
52+
expect(() => {
53+
withVar;
54+
}).toThrow();
55+
try {
56+
} finally {
57+
return;
58+
}
59+
expect(true).toBe(false);
60+
var a,
61+
[, , b] = [],
62+
{ c, D: d, ["E"]: e = 2 } = {};
63+
var [{ ["_"]: f }, ...g] = [];
64+
do {
65+
switch (g) {
66+
default:
67+
var h;
68+
break;
69+
}
70+
loop: for (var i; ; ) for (var j in {}) for (var k of {}) break;
71+
try {
72+
var l;
73+
} catch (e) {
74+
var m;
75+
} finally {
76+
var n;
77+
}
78+
{
79+
var o;
80+
}
81+
} while (true);
82+
with (o) {
83+
var withVar;
84+
}
85+
import("./esm3").then(() => {
86+
expect(true).toBe(false);
87+
});
88+
});
89+
90+
it("Should eliminate hoisted function in strict mode", () => {
91+
"use strict";
92+
expect(() => {
93+
funcDecl;
94+
}).toThrow();
95+
96+
if (true) return;
97+
98+
{
99+
function funcDecl() {}
100+
expect(true).toBe(false);
101+
}
102+
});
103+
104+
it("Shouldn't eliminate hoisted function in sloppy mode", () => {
105+
expect(() => {
106+
funcDecl;
107+
}).not.toThrow();
108+
if (true) return;
109+
{
110+
function funcDecl() {}
111+
expect(true).toBe(false);
112+
}
113+
});
114+
if (true) {
115+
return;
116+
}
117+
// We won't terminate in top level scope although it won't run.
118+
require("./esm2");

‎test/configCases/parsing/dead-code-elimination/test.config.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
module.exports = {
2+
findBundle() {
3+
return ["test.js", `bundle0.js`];
4+
}
5+
};

0 commit comments

Comments
 (0)