fix corner case in `collapse_vars` (#3239)
authorAlex Lam S.L <alexlamsl@gmail.com>
Wed, 29 Aug 2018 03:34:34 +0000 (11:34 +0800)
committerGitHub <noreply@github.com>
Wed, 29 Aug 2018 03:34:34 +0000 (11:34 +0800)
fixes #3238

lib/compress.js
test/compress/collapse_vars.js

index ad86dd8..f84409f 100644 (file)
@@ -1481,14 +1481,26 @@ merge(Compressor.prototype, {
             }
 
             function get_rhs(expr) {
-                if (!(candidate instanceof AST_Assign && candidate.operator == "=")) return;
-                return candidate.right;
+                return candidate instanceof AST_Assign && candidate.operator == "=" && candidate.right;
             }
 
             function get_rvalue(expr) {
                 return expr[expr instanceof AST_Assign ? "right" : "value"];
             }
 
+            function invariant(expr) {
+                if (expr instanceof AST_Array) return false;
+                if (expr instanceof AST_Binary && lazy_op[expr.operator]) {
+                    return invariant(expr.left) && invariant(expr.right);
+                }
+                if (expr instanceof AST_Call) return false;
+                if (expr instanceof AST_Conditional) {
+                    return invariant(expr.consequent) && invariant(expr.alternative);
+                }
+                if (expr instanceof AST_Object) return false;
+                return !expr.has_side_effects(compressor);
+            }
+
             function foldable(expr) {
                 if (expr instanceof AST_SymbolRef) {
                     var value = expr.evaluate(compressor);
@@ -1501,7 +1513,7 @@ merge(Compressor.prototype, {
                     return rhs_fuzzy_match(expr.evaluate(compressor), rhs_exact_match);
                 }
                 if (!(lhs instanceof AST_SymbolRef)) return false;
-                if (expr.has_side_effects(compressor)) return false;
+                if (!invariant(expr)) return false;
                 var circular;
                 var def = lhs.definition();
                 expr.walk(new TreeWalker(function(node) {
@@ -2980,19 +2992,21 @@ merge(Compressor.prototype, {
 
     // determine if expression has side effects
     (function(def) {
-        def(AST_Node, return_true);
-
-        def(AST_EmptyStatement, return_false);
-        def(AST_Constant, return_false);
-        def(AST_This, return_false);
-
         function any(list, compressor) {
             for (var i = list.length; --i >= 0;)
                 if (list[i].has_side_effects(compressor))
                     return true;
             return false;
         }
-
+        def(AST_Node, return_true);
+        def(AST_Array, function(compressor) {
+            return any(this.elements, compressor);
+        });
+        def(AST_Assign, return_true);
+        def(AST_Binary, function(compressor) {
+            return this.left.has_side_effects(compressor)
+                || this.right.has_side_effects(compressor);
+        });
         def(AST_Block, function(compressor) {
             return any(this.body, compressor);
         });
@@ -3004,19 +3018,24 @@ merge(Compressor.prototype, {
             }
             return any(this.args, compressor);
         });
-        def(AST_Switch, function(compressor) {
-            return this.expression.has_side_effects(compressor)
-                || any(this.body, compressor);
-        });
         def(AST_Case, function(compressor) {
             return this.expression.has_side_effects(compressor)
                 || any(this.body, compressor);
         });
-        def(AST_Try, function(compressor) {
-            return any(this.body, compressor)
-                || this.bcatch && this.bcatch.has_side_effects(compressor)
-                || this.bfinally && this.bfinally.has_side_effects(compressor);
+        def(AST_Conditional, function(compressor) {
+            return this.condition.has_side_effects(compressor)
+                || this.consequent.has_side_effects(compressor)
+                || this.alternative.has_side_effects(compressor);
         });
+        def(AST_Constant, return_false);
+        def(AST_Definitions, function(compressor) {
+            return any(this.definitions, compressor);
+        });
+        def(AST_Dot, function(compressor) {
+            return this.expression.may_throw_on_access(compressor)
+                || this.expression.has_side_effects(compressor);
+        });
+        def(AST_EmptyStatement, return_false);
         def(AST_If, function(compressor) {
             return this.condition.has_side_effects(compressor)
                 || this.body && this.body.has_side_effects(compressor)
@@ -3025,41 +3044,13 @@ merge(Compressor.prototype, {
         def(AST_LabeledStatement, function(compressor) {
             return this.body.has_side_effects(compressor);
         });
-        def(AST_SimpleStatement, function(compressor) {
-            return this.body.has_side_effects(compressor);
-        });
         def(AST_Lambda, return_false);
-        def(AST_Binary, function(compressor) {
-            return this.left.has_side_effects(compressor)
-                || this.right.has_side_effects(compressor);
-        });
-        def(AST_Assign, return_true);
-        def(AST_Conditional, function(compressor) {
-            return this.condition.has_side_effects(compressor)
-                || this.consequent.has_side_effects(compressor)
-                || this.alternative.has_side_effects(compressor);
-        });
-        def(AST_Unary, function(compressor) {
-            return unary_side_effects[this.operator]
-                || this.expression.has_side_effects(compressor);
-        });
-        def(AST_SymbolRef, function(compressor) {
-            return !this.is_declared(compressor);
-        });
-        def(AST_SymbolDeclaration, return_false);
         def(AST_Object, function(compressor) {
             return any(this.properties, compressor);
         });
         def(AST_ObjectProperty, function(compressor) {
             return this.value.has_side_effects(compressor);
         });
-        def(AST_Array, function(compressor) {
-            return any(this.elements, compressor);
-        });
-        def(AST_Dot, function(compressor) {
-            return this.expression.may_throw_on_access(compressor)
-                || this.expression.has_side_effects(compressor);
-        });
         def(AST_Sub, function(compressor) {
             return this.expression.may_throw_on_access(compressor)
                 || this.expression.has_side_effects(compressor)
@@ -3068,8 +3059,26 @@ merge(Compressor.prototype, {
         def(AST_Sequence, function(compressor) {
             return any(this.expressions, compressor);
         });
-        def(AST_Definitions, function(compressor) {
-            return any(this.definitions, compressor);
+        def(AST_SimpleStatement, function(compressor) {
+            return this.body.has_side_effects(compressor);
+        });
+        def(AST_Switch, function(compressor) {
+            return this.expression.has_side_effects(compressor)
+                || any(this.body, compressor);
+        });
+        def(AST_SymbolDeclaration, return_false);
+        def(AST_SymbolRef, function(compressor) {
+            return !this.is_declared(compressor);
+        });
+        def(AST_This, return_false);
+        def(AST_Try, function(compressor) {
+            return any(this.body, compressor)
+                || this.bcatch && this.bcatch.has_side_effects(compressor)
+                || this.bfinally && this.bfinally.has_side_effects(compressor);
+        });
+        def(AST_Unary, function(compressor) {
+            return unary_side_effects[this.operator]
+                || this.expression.has_side_effects(compressor);
         });
         def(AST_VarDef, function(compressor) {
             return this.value;
index 31eb238..d1c26c6 100644 (file)
@@ -5827,3 +5827,183 @@ issue_3215_4: {
     }
     expect_stdout: "number"
 }
+
+issue_3238_1: {
+    options = {
+        collapse_vars: true,
+        unsafe: true,
+    }
+    input: {
+        function f(a) {
+            var b, c;
+            if (a) {
+                b = Object.create(null);
+                c = Object.create(null);
+            }
+            return b === c;
+        }
+        console.log(f(0), f(1));
+    }
+    expect: {
+        function f(a) {
+            var b, c;
+            if (a) {
+                b = Object.create(null);
+                c = Object.create(null);
+            }
+            return b === c;
+        }
+        console.log(f(0), f(1));
+    }
+    expect_stdout: "true false"
+}
+
+issue_3238_2: {
+    options = {
+        collapse_vars: true,
+        unsafe: true,
+    }
+    input: {
+        function f(a) {
+            var b, c;
+            if (a) {
+                b = Error();
+                c = Error();
+            }
+            return b === c;
+        }
+        console.log(f(0), f(1));
+    }
+    expect: {
+        function f(a) {
+            var b, c;
+            if (a) {
+                b = Error();
+                c = Error();
+            }
+            return b === c;
+        }
+        console.log(f(0), f(1));
+    }
+    expect_stdout: "true false"
+}
+
+issue_3238_3: {
+    options = {
+        collapse_vars: true,
+        unsafe: true,
+    }
+    input: {
+        function f(a) {
+            var b, c;
+            if (a) {
+                b = new Date();
+                c = new Date();
+            }
+            return b === c;
+        }
+        console.log(f(0), f(1));
+    }
+    expect: {
+        function f(a) {
+            var b, c;
+            if (a) {
+                b = new Date();
+                c = new Date();
+            }
+            return b === c;
+        }
+        console.log(f(0), f(1));
+    }
+    expect_stdout: "true false"
+}
+
+issue_3238_4: {
+    options = {
+        collapse_vars: true,
+        unsafe: true,
+    }
+    input: {
+        function f(a) {
+            var b, c;
+            if (a) {
+                b = a && {};
+                c = a && {};
+            }
+            return b === c;
+        }
+        console.log(f(0), f(1));
+    }
+    expect: {
+        function f(a) {
+            var b, c;
+            if (a) {
+                b = a && {};
+                c = a && {};
+            }
+            return b === c;
+        }
+        console.log(f(0), f(1));
+    }
+    expect_stdout: "true false"
+}
+
+issue_3238_5: {
+    options = {
+        collapse_vars: true,
+        unsafe: true,
+    }
+    input: {
+        function f(a) {
+            var b, c;
+            if (a) {
+                b = a ? [] : 42;
+                c = a ? [] : 42;
+            }
+            return b === c;
+        }
+        console.log(f(0), f(1));
+    }
+    expect: {
+        function f(a) {
+            var b, c;
+            if (a) {
+                b = a ? [] : 42;
+                c = a ? [] : 42;
+            }
+            return b === c;
+        }
+        console.log(f(0), f(1));
+    }
+    expect_stdout: "true false"
+}
+
+issue_3238_6: {
+    options = {
+        collapse_vars: true,
+        unsafe: true,
+    }
+    input: {
+        function f(a) {
+            var b, c;
+            if (a) {
+                b = a && 0 || [];
+                c = a && 0 || [];
+            }
+            return b === c;
+        }
+        console.log(f(0), f(1));
+    }
+    expect: {
+        function f(a) {
+            var b, c;
+            if (a) {
+                b = a && 0 || [];
+                c = a && 0 || [];
+            }
+            return b === c;
+        }
+        console.log(f(0), f(1));
+    }
+    expect_stdout: "true false"
+}