Skip to content

Commit 7aa6911

Browse files
authoredMar 1, 2017
fix corner cases in reduce_vars (#1524)
Avoid variable substitution in the following cases: - use of variable before declaration - declaration within conditional code blocks - declaration within loop body fixes #1518 fixes #1525
1 parent bff7ad6 commit 7aa6911

File tree

2 files changed

+206
-20
lines changed

2 files changed

+206
-20
lines changed
 

‎lib/compress.js

+87-20
Original file line numberDiff line numberDiff line change
@@ -179,38 +179,105 @@ merge(Compressor.prototype, {
179179

180180
AST_Node.DEFMETHOD("reset_opt_flags", function(compressor, rescan){
181181
var reduce_vars = rescan && compressor.option("reduce_vars");
182-
var unsafe = compressor.option("unsafe");
182+
var safe_ids = [];
183+
push();
183184
var tw = new TreeWalker(function(node){
185+
if (!(node instanceof AST_Directive || node instanceof AST_Constant)) {
186+
node._squeezed = false;
187+
node._optimized = false;
188+
}
184189
if (reduce_vars) {
185190
if (node instanceof AST_Toplevel) node.globals.each(reset_def);
186191
if (node instanceof AST_Scope) node.variables.each(reset_def);
187192
if (node instanceof AST_SymbolRef) {
188193
var d = node.definition();
189194
d.references.push(node);
190-
if (d.fixed && (d.orig.length > 1 || isModified(node, 0))) {
195+
if (!d.fixed || isModified(node, 0) || !is_safe(d)) {
196+
d.fixed = false;
197+
}
198+
}
199+
if (node instanceof AST_VarDef) {
200+
var d = node.name.definition();
201+
if (d.fixed === undefined) {
202+
d.fixed = node.value || make_node(AST_Undefined, node);
203+
mark_as_safe(d);
204+
} else {
191205
d.fixed = false;
192206
}
193207
}
194-
if (node instanceof AST_Call && node.expression instanceof AST_Function) {
195-
node.expression.argnames.forEach(function(arg, i) {
196-
arg.definition().init = node.args[i] || make_node(AST_Undefined, node);
208+
var iife;
209+
if (node instanceof AST_Function
210+
&& (iife = tw.parent()) instanceof AST_Call
211+
&& iife.expression === node) {
212+
node.argnames.forEach(function(arg, i) {
213+
var d = arg.definition();
214+
d.fixed = iife.args[i] || make_node(AST_Undefined, iife);
215+
mark_as_safe(d);
197216
});
198217
}
199-
}
200-
if (!(node instanceof AST_Directive || node instanceof AST_Constant)) {
201-
node._squeezed = false;
202-
node._optimized = false;
218+
if (node instanceof AST_If || node instanceof AST_DWLoop) {
219+
node.condition.walk(tw);
220+
push();
221+
node.body.walk(tw);
222+
pop();
223+
if (node.alternative) {
224+
push();
225+
node.alternative.walk(tw);
226+
pop();
227+
}
228+
return true;
229+
}
230+
if (node instanceof AST_LabeledStatement) {
231+
push();
232+
node.body.walk(tw);
233+
pop();
234+
return true;
235+
}
236+
if (node instanceof AST_For) {
237+
if (node.init) node.init.walk(tw);
238+
push();
239+
if (node.condition) node.condition.walk(tw);
240+
node.body.walk(tw);
241+
if (node.step) node.step.walk(tw);
242+
pop();
243+
return true;
244+
}
245+
if (node instanceof AST_ForIn) {
246+
if (node.init instanceof AST_SymbolRef) {
247+
node.init.definition().fixed = false;
248+
}
249+
node.object.walk(tw);
250+
push();
251+
node.body.walk(tw);
252+
pop();
253+
return true;
254+
}
203255
}
204256
});
205257
this.walk(tw);
206258

259+
function mark_as_safe(def) {
260+
safe_ids[safe_ids.length - 1][def.id] = true;
261+
}
262+
263+
function is_safe(def) {
264+
for (var i = safe_ids.length, id = def.id; --i >= 0;) {
265+
if (safe_ids[i][id]) return true;
266+
}
267+
}
268+
269+
function push() {
270+
safe_ids.push(Object.create(null));
271+
}
272+
273+
function pop() {
274+
safe_ids.pop();
275+
}
276+
207277
function reset_def(def) {
208-
def.fixed = true;
278+
def.fixed = undefined;
209279
def.references = [];
210280
def.should_replace = undefined;
211-
if (unsafe && def.init) {
212-
def.init._evaluated = undefined;
213-
}
214281
}
215282

216283
function isModified(node, level) {
@@ -1263,14 +1330,14 @@ merge(Compressor.prototype, {
12631330
this._evaluating = true;
12641331
try {
12651332
var d = this.definition();
1266-
if (compressor.option("reduce_vars") && d.fixed && d.init) {
1333+
if (compressor.option("reduce_vars") && d.fixed) {
12671334
if (compressor.option("unsafe")) {
1268-
if (d.init._evaluated === undefined) {
1269-
d.init._evaluated = ev(d.init, compressor);
1335+
if (!HOP(d.fixed, "_evaluated")) {
1336+
d.fixed._evaluated = ev(d.fixed, compressor);
12701337
}
1271-
return d.init._evaluated;
1338+
return d.fixed._evaluated;
12721339
}
1273-
return ev(d.init, compressor);
1340+
return ev(d.fixed, compressor);
12741341
}
12751342
} finally {
12761343
this._evaluating = false;
@@ -3046,9 +3113,9 @@ merge(Compressor.prototype, {
30463113
}
30473114
if (compressor.option("evaluate") && compressor.option("reduce_vars")) {
30483115
var d = self.definition();
3049-
if (d.fixed && d.init) {
3116+
if (d.fixed) {
30503117
if (d.should_replace === undefined) {
3051-
var init = d.init.evaluate(compressor);
3118+
var init = d.fixed.evaluate(compressor);
30523119
if (init.length > 1) {
30533120
var value = init[0].print_to_string().length;
30543121
var name = d.name.length;

‎test/compress/reduce_vars.js

+119
Original file line numberDiff line numberDiff line change
@@ -470,3 +470,122 @@ multi_def_2: {
470470
var repeatLength = this.getBits(bitsLength) + bitsOffset;
471471
}
472472
}
473+
474+
use_before_var: {
475+
options = {
476+
evaluate: true,
477+
reduce_vars: true,
478+
}
479+
input: {
480+
console.log(t);
481+
var t = 1;
482+
}
483+
expect: {
484+
console.log(t);
485+
var t = 1;
486+
}
487+
}
488+
489+
inner_var_if: {
490+
options = {
491+
evaluate: true,
492+
reduce_vars: true,
493+
}
494+
input: {
495+
function f(){
496+
return 0;
497+
}
498+
if (f())
499+
var t = 1;
500+
if (!t)
501+
console.log(t);
502+
}
503+
expect: {
504+
function f(){
505+
return 0;
506+
}
507+
if (f())
508+
var t = 1;
509+
if (!t)
510+
console.log(t);
511+
}
512+
}
513+
514+
inner_var_label: {
515+
options = {
516+
evaluate: true,
517+
reduce_vars: true,
518+
}
519+
input: {
520+
function f(){
521+
return 1;
522+
}
523+
l: {
524+
if (f()) break l;
525+
var t = 1;
526+
}
527+
console.log(t);
528+
}
529+
expect: {
530+
function f(){
531+
return 1;
532+
}
533+
l: {
534+
if (f()) break l;
535+
var t = 1;
536+
}
537+
console.log(t);
538+
}
539+
}
540+
541+
inner_var_for: {
542+
options = {
543+
evaluate: true,
544+
reduce_vars: true,
545+
}
546+
input: {
547+
var a = 1;
548+
x(a, b, d);
549+
for (var b = 2, c = 3; x(a, b, c, d); x(a, b, c, d)) {
550+
var d = 4, e = 5;
551+
x(a, b, c, d, e);
552+
}
553+
x(a, b, c, d, e)
554+
}
555+
expect: {
556+
var a = 1;
557+
x(1, b, d);
558+
for (var b = 2, c = 3; x(1, b, 3, d); x(1, b, 3, d)) {
559+
var d = 4, e = 5;
560+
x(1, b, 3, d, e);
561+
}
562+
x(1, b, 3, d, e);
563+
}
564+
}
565+
566+
inner_var_for_in: {
567+
options = {
568+
evaluate: true,
569+
reduce_vars: true,
570+
}
571+
input: {
572+
var a = 1, b = 2;
573+
for (b in (function() {
574+
return x(a, b, c);
575+
})()) {
576+
var c = 3, d = 4;
577+
x(a, b, c, d);
578+
}
579+
x(a, b, c, d);
580+
}
581+
expect: {
582+
var a = 1, b = 2;
583+
for (b in (function() {
584+
return x(1, b, c);
585+
})()) {
586+
var c = 3, d = 4;
587+
x(1, b, c, d);
588+
}
589+
x(1, b, c, d);
590+
}
591+
}

0 commit comments

Comments
 (0)
Please sign in to comment.