Skip to content

Commit

Permalink
fix corner case in booleans (#3659)
Browse files Browse the repository at this point in the history 
fixes #3658
  • Loading branch information
@alexlamsl
alexlamsl committed Dec 31, 2019
1 parent 4dbdac9 commit 94785e8
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 35 deletions.
1 change: 1 addition & 0 deletions lib/ast.js
View file
Expand Up @@ -969,6 +969,7 @@ TreeWalker.prototype = {
|| p instanceof AST_DWLoop && p.condition === self
|| p instanceof AST_For && p.condition === self
|| p instanceof AST_If && p.condition === self
|| p instanceof AST_Return && p.in_bool
|| p instanceof AST_UnaryPrefix && p.operator == "!" && p.expression === self) {
return true;
}
Expand Down
142 changes: 107 additions & 35 deletions lib/compress.js
View file
Expand Up @@ -209,6 +209,7 @@ merge(Compressor.prototype, {
if (is_scope) {
node.hoist_properties(this);
node.hoist_declarations(this);
node.process_boolean_returns(this);
}
// Before https://github.com/mishoo/UglifyJS2/pull/1602 AST_Node.optimize()
// would call AST_Node.transform() if a different instance of AST_Node is
Expand Down Expand Up @@ -591,15 +592,22 @@ merge(Compressor.prototype, {
def(AST_Call, function(tw, descend) {
tw.find_parent(AST_Scope).may_call_this();
var exp = this.expression;
if (!(exp instanceof AST_SymbolRef)) return;
var def = exp.definition();
if (tw.in_boolean_context()) def.bool_fn++;
if (!(def.fixed instanceof AST_Defun)) return;
var defun = mark_defun(tw, def);
if (!defun) return;
descend();
defun.walk(tw);
return true;
if (exp instanceof AST_SymbolRef) {
var def = exp.definition();
if (this.TYPE == "Call" && tw.in_boolean_context()) def.bool_fn++;
if (!(def.fixed instanceof AST_Defun)) return;
var defun = mark_defun(tw, def);
if (!defun) return;
descend();
defun.walk(tw);
return true;
} else if (this.TYPE == "Call"
&& exp instanceof AST_Assign
&& exp.operator == "="
&& exp.left instanceof AST_SymbolRef
&& tw.in_boolean_context()) {
exp.left.definition().bool_fn++;
}
});
def(AST_Case, function(tw) {
push(tw);
Expand Down Expand Up @@ -4344,6 +4352,96 @@ merge(Compressor.prototype, {
self.body = dirs.concat(hoisted, self.body);
});

function scan_local_returns(fn, transform) {
fn.walk(new TreeWalker(function(node) {
if (node instanceof AST_Return) {
transform(node);
return true;
}
if (node instanceof AST_Scope && node !== fn) return true;
}));
}

function map_bool_returns(fn) {
var map = Object.create(null);
scan_local_returns(fn, function(node) {
var value = node.value;
if (value) value = value.tail_node();
if (value instanceof AST_SymbolRef) {
var id = value.definition().id;
map[id] = (map[id] || 0) + 1;
}
});
return map;
}

function all_bool(def, bool_returns, compressor) {
return def.bool_fn + (bool_returns[def.id] || 0) === def.references.length
&& !compressor.exposed(def);
}

function process_boolean_returns(fn, compressor) {
scan_local_returns(fn, function(node) {
node.in_bool = true;
var value = node.value;
if (value) {
var ev = value.is_truthy() || value.tail_node().evaluate(compressor);
if (!ev) {
value = value.drop_side_effect_free(compressor);
if (node.value !== value) node.value = value ? make_sequence(node.value, [
value,
make_node(AST_Number, node.value, {
value: 0
})
]) : null;
} else if (ev && !(ev instanceof AST_Node)) {
value = value.drop_side_effect_free(compressor);
if (node.value !== value) node.value = value ? make_sequence(node.value, [
value,
make_node(AST_Number, node.value, {
value: 1
})
]) : make_node(AST_Number, node.value, {
value: 1
});
}
}
});
}

AST_Scope.DEFMETHOD("process_boolean_returns", noop);
AST_Defun.DEFMETHOD("process_boolean_returns", function(compressor) {
if (!compressor.option("booleans")) return;
var bool_returns = map_bool_returns(this);
if (!all_bool(this.name.definition(), bool_returns, compressor)) return;
process_boolean_returns(this, compressor);
});
AST_Function.DEFMETHOD("process_boolean_returns", function(compressor) {
if (!compressor.option("booleans")) return;
var bool_returns = map_bool_returns(this);
if (this.name && !all_bool(this.name.definition(), bool_returns, compressor)) return;
var parent = compressor.parent();
if (parent instanceof AST_Assign) {
if (parent.operator != "=") return;
var sym = parent.left;
if (!(sym instanceof AST_SymbolRef)) return;
if (!all_bool(sym.definition(), bool_returns, compressor)) return;
} else if (parent instanceof AST_Call && parent.expression !== this) {
var exp = parent.expression;
if (exp instanceof AST_SymbolRef) exp = exp.fixed_value();
if (!(exp instanceof AST_Lambda)) return;
if (exp.uses_arguments || exp.pinned()) return;
var sym = exp.argnames[parent.args.indexOf(this)];
if (sym && !all_bool(sym.definition(), bool_returns, compressor)) return;
} else if (parent.TYPE == "Call") {
compressor.pop();
var in_bool = compressor.in_boolean_context();
compressor.push(this);
if (!in_bool) return;
} else return;
process_boolean_returns(this, compressor);
});

AST_Scope.DEFMETHOD("var_names", function() {
var var_names = this._var_names;
if (!var_names) {
Expand Down Expand Up @@ -6686,32 +6784,6 @@ merge(Compressor.prototype, {
if (compressor.option("reduce_vars") && is_lhs(compressor.self(), parent) !== compressor.self()) {
var def = self.definition();
var fixed = self.fixed_value();
if (compressor.option("booleans") && def.bool_fn === def.references.length && fixed instanceof AST_Lambda) {
def.bool_fn = null;
fixed.process_expression(false, function(node) {
if (!node.value) return node;
var value = node.value.is_truthy() || node.value.tail_node().evaluate(compressor);
if (!value) {
value = node.value.drop_side_effect_free(compressor);
node.value = value ? make_sequence(node.value, [
value,
make_node(AST_Number, node.value, {
value: 0
})
]) : null;
} else if (value && !(value instanceof AST_Node)) {
var num = make_node(AST_Number, node.value, {
value: 1
});
value = node.value.drop_side_effect_free(compressor);
node.value = value ? make_sequence(node.value, [
value,
num
]) : num;
}
return node;
});
}
var single_use = def.single_use && !(parent instanceof AST_Call && parent.is_expr_pure(compressor));
if (single_use && fixed instanceof AST_Lambda) {
if (def.scope !== self.scope
Expand Down
21 changes: 21 additions & 0 deletions test/compress/booleans.js
View file
Expand Up @@ -110,3 +110,24 @@ issue_2737_2: {
}
expect_stdout: "PASS"
}

issue_3658: {
options = {
booleans: true,
evaluate: true,
reduce_vars: true,
}
input: {
console.log(function f() {
console || f();
return "PASS";
}());
}
expect: {
console.log(function f() {
console || f();
return "PASS";
}());
}
expect_stdout: "PASS"
}

0 comments on commit 94785e8

Please sign in to comment.