Take operator || precendence into account for AST_If optimization.
authorkzc <zaxxon2011@gmail.com>
Sun, 21 Feb 2016 17:05:02 +0000 (12:05 -0500)
committerkzc <zaxxon2011@gmail.com>
Sun, 21 Feb 2016 17:05:02 +0000 (12:05 -0500)
Fixes #979.

lib/compress.js
test/compress/issue-979.js [new file with mode: 0644]

index ed4f62f..a2666fa 100644 (file)
@@ -1688,9 +1688,13 @@ merge(Compressor.prototype, {
         }
         if (is_empty(self.alternative)) self.alternative = null;
         var negated = self.condition.negate(compressor);
-        var negated_is_best = best_of(self.condition, negated) === negated;
+        var self_condition_length = self.condition.print_to_string().length;
+        var negated_length = negated.print_to_string().length;
+        var negated_is_best = negated_length < self_condition_length;
         if (self.alternative && negated_is_best) {
             negated_is_best = false; // because we already do the switch here.
+            // no need to swap values of self_condition_length and negated_length
+            // here because they are only used in an equality comparison later on.
             self.condition = negated;
             var tmp = self.body;
             self.body = self.alternative || make_node(AST_EmptyStatement);
@@ -1712,6 +1716,13 @@ merge(Compressor.prototype, {
             }).transform(compressor);
         }
         if (is_empty(self.alternative) && self.body instanceof AST_SimpleStatement) {
+            if (self_condition_length === negated_length && !negated_is_best
+                && self.condition instanceof AST_Binary && self.condition.operator == "||") {
+                // although the code length of self.condition and negated are the same,
+                // negated does not require additional surrounding parentheses.
+                // see https://github.com/mishoo/UglifyJS2/issues/979
+                negated_is_best = true;
+            }
             if (negated_is_best) return make_node(AST_SimpleStatement, self, {
                 body: make_node(AST_Binary, self, {
                     operator : "||",
diff --git a/test/compress/issue-979.js b/test/compress/issue-979.js
new file mode 100644 (file)
index 0000000..bae15db
--- /dev/null
@@ -0,0 +1,89 @@
+issue979_reported: {
+    options = {
+        sequences:true, properties:true, dead_code:true, conditionals:true,
+        comparisons:true, evaluate:true, booleans:true, loops:true, unused:true, hoist_funs:true,
+        keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true
+    }
+    input: {
+        function f1() {
+            if (a == 1 || b == 2) {
+                foo();
+            }
+        }
+        function f2() {
+            if (!(a == 1 || b == 2)) {
+            }
+            else {
+                foo();
+            }
+        }
+    }
+    expect: {
+        function f1() {
+            1!=a&&2!=b||foo();
+        }
+        function f2() {
+            1!=a&&2!=b||foo();
+        }
+    }
+}
+
+issue979_test_negated_is_best: {
+    options = {
+        sequences:true, properties:true, dead_code:true, conditionals:true,
+        comparisons:true, evaluate:true, booleans:true, loops:true, unused:true, hoist_funs:true,
+        keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true
+    }
+    input: {
+        function f3() {
+            if (a == 1 | b == 2) {
+                foo();
+            }
+        }
+        function f4() {
+            if (!(a == 1 | b == 2)) {
+            }
+            else {
+                foo();
+            }
+        }
+        function f5() {
+            if (a == 1 && b == 2) {
+                foo();
+            }
+        }
+        function f6() {
+            if (!(a == 1 && b == 2)) {
+            }
+            else {
+                foo();
+            }
+        }
+        function f7() {
+            if (a == 1 || b == 2) {
+                foo();
+            }
+            else {
+                return bar();
+            }
+        }
+    }
+    expect: {
+        function f3() {
+            1==a|2==b&&foo();
+        }
+        function f4() {
+            1==a|2==b&&foo();
+        }
+        function f5() {
+            1==a&&2==b&&foo();
+        }
+        function f6() {
+            1!=a||2!=b||foo();
+        }
+        function f7() {
+            return 1!=a&&2!=b?bar():void foo();
+        }
+    }
+}
+