Skip to content

Commit

Permalink
fix corner case in conditionals (#5674)
Browse files Browse the repository at this point in the history 
fixes #5673
  • Loading branch information
@alexlamsl
alexlamsl authored Sep 22, 2022
1 parent 63b04a6 commit 43ec350
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 27 deletions.
59 changes: 32 additions & 27 deletions lib/compress.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1828,6 +1828,29 @@ Compressor.prototype.compress = function(node) {
return wrap ? make_sequence(orig, [ make_node(AST_Number, orig, { value: 0 }), val ]) : val;
}

function merge_expression(base, target, scope) {
var fixed_by_id = new Dictionary();
base.walk(new TreeWalker(function(node) {
if (!(node instanceof AST_SymbolRef)) return;
var def = node.definition();
if (scope && def.scope.resolve() !== scope) return;
var fixed = node.fixed;
if (!fixed || !fixed_by_id.has(def.id)) {
fixed_by_id.set(def.id, fixed);
} else if (fixed_by_id.get(def.id) !== fixed) {
fixed_by_id.set(def.id, false);
}
}));
if (fixed_by_id.size() > 0) target.walk(new TreeWalker(function(node) {
if (!(node instanceof AST_SymbolRef)) return;
var def = node.definition();
var fixed = node.fixed;
if (!fixed || !fixed_by_id.has(def.id)) return;
if (fixed_by_id.get(def.id) !== fixed) node.fixed = false;
}));
return target;
}

function merge_sequence(array, node) {
if (node instanceof AST_Sequence) {
[].push.apply(array, node.expressions);
Expand Down Expand Up @@ -3813,25 +3836,7 @@ Compressor.prototype.compress = function(node) {
case 2:
value = value.tail_node();
}
var fixed_by_id = new Dictionary();
value.walk(new TreeWalker(function(node) {
if (!(node instanceof AST_SymbolRef)) return;
var def = node.definition();
if (def.scope.resolve() !== scope) return;
var fixed = node.fixed;
if (!fixed || !fixed_by_id.has(def.id)) {
fixed_by_id.set(def.id, fixed);
} else if (fixed_by_id.get(def.id) !== fixed) {
fixed_by_id.set(def.id, false);
}
}));
if (fixed_by_id.size() > 0) jump.value.walk(new TreeWalker(function(node) {
if (!(node instanceof AST_SymbolRef)) return;
var def = node.definition();
var fixed = node.fixed;
if (!fixed || !fixed_by_id.has(def.id)) return;
if (fixed_by_id.get(def.id) !== fixed) node.fixed = false;
}));
merge_expression(value, jump.value, scope);
}

function next_index(i) {
Expand Down Expand Up @@ -12933,7 +12938,7 @@ Compressor.prototype.compress = function(node) {
right: fuse(consequent, seq_tail, "condition"),
}),
consequent: seq_tail.consequent,
alternative: alternative,
alternative: merge_expression(seq_tail.alternative, alternative),
});
}
// x ? (y ? a : b) : a ---> !x || y ? a : b
Expand All @@ -12945,7 +12950,7 @@ Compressor.prototype.compress = function(node) {
operator: "||",
right: fuse(consequent, seq_tail, "condition"),
}),
consequent: alternative,
consequent: merge_expression(seq_tail.consequent, alternative),
alternative: seq_tail.alternative,
});
}
Expand All @@ -12958,7 +12963,7 @@ Compressor.prototype.compress = function(node) {
operator: "||",
right: fuse(alternative, alt_tail, "condition"),
}),
consequent: consequent,
consequent: merge_expression(consequent, alt_tail.consequent),
alternative: alt_tail.alternative,
});
}
Expand All @@ -12972,7 +12977,7 @@ Compressor.prototype.compress = function(node) {
right: fuse(alternative, alt_tail, "condition"),
}),
consequent: alt_tail.consequent,
alternative: consequent,
alternative: merge_expression(consequent, alt_tail.alternative),
});
}
// x ? y && a : a ---> (!x || y) && a
Expand All @@ -12986,7 +12991,7 @@ Compressor.prototype.compress = function(node) {
left: negated,
right: fuse(consequent, seq_tail, "left"),
}),
right: alternative,
right: merge_expression(seq_tail.right, alternative),
}).optimize(compressor);
}
// x ? y || a : a ---> x && y || a
Expand All @@ -13000,7 +13005,7 @@ Compressor.prototype.compress = function(node) {
left: condition,
right: fuse(consequent, seq_tail, "left"),
}),
right: alternative,
right: merge_expression(seq_tail.right, alternative),
}).optimize(compressor);
}
// x ? a : y && a ---> (x || y) && a
Expand All @@ -13014,7 +13019,7 @@ Compressor.prototype.compress = function(node) {
left: condition,
right: fuse(alternative, alt_tail, "left"),
}),
right: consequent,
right: merge_expression(consequent, alt_tail.right),
}).optimize(compressor);
}
// x ? a : y || a ---> !x && y || a
Expand All @@ -13028,7 +13033,7 @@ Compressor.prototype.compress = function(node) {
left: negated,
right: fuse(alternative, alt_tail, "left"),
}),
right: consequent,
right: merge_expression(consequent, alt_tail.right),
}).optimize(compressor);
}
var in_bool = compressor.option("booleans") && compressor.in_boolean_context();
Expand Down
42 changes: 42 additions & 0 deletions test/compress/conditionals.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2928,3 +2928,45 @@ issue_5666_2: {
}
expect_stdout: "NaN"
}

issue_5673_1: {
options = {
conditionals: true,
reduce_vars: true,
unused: true,
}
input: {
var a = "PASS", b = null;
console.log(function(c) {
return c || (b ? c : (c = a) && c);
}());
}
expect: {
var a = "PASS", b = null;
console.log(function(c) {
return c || (b || (c = a)) && c;
}());
}
expect_stdout: "PASS"
}

issue_5673_2: {
options = {
conditionals: true,
reduce_vars: true,
unused: true,
}
input: {
var a = "PASS";
console.log(function(b) {
return (b = a) ? b : (b = a) && b;
}());
}
expect: {
var a = "PASS";
console.log(function(b) {
return ((b = a) || (b = a)) && b;
}());
}
expect_stdout: "PASS"
}

0 comments on commit 43ec350

Please sign in to comment.