Skip to content

Commit cea685f

Browse files
alexlamslalexlamsl
authored
fix corner case in ie8 (#3216)
fixes #3215
1 parent 8d4b534 commit cea685f

File tree

7 files changed

+350
-100
lines changed

7 files changed

+350
-100
lines changed

‎lib/compress.js

+40-44
Original file line numberDiff line numberDiff line change
@@ -1264,6 +1264,9 @@ merge(Compressor.prototype, {
12641264
if (node instanceof AST_Exit) {
12651265
return side_effects || lhs instanceof AST_PropAccess || may_modify(lhs);
12661266
}
1267+
if (node instanceof AST_Function) {
1268+
return compressor.option("ie8") && node.name && node.name.name in lvalues;
1269+
}
12671270
if (node instanceof AST_PropAccess) {
12681271
return side_effects || node.expression.may_throw_on_access(compressor);
12691272
}
@@ -1610,10 +1613,7 @@ merge(Compressor.prototype, {
16101613
if (def.orig.length == 1 && def.orig[0] instanceof AST_SymbolDefun) return false;
16111614
if (def.scope !== scope) return true;
16121615
return !all(def.references, function(ref) {
1613-
var s = ref.scope;
1614-
// "block" scope within AST_Catch
1615-
if (s.TYPE == "Scope") s = s.parent_scope;
1616-
return s === scope;
1616+
return ref.scope.resolve() === scope;
16171617
});
16181618
}
16191619

@@ -2704,35 +2704,30 @@ merge(Compressor.prototype, {
27042704
if (right === this.right) return this;
27052705
var result;
27062706
switch (this.operator) {
2707-
case "&&" : result = left && right; break;
2708-
case "||" : result = left || right; break;
2709-
case "|" : result = left | right; break;
2710-
case "&" : result = left & right; break;
2711-
case "^" : result = left ^ right; break;
2712-
case "+" : result = left + right; break;
2713-
case "*" : result = left * right; break;
2714-
case "/" : result = left / right; break;
2715-
case "%" : result = left % right; break;
2716-
case "-" : result = left - right; break;
2717-
case "<<" : result = left << right; break;
2718-
case ">>" : result = left >> right; break;
2719-
case ">>>" : result = left >>> right; break;
2720-
case "==" : result = left == right; break;
2721-
case "===" : result = left === right; break;
2722-
case "!=" : result = left != right; break;
2723-
case "!==" : result = left !== right; break;
2724-
case "<" : result = left < right; break;
2725-
case "<=" : result = left <= right; break;
2726-
case ">" : result = left > right; break;
2727-
case ">=" : result = left >= right; break;
2728-
default:
2729-
return this;
2730-
}
2731-
if (isNaN(result) && compressor.find_parent(AST_With)) {
2732-
// leave original expression as is
2733-
return this;
2734-
}
2735-
return result;
2707+
case "&&" : result = left && right; break;
2708+
case "||" : result = left || right; break;
2709+
case "|" : result = left | right; break;
2710+
case "&" : result = left & right; break;
2711+
case "^" : result = left ^ right; break;
2712+
case "+" : result = left + right; break;
2713+
case "*" : result = left * right; break;
2714+
case "/" : result = left / right; break;
2715+
case "%" : result = left % right; break;
2716+
case "-" : result = left - right; break;
2717+
case "<<" : result = left << right; break;
2718+
case ">>" : result = left >> right; break;
2719+
case ">>>": result = left >>> right; break;
2720+
case "==" : result = left == right; break;
2721+
case "===": result = left === right; break;
2722+
case "!=" : result = left != right; break;
2723+
case "!==": result = left !== right; break;
2724+
case "<" : result = left < right; break;
2725+
case "<=" : result = left <= right; break;
2726+
case ">" : result = left > right; break;
2727+
case ">=" : result = left >= right; break;
2728+
default : return this;
2729+
}
2730+
return isNaN(result) && compressor.find_parent(AST_With) ? this : result;
27362731
});
27372732
def(AST_Conditional, function(compressor, cached, depth) {
27382733
var condition = this.condition._eval(compressor, cached, depth);
@@ -3412,6 +3407,14 @@ merge(Compressor.prototype, {
34123407
init.walk(tw);
34133408
});
34143409
}
3410+
var drop_fn_name = compressor.option("keep_fnames") ? return_false : compressor.option("ie8") ? function(def) {
3411+
return !compressor.exposed(def) && !def.references.length;
3412+
} : function(def) {
3413+
// any declarations with same name will overshadow
3414+
// name of this anonymous function and can therefore
3415+
// never be used anywhere
3416+
return !(def.id in in_use_ids) || def.orig.length > 1;
3417+
};
34153418
// pass 3: we should drop declarations not in_use
34163419
var tt = new TreeTransformer(function(node, descend, in_list) {
34173420
var parent = tt.parent();
@@ -3439,15 +3442,8 @@ merge(Compressor.prototype, {
34393442
}
34403443
}
34413444
if (scope !== self) return;
3442-
if (node instanceof AST_Function
3443-
&& node.name
3444-
&& !compressor.option("ie8")
3445-
&& !compressor.option("keep_fnames")) {
3446-
var def = node.name.definition();
3447-
// any declarations with same name will overshadow
3448-
// name of this anonymous function and can therefore
3449-
// never be used anywhere
3450-
if (!(def.id in in_use_ids) || def.orig.length > 1) node.name = null;
3445+
if (node instanceof AST_Function && node.name && drop_fn_name(node.name.definition())) {
3446+
node.name = null;
34513447
}
34523448
if (node instanceof AST_Lambda && !(node instanceof AST_Accessor)) {
34533449
var trim = !compressor.option("keep_fargs");
@@ -5726,10 +5722,10 @@ merge(Compressor.prototype, {
57265722
if (def) {
57275723
return def.optimize(compressor);
57285724
}
5729-
// testing against !self.scope.uses_with first is an optimization
57305725
if (!compressor.option("ie8")
57315726
&& is_undeclared_ref(self)
5732-
&& (!self.scope.uses_with || !compressor.find_parent(AST_With))) {
5727+
// testing against `self.scope.uses_with` is an optimization
5728+
&& !(self.scope.uses_with && compressor.find_parent(AST_With))) {
57335729
switch (self.name) {
57345730
case "undefined":
57355731
return make_node(AST_Undefined, self).optimize(compressor);

‎lib/scope.js

+51-44
Original file line numberDiff line numberDiff line change
@@ -118,11 +118,10 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options) {
118118
descend();
119119
scope = save_scope;
120120
defun = save_defun;
121-
return true; // don't descend again in TreeWalker
121+
return true;
122122
}
123123
if (node instanceof AST_With) {
124-
for (var s = scope; s; s = s.parent_scope)
125-
s.uses_with = true;
124+
for (var s = scope; s; s = s.parent_scope) s.uses_with = true;
126125
return;
127126
}
128127
if (node instanceof AST_Symbol) {
@@ -132,18 +131,13 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options) {
132131
node.thedef = node;
133132
node.references = [];
134133
}
135-
if (node instanceof AST_SymbolDefun || options.ie8 && node instanceof AST_SymbolLambda) {
136-
// Careful here, the scope where this should be defined is
137-
// the parent scope. The reason is that we enter a new
138-
// scope when we encounter the AST_Defun node (which is
139-
// instanceof AST_Scope) but we get to the symbol a bit
140-
// later.
141-
(node.scope = defun.parent_scope).def_function(node, defun);
142-
}
143-
else if (node instanceof AST_SymbolLambda) {
134+
if (node instanceof AST_SymbolDefun) {
135+
// This should be defined in the parent scope, as we encounter the
136+
// AST_Defun node before getting to its AST_Symbol.
137+
(node.scope = defun.parent_scope.resolve()).def_function(node, defun);
138+
} else if (node instanceof AST_SymbolLambda) {
144139
defun.def_function(node, node.name == "arguments" ? undefined : defun);
145-
}
146-
else if (node instanceof AST_SymbolVar) {
140+
} else if (node instanceof AST_SymbolVar) {
147141
defun.def_variable(node, node.TYPE == "SymbolVar" ? null : undefined);
148142
if (defun !== scope) {
149143
node.mark_enclosed(options);
@@ -153,18 +147,17 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options) {
153147
}
154148
node.reference(options);
155149
}
156-
}
157-
else if (node instanceof AST_SymbolCatch) {
150+
} else if (node instanceof AST_SymbolCatch) {
158151
scope.def_variable(node).defun = defun;
159152
}
160153
});
161154
self.walk(tw);
162155

163156
// pass 2: find back references and eval
164157
self.globals = new Dictionary();
165-
var tw = new TreeWalker(function(node, descend) {
166-
if (node instanceof AST_LoopControl && node.label) {
167-
node.label.thedef.references.push(node);
158+
var tw = new TreeWalker(function(node) {
159+
if (node instanceof AST_LoopControl) {
160+
if (node.label) node.label.thedef.references.push(node);
168161
return true;
169162
}
170163
if (node instanceof AST_SymbolRef) {
@@ -185,35 +178,43 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options) {
185178
return true;
186179
}
187180
// ensure mangling works if catch reuses a scope variable
188-
var def;
189-
if (node instanceof AST_SymbolCatch && (def = node.definition().redefined())) {
190-
var s = node.scope;
191-
while (s) {
181+
if (node instanceof AST_SymbolCatch) {
182+
var def = node.definition().redefined();
183+
if (def) for (var s = node.scope; s; s = s.parent_scope) {
192184
push_uniq(s.enclosed, def);
193185
if (s === def.scope) break;
194-
s = s.parent_scope;
195186
}
187+
return true;
196188
}
197189
});
198190
self.walk(tw);
199191

200192
// pass 3: fix up any scoping issue with IE8
201-
if (options.ie8) {
202-
self.walk(new TreeWalker(function(node, descend) {
203-
if (node instanceof AST_SymbolCatch) {
204-
var name = node.name;
205-
var refs = node.thedef.references;
206-
var scope = node.thedef.defun;
207-
var def = scope.find_variable(name) || self.globals.get(name) || scope.def_variable(node);
208-
refs.forEach(function(ref) {
209-
ref.thedef = def;
210-
ref.reference(options);
211-
});
212-
node.thedef = def;
213-
node.reference(options);
214-
return true;
193+
if (options.ie8) self.walk(new TreeWalker(function(node) {
194+
if (node instanceof AST_SymbolCatch) {
195+
redefine(node, node.thedef.defun);
196+
return true;
197+
}
198+
if (node instanceof AST_SymbolLambda) {
199+
var def = node.thedef;
200+
if (def.orig.length == 1) {
201+
redefine(node, node.scope.parent_scope);
202+
node.thedef.init = def.init;
215203
}
216-
}));
204+
return true;
205+
}
206+
}));
207+
208+
function redefine(node, scope) {
209+
var name = node.name;
210+
var refs = node.thedef.references;
211+
var def = scope.find_variable(name) || self.globals.get(name) || scope.def_variable(node);
212+
refs.forEach(function(ref) {
213+
ref.thedef = def;
214+
ref.reference(options);
215+
});
216+
node.thedef = def;
217+
node.reference(options);
217218
}
218219
});
219220

@@ -252,16 +253,14 @@ AST_Lambda.DEFMETHOD("init_scope_vars", function() {
252253

253254
AST_Symbol.DEFMETHOD("mark_enclosed", function(options) {
254255
var def = this.definition();
255-
var s = this.scope;
256-
while (s) {
256+
for (var s = this.scope; s; s = s.parent_scope) {
257257
push_uniq(s.enclosed, def);
258258
if (options.keep_fnames) {
259259
s.functions.each(function(d) {
260260
push_uniq(def.scope.enclosed, d);
261261
});
262262
}
263263
if (s === def.scope) break;
264-
s = s.parent_scope;
265264
}
266265
});
267266

@@ -298,6 +297,12 @@ AST_Scope.DEFMETHOD("def_variable", function(symbol, init) {
298297
return symbol.thedef = def;
299298
});
300299

300+
AST_Lambda.DEFMETHOD("resolve", return_this);
301+
AST_Scope.DEFMETHOD("resolve", function() {
302+
return this.parent_scope;
303+
});
304+
AST_Toplevel.DEFMETHOD("resolve", return_this);
305+
301306
function names_in_use(scope, options) {
302307
var names = scope.names_in_use;
303308
if (!names) {
@@ -410,7 +415,7 @@ AST_Toplevel.DEFMETHOD("mangle_names", function(options) {
410415
var save_nesting = lname;
411416
descend();
412417
lname = save_nesting;
413-
return true; // don't descend again in TreeWalker
418+
return true;
414419
}
415420
if (node instanceof AST_Scope) {
416421
descend();
@@ -422,7 +427,9 @@ AST_Toplevel.DEFMETHOD("mangle_names", function(options) {
422427
}
423428
if (node instanceof AST_Label) {
424429
var name;
425-
do name = base54(++lname); while (!is_identifier(name));
430+
do {
431+
name = base54(++lname);
432+
} while (!is_identifier(name));
426433
node.mangled_name = name;
427434
return true;
428435
}

‎lib/utils.js

+2-3
Original file line numberDiff line numberDiff line change
@@ -172,9 +172,8 @@ function string_template(text, props) {
172172
}
173173

174174
function remove(array, el) {
175-
for (var i = array.length; --i >= 0;) {
176-
if (array[i] === el) array.splice(i, 1);
177-
}
175+
var index = array.indexOf(el);
176+
if (index >= 0) array.splice(index, 1);
178177
}
179178

180179
function makePredicate(words) {

0 commit comments

Comments
 (0)