fix corner cases in `collapse_vars` (#4978)

fixes #4977

7 files changed, 179 insertions, 18 deletions

diff --git a/lib/compress.js b/lib/compress.js
index 838a912e..6419c363 100644
--- a/lib/compress.js
+++ b/lib/compress.js
@@ -652,6 +652,7 @@ merge(Compressor.prototype, {
                 d.escaped.push(parent);
                 if (depth > 1 && !(value && value.is_constant_expression(scope))) depth = 1;
                 if (!d.escaped.depth || d.escaped.depth > depth) d.escaped.depth = depth;
+                if (d.scope !== scope) d.escaped.cross_scope = true;
                 return;
             } else if (value_in_use(node, parent)) {
                 mark_escaped(tw, d, scope, parent, parent, level + 1, depth);
@@ -1973,6 +1974,7 @@ merge(Compressor.prototype, {
                     var read_toplevel = false;
                     var modify_toplevel = false;
                     // Locate symbols which may execute code outside of scanning range
+                    var well_defined = true;
                     var lvalues = get_lvalues(candidate);
                     var lhs_local = is_lhs_local(lhs);
                     var rvalue = get_rvalue(candidate);
@@ -2173,11 +2175,18 @@ merge(Compressor.prototype, {
                 }
                 if (node instanceof AST_ObjectIdentity) return symbol_in_lvalues(node, parent);
                 if (node instanceof AST_PropAccess) {
+                    if (side_effects) return true;
                     var exp = node.expression;
-                    return side_effects
-                        || exp instanceof AST_SymbolRef && is_arguments(exp.definition())
-                        || !value_def && (in_try || !lhs_local)
-                            && !node.optional && exp.may_throw_on_access(compressor);
+                    if (exp instanceof AST_SymbolRef && is_arguments(exp.definition())) return true;
+                    if (compressor.option("unsafe")) {
+                        if (is_undeclared_ref(exp) && global_names[exp.name]) return false;
+                        if (is_static_fn(exp)) return false;
+                    }
+                    if (!well_defined) return true;
+                    if (value_def) return false;
+                    if (!in_try && lhs_local) return false;
+                    if (node.optional) return false;
+                    return exp.may_throw_on_access(compressor);
                 }
                 if (node instanceof AST_Spread) return true;
                 if (node instanceof AST_SymbolRef) {
@@ -2537,8 +2546,7 @@ merge(Compressor.prototype, {
                 }
                 if (parent instanceof AST_PropAccess) {
                     var exp = parent.expression;
-                    if (exp === node) return find_stop_value(parent, level + 1);
-                    return find_stop_expr(exp, find_stop_value, node, parent, level);
+                    return exp === node ? find_stop_value(parent, level + 1) : node;
                 }
                 if (parent instanceof AST_Sequence) {
                     return (parent.tail_node() === node ? find_stop_value : find_stop_unused)(parent, level + 1);
@@ -2770,13 +2778,23 @@ merge(Compressor.prototype, {
 
             function get_lvalues(expr) {
                 var lvalues = new Dictionary();
-                if (expr instanceof AST_VarDef) lvalues.add(expr.name.name, lhs);
+                if (expr instanceof AST_VarDef) {
+                    if (!expr.name.definition().fixed) well_defined = false;
+                    lvalues.add(expr.name.name, lhs);
+                }
                 var find_arguments = scope.uses_arguments && !compressor.has_directive("use strict");
                 var scan_toplevel = scope instanceof AST_Toplevel;
                 var tw = new TreeWalker(function(node) {
                     var value;
                     if (node instanceof AST_SymbolRef) {
-                        value = node.fixed_value() || node;
+                        value = node.fixed_value();
+                        if (!value) {
+                            value = node;
+                            var def = node.definition();
+                            if (!def.undeclared && (def.assignments || !def.escaped || def.escaped.cross_scope)) {
+                                well_defined = false;
+                            }
+                        }
                     } else if (node instanceof AST_ObjectIdentity) {
                         value = node;
                     }
@@ -2784,6 +2802,7 @@ merge(Compressor.prototype, {
                     if (find_arguments && node instanceof AST_Sub) {
                         scope.each_argname(function(argname) {
                             if (!compressor.option("reduce_vars") || argname.definition().assignments) {
+                                if (!argname.definition().fixed) well_defined = false;
                                 lvalues.add(argname.name, true);
                             }
                         });
@@ -4290,6 +4309,14 @@ merge(Compressor.prototype, {
         ],
     });
 
+    function is_static_fn(node) {
+        if (!(node instanceof AST_Dot)) return false;
+        var expr = node.expression;
+        if (!is_undeclared_ref(expr)) return false;
+        var static_fn = static_fns[expr.name];
+        return static_fn && (static_fn[node.property] || expr.name == "Math" && node.property == "random");
+    }
+
     // Accomodate when compress option evaluate=false
     // as well as the common constant expressions !0 and -1
     (function(def) {
@@ -4912,11 +4939,7 @@ merge(Compressor.prototype, {
                 if (global_pure_fns[expr.name]) return true;
                 if (this instanceof AST_New && global_pure_constructors[expr.name]) return true;
             }
-            if (expr instanceof AST_Dot && is_undeclared_ref(expr.expression)) {
-                var static_fn = static_fns[expr.expression.name];
-                return static_fn && (static_fn[expr.property]
-                    || expr.expression.name == "Math" && expr.property == "random");
-            }
+            if (is_static_fn(expr)) return true;
         }
         return compressor.option("annotations") && this.pure || !compressor.pure_funcs(this);
     });
diff --git a/test/compress/annotations.js b/test/compress/annotations.js
index 1d969c57..e85c8d81 100644
--- a/test/compress/annotations.js
+++ b/test/compress/annotations.js
@@ -256,6 +256,7 @@ issue_3858: {
         collapse_vars: true,
         inline: true,
         keep_fargs: false,
+        unsafe: true,
         unused: true,
     }
     input: {
diff --git a/test/compress/arrows.js b/test/compress/arrows.js
index 56355edb..be869512 100644
--- a/test/compress/arrows.js
+++ b/test/compress/arrows.js
@@ -421,6 +421,7 @@ collapse_value: {
         arrows: true,
         collapse_vars: true,
         keep_fargs: false,
+        unsafe: true,
         unused: true,
     }
     input: {
diff --git a/test/compress/classes.js b/test/compress/classes.js
index 11dc801f..86af3f85 100644
--- a/test/compress/classes.js
+++ b/test/compress/classes.js
@@ -632,6 +632,7 @@ collapse_non_strict: {
 collapse_rhs: {
     options = {
         collapse_vars: true,
+        unsafe: true,
     }
     input: {
         "use strict";
diff --git a/test/compress/collapse_vars.js b/test/compress/collapse_vars.js
index d2f27299..66190e3e 100644
--- a/test/compress/collapse_vars.js
+++ b/test/compress/collapse_vars.js
@@ -2264,6 +2264,7 @@ var_defs: {
         properties: true,
         sequences: true,
         side_effects: true,
+        unsafe: true,
         unused: true,
     }
     input: {
@@ -2486,6 +2487,7 @@ issue_1858: {
     options = {
         collapse_vars: true,
         pure_getters: true,
+        reduce_vars: true,
         unused: true,
     }
     input: {
@@ -2907,6 +2909,7 @@ compound_assignment_2: {
 issue_2187_1: {
     options = {
         collapse_vars: true,
+        reduce_vars: true,
         unused: true,
     }
     input: {
@@ -4271,6 +4274,7 @@ issue_2436_14: {
     options = {
         collapse_vars: true,
         reduce_vars: true,
+        unsafe: true,
         unused: true,
     }
     input: {
@@ -4761,6 +4765,7 @@ unsafe_builtin: {
     options = {
         collapse_vars: true,
         pure_getters: "strict",
+        reduce_vars: true,
         unsafe: true,
         unused: true,
     }
@@ -6965,6 +6970,7 @@ sequence_in_iife_2: {
         inline: true,
         passes: 2,
         side_effects: true,
+        unsafe: true,
         unused: true,
     }
     input: {
@@ -6976,7 +6982,7 @@ sequence_in_iife_2: {
     }
     expect: {
         var a = "foo", b = 42;
-        console.log(a, b = a);
+        console.log(b = a, b);
     }
     expect_stdout: "foo foo"
 }
@@ -6988,6 +6994,7 @@ sequence_in_iife_3: {
         passes: 2,
         side_effects: true,
         toplevel: true,
+        unsafe: true,
         unused: true,
     }
     input: {
@@ -7092,6 +7099,7 @@ setter_side_effect: {
 substitution_assign: {
     options = {
         collapse_vars: true,
+        unsafe: true,
     }
     input: {
         function f1(a, b) {
@@ -7112,8 +7120,7 @@ substitution_assign: {
     }
     expect: {
         function f1(a, b) {
-            f1 = a;
-            console.log(a, a);
+            console.log(f1 = a, a);
         }
         function f2(a, b) {
             a = 1 + (b = a);
@@ -7917,6 +7924,7 @@ issue_3744: {
 assign_value_def: {
     options = {
         collapse_vars: true,
+        reduce_vars: true,
         unused: true,
     }
     input: {
@@ -7957,6 +7965,7 @@ join_vars_value_def: {
     options = {
         collapse_vars: true,
         join_vars: true,
+        reduce_vars: true,
         unused: true,
     }
     input: {
@@ -7996,6 +8005,7 @@ join_vars_value_def: {
 var_value_def: {
     options = {
         collapse_vars: true,
+        reduce_vars: true,
         unused: true,
     }
     input: {
@@ -8033,6 +8043,7 @@ var_value_def: {
 mangleable_var: {
     options = {
         collapse_vars: true,
+        reduce_vars: true,
         unused: true,
     }
     input: {
@@ -8067,6 +8078,7 @@ mangleable_var: {
 mangleable_assignment_1: {
     options = {
         collapse_vars: true,
+        reduce_vars: true,
         unused: true,
     }
     input: {
@@ -8098,6 +8110,7 @@ mangleable_assignment_1: {
 mangleable_assignment_2: {
     options = {
         collapse_vars: true,
+        reduce_vars: true,
         unused: true,
     }
     input: {
@@ -8939,6 +8952,8 @@ dot_non_local: {
 issue_4806: {
     options = {
         collapse_vars: true,
+        reduce_vars: true,
+        toplevel: true,
     }
     input: {
         var a, o = {
@@ -8962,6 +8977,7 @@ issue_4806: {
 issue_4852: {
     options = {
         collapse_vars: true,
+        unsafe: true,
     }
     input: {
         var a = "PASS";
@@ -9195,7 +9211,7 @@ issue_4918: {
     expect_stdout: "PASS"
 }
 
-issue_4920: {
+issue_4920_1: {
     options = {
         collapse_vars: true,
         toplevel: true,
@@ -9221,6 +9237,64 @@ issue_4920: {
     expect_stdout: "PASS"
 }
 
+issue_4920_2: {
+    options = {
+        collapse_vars: true,
+        toplevel: true,
+    }
+    input: {
+        var o = {
+            get PASS() {
+                a = "FAIL";
+            },
+        };
+        var a = "PASS", b;
+        o[b = a];
+        console.log(b);
+    }
+    expect: {
+        var o = {
+            get PASS() {
+                a = "FAIL";
+            },
+        };
+        var a = "PASS", b;
+        o[b = a];
+        console.log(b);
+    }
+    expect_stdout: "PASS"
+}
+
+issue_4920_3: {
+    options = {
+        collapse_vars: true,
+        toplevel: true,
+    }
+    input: {
+        var log = console.log;
+        var o = {
+            get PASS() {
+                a = "FAIL";
+            },
+        };
+        var a = "PASS", b;
+        o[b = a];
+        log(b);
+    }
+    expect: {
+        var log = console.log;
+        var o = {
+            get PASS() {
+                a = "FAIL";
+            },
+        };
+        var a = "PASS", b;
+        o[b = a];
+        log(b);
+    }
+    expect_stdout: "PASS"
+}
+
 issue_4935: {
     options = {
         collapse_vars: true,
@@ -9279,3 +9353,62 @@ inline_throw: {
     }
     expect_stdout: "PASS"
 }
+
+issue_4977_1: {
+    options = {
+        collapse_vars: true,
+        unsafe: true,
+    }
+    input: {
+        var a = "FAIL";
+        var o = {
+            get p() {
+                return a;
+            }
+        };
+        a = "PASS";
+        console.log(o.p, a);
+    }
+    expect: {
+        var a = "FAIL";
+        var o = {
+            get p() {
+                return a;
+            }
+        };
+        a = "PASS";
+        console.log(o.p, a);
+    }
+    expect_stdout: "PASS PASS"
+}
+
+issue_4977_2: {
+    options = {
+        collapse_vars: true,
+        reduce_vars: true,
+        unsafe: true,
+    }
+    input: {
+        var a, o = {
+            get p() {
+                return a = "PASS";
+            },
+        };
+        if (console) {
+            var a = "FAIL";
+            console.log(o.p, a);
+        }
+    }
+    expect: {
+        var a, o = {
+            get p() {
+                return a = "PASS";
+            },
+        };
+        if (console) {
+            var a = "FAIL";
+            console.log(o.p, a);
+        }
+    }
+    expect_stdout: "PASS PASS"
+}
diff --git a/test/compress/keep_fargs.js b/test/compress/keep_fargs.js
index 4c1c1b7d..8761c330 100644
--- a/test/compress/keep_fargs.js
+++ b/test/compress/keep_fargs.js
@@ -117,6 +117,7 @@ issue_1858: {
         collapse_vars: true,
         keep_fargs: false,
         pure_getters: true,
+        reduce_vars: true,
         unused: true,
     }
     input: {
diff --git a/test/compress/sequences.js b/test/compress/sequences.js
index cc05fd7b..861ed5a6 100644
--- a/test/compress/sequences.js
+++ b/test/compress/sequences.js
@@ -727,7 +727,8 @@ side_effects_cascade_1: {
     }
     expect: {
         function f(a, b) {
-            b.a = a = (a -= 42) < 0 ? 0 : a;
+            (a -= 42) < 0 && (a = 0),
+            b.a = a;
         }
         var m = {}, n = {};
         f(13, m),