optimise binary operands with evaluate() (#1427)

- remove call to evaluate() in is_constant() and let nested optimize() does its job instead
- reject RegExp in is_constant() and remove special case logic under collapse_vars
- operands to conditionals optimisation are now always evaluate()-ed
- throw error in constant_value() instead of returning undefined to catch possible bugs, similar to make_node_from_constant()
- optimise binary boolean operators under `evaluate` instead of `conditionals`

1 files changed, 69 insertions, 46 deletions

diff --git a/lib/compress.js b/lib/compress.js
index 5c019623..4e45df92 100644
--- a/lib/compress.js
+++ b/lib/compress.js
@@ -347,7 +347,7 @@ merge(Compressor.prototype, {
                     if (ref.scope.uses_eval || ref.scope.uses_with) break;
 
                     // Constant single use vars can be replaced in any scope.
-                    if (!(var_decl.value instanceof AST_RegExp) && var_decl.value.is_constant(compressor)) {
+                    if (var_decl.value.is_constant()) {
                         var ctt = new TreeTransformer(function(node) {
                             if (node === ref)
                                 return replace_var(node, ctt.parent(), true);
@@ -1013,31 +1013,46 @@ merge(Compressor.prototype, {
             }
             return [ best_of(node, this), val ];
         });
-        AST_Node.DEFMETHOD("is_constant", function(compressor){
+        var unaryPrefix = makePredicate("! ~ - +");
+        AST_Node.DEFMETHOD("is_constant", function(){
             // Accomodate when compress option evaluate=false
-            // as well as the common constant expressions !0 and !1
-            return this instanceof AST_Constant
-                || (this instanceof AST_UnaryPrefix && this.operator == "!"
-                    && this.expression instanceof AST_Constant)
-                || this.evaluate(compressor).length > 1;
+            // as well as the common constant expressions !0 and -1
+            if (this instanceof AST_Constant) {
+                return !(this instanceof AST_RegExp);
+            } else {
+                return this instanceof AST_UnaryPrefix
+                    && this.expression instanceof AST_Constant
+                    && unaryPrefix(this.operator);
+            }
         });
         // Obtain the constant value of an expression already known to be constant.
-        // Result only valid iff this.is_constant(compressor) is true.
+        // Result only valid iff this.is_constant() is true.
         AST_Node.DEFMETHOD("constant_value", function(compressor){
             // Accomodate when option evaluate=false.
-            if (this instanceof AST_Constant) return this.value;
-            // Accomodate the common constant expressions !0 and !1 when option evaluate=false.
+            if (this instanceof AST_Constant && !(this instanceof AST_RegExp)) {
+                return this.value;
+            }
+            // Accomodate the common constant expressions !0 and -1 when option evaluate=false.
             if (this instanceof AST_UnaryPrefix
-                && this.operator == "!"
-                && this.expression instanceof AST_Constant) {
+                && this.expression instanceof AST_Constant) switch (this.operator) {
+              case "!":
                 return !this.expression.value;
+              case "~":
+                return ~this.expression.value;
+              case "-":
+                return -this.expression.value;
+              case "+":
+                return +this.expression.value;
+              default:
+                throw new Error(string_template("Cannot evaluate unary expression {value}", {
+                    value: this.print_to_string()
+                }));
             }
-            var result = this.evaluate(compressor)
+            var result = this.evaluate(compressor);
             if (result.length > 1) {
                 return result[1];
             }
-            // should never be reached
-            return undefined;
+            throw new Error(string_template("Cannot evaluate constant [{file}:{line},{col}]", this.start));
         });
         def(AST_Statement, function(){
             throw new Error(string_template("Cannot evaluate a statement [{file}:{line},{col}]", this.start));
@@ -2419,6 +2434,16 @@ merge(Compressor.prototype, {
     var commutativeOperators = makePredicate("== === != !== * & | ^");
 
     OPT(AST_Binary, function(self, compressor){
+        var lhs = self.left.evaluate(compressor);
+        var rhs = self.right.evaluate(compressor);
+        if (lhs.length > 1 && lhs[0].is_constant() !== self.left.is_constant()
+            || rhs.length > 1 && rhs[0].is_constant() !== self.right.is_constant()) {
+            return make_node(AST_Binary, self, {
+                operator: self.operator,
+                left: lhs[0],
+                right: rhs[0]
+            }).optimize(compressor);
+        }
         function reverse(op, force) {
             if (force || !(self.left.has_side_effects(compressor) || self.right.has_side_effects(compressor))) {
                 if (op) self.operator = op;
@@ -2491,32 +2516,6 @@ merge(Compressor.prototype, {
             }
             break;
         }
-        if (compressor.option("conditionals")) {
-            if (self.operator == "&&") {
-                var ll = self.left.evaluate(compressor);
-                if (ll.length > 1) {
-                    if (ll[1]) {
-                        compressor.warn("Condition left of && always true [{file}:{line},{col}]", self.start);
-                        return maintain_this_binding(compressor.parent(), self, self.right.evaluate(compressor)[0]);
-                    } else {
-                        compressor.warn("Condition left of && always false [{file}:{line},{col}]", self.start);
-                        return maintain_this_binding(compressor.parent(), self, ll[0]);
-                    }
-                }
-            }
-            else if (self.operator == "||") {
-                var ll = self.left.evaluate(compressor);
-                if (ll.length > 1) {
-                    if (ll[1]) {
-                        compressor.warn("Condition left of || always true [{file}:{line},{col}]", self.start);
-                        return maintain_this_binding(compressor.parent(), self, ll[0]);
-                    } else {
-                        compressor.warn("Condition left of || always false [{file}:{line},{col}]", self.start);
-                        return maintain_this_binding(compressor.parent(), self, self.right.evaluate(compressor)[0]);
-                    }
-                }
-            }
-        }
         if (compressor.option("booleans") && compressor.in_boolean_context()) switch (self.operator) {
           case "&&":
             var ll = self.left.evaluate(compressor);
@@ -2590,6 +2589,30 @@ merge(Compressor.prototype, {
             return self.left;
         }
         if (compressor.option("evaluate")) {
+            switch (self.operator) {
+              case "&&":
+                if (self.left.is_constant()) {
+                    if (self.left.constant_value(compressor)) {
+                        compressor.warn("Condition left of && always true [{file}:{line},{col}]", self.start);
+                        return maintain_this_binding(compressor.parent(), self, self.right);
+                    } else {
+                        compressor.warn("Condition left of && always false [{file}:{line},{col}]", self.start);
+                        return maintain_this_binding(compressor.parent(), self, self.left);
+                    }
+                }
+                break;
+              case "||":
+                if (self.left.is_constant()) {
+                    if (self.left.constant_value(compressor)) {
+                        compressor.warn("Condition left of || always true [{file}:{line},{col}]", self.start);
+                        return maintain_this_binding(compressor.parent(), self, self.left);
+                    } else {
+                        compressor.warn("Condition left of || always false [{file}:{line},{col}]", self.start);
+                        return maintain_this_binding(compressor.parent(), self, self.right);
+                    }
+                }
+                break;
+            }
             if (self.operator == "+") {
                 if (self.left instanceof AST_Constant
                     && self.right instanceof AST_Binary
@@ -2816,14 +2839,14 @@ merge(Compressor.prototype, {
             });
         }
         // y?1:1 --> 1
-        if (consequent.is_constant(compressor)
-            && alternative.is_constant(compressor)
+        if (consequent.is_constant()
+            && alternative.is_constant()
             && consequent.equivalent_to(alternative)) {
-            var consequent_value = consequent.constant_value(compressor);
+            var consequent_value = consequent.evaluate(compressor)[0];
             if (self.condition.has_side_effects(compressor)) {
-                return AST_Seq.from_array([self.condition, make_node_from_constant(compressor, consequent_value, self)]);
+                return AST_Seq.from_array([self.condition, consequent_value]);
             } else {
-                return make_node_from_constant(compressor, consequent_value, self);
+                return consequent_value;
             }
         }