Skip to content

Commit e6fb58d

Browse files
sokrasokra
authored
Merge pull request webpack#6273 from ooflorent/fix-4857/unreachable_branches
Eliminate unreachable branches
2 parents 7112943 + 2cf8660 commit e6fb58d

File tree

6 files changed

+272
-3
lines changed

6 files changed

+272
-3
lines changed

‎lib/ConstPlugin.js

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,90 @@ const getQuery = (request) => {
1212
return request.includes("?") ? request.substr(i) : "";
1313
};
1414

15+
const collectDeclaration = (declarations, pattern) => {
16+
const stack = [pattern];
17+
while(stack.length > 0) {
18+
const node = stack.pop();
19+
switch(node.type) {
20+
case "Identifier":
21+
declarations.add(node.name);
22+
break;
23+
case "ArrayPattern":
24+
for(const element of node.elements)
25+
if(element) stack.push(element);
26+
break;
27+
case "AssignmentPattern":
28+
stack.push(node.left);
29+
break;
30+
case "ObjectPattern":
31+
for(const property of node.properties)
32+
stack.push(property.value);
33+
break;
34+
case "RestElement":
35+
stack.push(node.argument);
36+
break;
37+
}
38+
}
39+
};
40+
41+
const getHoistedDeclarations = (branch, includeFunctionDeclarations) => {
42+
const declarations = new Set();
43+
const stack = [branch];
44+
while(stack.length > 0) {
45+
const node = stack.pop();
46+
// Some node could be `null` or `undefined`.
47+
if(!node)
48+
continue;
49+
switch(node.type) {
50+
// Walk through control statements to look for hoisted declarations.
51+
// Some branches are skipped since they do not allow declarations.
52+
case "BlockStatement":
53+
for(const stmt of node.body)
54+
stack.push(stmt);
55+
break;
56+
case "IfStatement":
57+
stack.push(node.consequent);
58+
stack.push(node.alternate);
59+
break;
60+
case "ForStatement":
61+
stack.push(node.init);
62+
stack.push(node.body);
63+
break;
64+
case "ForInStatement":
65+
case "ForOfStatement":
66+
stack.push(node.left);
67+
stack.push(node.body);
68+
break;
69+
case "DoWhileStatement":
70+
case "WhileStatement":
71+
case "LabeledStatement":
72+
stack.push(node.body);
73+
break;
74+
case "SwitchStatement":
75+
for(const cs of node.cases)
76+
for(const consequent of cs.consequent)
77+
stack.push(consequent);
78+
break;
79+
case "TryStatement":
80+
stack.push(node.block);
81+
if(node.handler)
82+
stack.push(node.handler.body);
83+
stack.push(node.finalizer);
84+
break;
85+
case "FunctionDeclaration":
86+
if(includeFunctionDeclarations)
87+
collectDeclaration(declarations, node.id);
88+
break;
89+
case "VariableDeclaration":
90+
if(node.kind === "var")
91+
for(const decl of node.declarations)
92+
collectDeclaration(declarations, decl.id);
93+
break;
94+
}
95+
}
96+
return Array.from(declarations);
97+
};
98+
1599
class ConstPlugin {
16100
apply(compiler) {
17101
compiler.hooks.compilation.tap("ConstPlugin", (compilation, {
@@ -30,6 +114,57 @@ class ConstPlugin {
30114
dep.loc = statement.loc;
31115
parser.state.current.addDependency(dep);
32116
}
117+
const branchToRemove = bool ? statement.alternate : statement.consequent;
118+
if(branchToRemove) {
119+
// Before removing the dead branch, the hoisted declarations
120+
// must be collected.
121+
//
122+
// Given the following code:
123+
//
124+
// if (true) f() else g()
125+
// if (false) {
126+
// function f() {}
127+
// const g = function g() {}
128+
// if (someTest) {
129+
// let a = 1
130+
// var x, {y, z} = obj
131+
// }
132+
// } else {
133+
// …
134+
// }
135+
//
136+
// the generated code is:
137+
//
138+
// if (true) f() else {}
139+
// if (false) {
140+
// var f, x, y, z; (in loose mode)
141+
// var x, y, z; (in strict mode)
142+
// } else {
143+
// …
144+
// }
145+
//
146+
// NOTE: When code runs in strict mode, `var` declarations
147+
// are hoisted but `function` declarations don't.
148+
//
149+
let declarations;
150+
if(parser.scope.isStrict) {
151+
// If the code runs in strict mode, variable declarations
152+
// using `var` must be hoisted.
153+
declarations = getHoistedDeclarations(branchToRemove, false);
154+
} else {
155+
// Otherwise, collect all hoisted declaration.
156+
declarations = getHoistedDeclarations(branchToRemove, true);
157+
}
158+
let replacement;
159+
if(declarations.length > 0) {
160+
replacement = `{ var ${declarations.join(", ")}; }`;
161+
} else {
162+
replacement = "{}";
163+
}
164+
const dep = new ConstDependency(replacement, branchToRemove.range);
165+
dep.loc = branchToRemove.loc;
166+
parser.state.current.addDependency(dep);
167+
}
33168
return bool;
34169
}
35170
});
@@ -42,6 +177,21 @@ class ConstPlugin {
42177
dep.loc = expression.loc;
43178
parser.state.current.addDependency(dep);
44179
}
180+
// Expressions do not hoist.
181+
// It is safe to remove the dead branch.
182+
//
183+
// Given the following code:
184+
//
185+
// false ? someExpression() : otherExpression();
186+
//
187+
// the generated code is:
188+
//
189+
// false ? undefined : otherExpression();
190+
//
191+
const branchToRemove = bool ? expression.alternate : expression.consequent;
192+
const dep = new ConstDependency("undefined", branchToRemove.range);
193+
dep.loc = branchToRemove.loc;
194+
parser.state.current.addDependency(dep);
45195
return bool;
46196
}
47197
});

‎lib/Parser.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -948,6 +948,7 @@ class Parser extends Tapable {
948948
this.walkPattern(param);
949949
this.inScope(statement.params, () => {
950950
if(statement.body.type === "BlockStatement") {
951+
this.detectStrictMode(statement.body.body);
951952
this.prewalkStatement(statement.body);
952953
this.walkStatement(statement.body);
953954
} else {
@@ -1316,6 +1317,7 @@ class Parser extends Tapable {
13161317
this.walkPattern(param);
13171318
this.inScope(expression.params, () => {
13181319
if(expression.body.type === "BlockStatement") {
1320+
this.detectStrictMode(expression.body.body);
13191321
this.prewalkStatement(expression.body);
13201322
this.walkStatement(expression.body);
13211323
} else {
@@ -1329,6 +1331,7 @@ class Parser extends Tapable {
13291331
this.walkPattern(param);
13301332
this.inScope(expression.params, () => {
13311333
if(expression.body.type === "BlockStatement") {
1334+
this.detectStrictMode(expression.body.body);
13321335
this.prewalkStatement(expression.body);
13331336
this.walkStatement(expression.body);
13341337
} else {
@@ -1580,6 +1583,7 @@ class Parser extends Tapable {
15801583
this.scope = {
15811584
inTry: false,
15821585
inShorthand: false,
1586+
isStrict: oldScope.isStrict,
15831587
definitions: oldScope.definitions.createChild(),
15841588
renames: oldScope.renames.createChild()
15851589
};
@@ -1604,6 +1608,16 @@ class Parser extends Tapable {
16041608
this.scope = oldScope;
16051609
}
16061610

1611+
detectStrictMode(statements) {
1612+
const isStrict = statements.length >= 1 &&
1613+
statements[0].type === "ExpressionStatement" &&
1614+
statements[0].expression.type === "Literal" &&
1615+
statements[0].expression.value === "use strict";
1616+
if(isStrict) {
1617+
this.scope.isStrict = true;
1618+
}
1619+
}
1620+
16071621
enterPattern(pattern, onIdent) {
16081622
if(!pattern) return;
16091623
switch(pattern.type) {
@@ -1775,12 +1789,15 @@ class Parser extends Tapable {
17751789
const oldComments = this.comments;
17761790
this.scope = {
17771791
inTry: false,
1792+
inShorthand: false,
1793+
isStrict: false,
17781794
definitions: new StackedSetMap(),
17791795
renames: new StackedSetMap()
17801796
};
17811797
const state = this.state = initialState || {};
17821798
this.comments = comments;
17831799
if(this.hooks.program.call(ast, comments) === undefined) {
1800+
this.detectStrictMode(ast.body);
17841801
this.prewalkStatements(ast.body);
17851802
this.walkStatements(ast.body);
17861803
}

‎lib/UseStrictPlugin.js

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ class UseStrictPlugin {
1212
normalModuleFactory
1313
}) => {
1414
const handler = (parser) => {
15-
const parserInstance = parser;
1615
parser.hooks.program.tap("UseStrictPlugin", (ast) => {
1716
const firstNode = ast.body[0];
1817
if(firstNode &&
@@ -24,8 +23,8 @@ class UseStrictPlugin {
2423
// @see https://github.com/webpack/webpack/issues/1970
2524
const dep = new ConstDependency("", firstNode.range);
2625
dep.loc = firstNode.loc;
27-
parserInstance.state.current.addDependency(dep);
28-
parserInstance.state.module.buildInfo.strict = true;
26+
parser.state.current.addDependency(dep);
27+
parser.state.module.buildInfo.strict = true;
2928
}
3029
});
3130
};

‎lib/dependencies/HarmonyDetectionParserPlugin.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ module.exports = class HarmonyDetectionParserPlugin {
4343
};
4444
module.addDependency(initDep);
4545
parser.state.harmonyParserScope = parser.state.harmonyParserScope || {};
46+
parser.scope.isStrict = true;
4647
module.buildMeta.exportsType = "namespace";
4748
module.buildInfo.strict = true;
4849
module.buildInfo.exportsArgument = "__webpack_exports__";

‎test/configCases/parsing/issue-4857/index.js

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
it("should transpile unreachable branches", () => {
2+
let count = 0;
3+
4+
// BlockStatement
5+
if(true) {
6+
count++;
7+
} else {
8+
import("NOT_REACHABLE");
9+
}
10+
if(false) {
11+
import("NOT_REACHABLE");
12+
} else {
13+
count++;
14+
}
15+
16+
// ExpressionStatement
17+
if(true) count++;
18+
else import("NOT_REACHABLE");
19+
if(false) import("NOT_REACHABLE");
20+
else count++;
21+
22+
// ConditionalExpression
23+
true ? count++ : import("NOT_REACHABLE");
24+
false ? import("NOT_REACHABLE") : count++;
25+
26+
count.should.be.eql(6);
27+
});
28+
29+
it("should not remove hoisted variable declarations", () => {
30+
if(false) {
31+
var a, [,,b,] = [], {c, D: d, ["E"]: e = 2} = {};
32+
var [{["_"]: f}, ...g] = [];
33+
do {
34+
switch(g) {
35+
default:
36+
var h;
37+
break;
38+
}
39+
loop: for(var i;;)
40+
for(var j in {})
41+
for(var k of {})
42+
break;
43+
try {
44+
var l;
45+
} catch(e) {
46+
var m;
47+
} finally {
48+
var n;
49+
}
50+
{
51+
var o;
52+
}
53+
} while(true);
54+
with (o) {
55+
var withVar;
56+
}
57+
}
58+
(() => {
59+
a;
60+
b;
61+
c;
62+
d;
63+
e;
64+
f;
65+
g;
66+
h;
67+
i;
68+
j;
69+
k;
70+
l;
71+
m;
72+
n;
73+
o;
74+
}).should.not.throw();
75+
(() => {
76+
withVar;
77+
}).should.throw();
78+
});
79+
80+
it("should not remove hoisted function declarations in loose mode", () => {
81+
if(false) {
82+
function funcDecl() {}
83+
}
84+
(() => {
85+
funcDecl;
86+
}).should.not.throw();
87+
});
88+
89+
it("should remove hoisted function declarations in strict mode", () => {
90+
"use strict";
91+
if(false) {
92+
function funcDecl() {}
93+
}
94+
(() => {
95+
funcDecl;
96+
}).should.throw();
97+
});

‎test/configCases/parsing/issue-4857/webpack.config.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
module.exports = {
2+
optimization: {
3+
minimize: false
4+
}
5+
};

0 commit comments

Comments
 (0)