fix corner cases in `varify` (#4719)

3 files changed, 115 insertions, 15 deletions

diff --git a/lib/compress.js b/lib/compress.js
index e5e822af..7f642797 100644
--- a/lib/compress.js
+++ b/lib/compress.js
@@ -7723,7 +7723,10 @@ merge(Compressor.prototype, {
             var name = self.init.definitions[0].name;
             if ((name instanceof AST_Destructured || name instanceof AST_SymbolLet)
                 && !name.match_symbol(function(node) {
-                    if (node instanceof AST_SymbolDeclaration) return may_overlap(compressor, node.definition());
+                    if (node instanceof AST_SymbolDeclaration) {
+                        var def = node.definition();
+                        return !same_scope(def) || may_overlap(compressor, def);
+                    }
                 }, true)) {
                 self.init = to_var(self.init);
             }
@@ -8269,7 +8272,7 @@ merge(Compressor.prototype, {
     function can_varify(compressor, sym) {
         if (!sym.fixed_value()) return false;
         var def = sym.definition();
-        return is_safe_lexical(def) && !may_overlap(compressor, def);
+        return is_safe_lexical(def) && same_scope(def) && !may_overlap(compressor, def);
     }
 
     function varify(self, compressor) {
@@ -10214,6 +10217,13 @@ merge(Compressor.prototype, {
         } while (node = compressor.parent(level++));
     }
 
+    function same_scope(def) {
+        var scope = def.scope.resolve();
+        return all(def.references, function(ref) {
+            return scope === ref.scope.resolve();
+        });
+    }
+
     OPT(AST_SymbolRef, function(self, compressor) {
         if (!compressor.option("ie8")
             && is_undeclared_ref(self)
@@ -10380,13 +10390,6 @@ merge(Compressor.prototype, {
         }
         return self;
 
-        function same_scope(def) {
-            var scope = def.scope.resolve();
-            return all(def.references, function(ref) {
-                return scope === ref.scope.resolve();
-            });
-        }
-
         function has_symbol_ref(value) {
             var found;
             value.walk(new TreeWalker(function(node) {
diff --git a/test/compress/varify.js b/test/compress/varify.js
index ab212842..f60c5474 100644
--- a/test/compress/varify.js
+++ b/test/compress/varify.js
@@ -354,6 +354,92 @@ forin_let_2: {
     node_version: ">=6"
 }
 
+loop_scope_1: {
+    options = {
+        toplevel: true,
+        varify: true,
+    }
+    input: {
+        "use strict";
+        var o = { foo: 1, bar: 2 };
+        for (let i in o) {
+            console.log(i);
+        }
+        for (const j in o)
+            setTimeout(() => console.log(j), 0);
+        for (let k in o)
+            setTimeout(function() {
+                console.log(k);
+            }, 0);
+    }
+    expect: {
+        "use strict";
+        var o = { foo: 1, bar: 2 };
+        for (var i in o)
+            console.log(i);
+        for (const j in o)
+            setTimeout(() => console.log(j), 0);
+        for (let k in o)
+            setTimeout(function() {
+                console.log(k);
+            }, 0);
+    }
+    expect_stdout: [
+        "foo",
+        "bar",
+        "foo",
+        "bar",
+        "foo",
+        "bar",
+    ]
+    node_version: ">=4"
+}
+
+loop_scope_2: {
+    options = {
+        reduce_vars: true,
+        toplevel: true,
+        varify: true,
+    }
+    input: {
+        "use strict";
+        var a = [ "foo", "bar" ];
+        for (var i = 0; i < a.length; i++) {
+            const x = a[i];
+            console.log(x);
+            let y = a[i];
+            setTimeout(() => console.log(y), 0);
+            const z = a[i];
+            setTimeout(function() {
+                console.log(z);
+            }, 0);
+        }
+    }
+    expect: {
+        "use strict";
+        var a = [ "foo", "bar" ];
+        for (var i = 0; i < a.length; i++) {
+            var x = a[i];
+            console.log(x);
+            let y = a[i];
+            setTimeout(() => console.log(y), 0);
+            const z = a[i];
+            setTimeout(function() {
+                console.log(z);
+            }, 0);
+        }
+    }
+    expect_stdout: [
+        "foo",
+        "bar",
+        "foo",
+        "foo",
+        "bar",
+        "bar",
+    ]
+    node_version: ">=4"
+}
+
 issue_4290_1_const: {
     options = {
         reduce_vars: true,
diff --git a/test/ufuzz/index.js b/test/ufuzz/index.js
index f38cdd96..191814c1 100644
--- a/test/ufuzz/index.js
+++ b/test/ufuzz/index.js
@@ -919,6 +919,12 @@ function getLabel(label) {
     return label && " L" + label;
 }
 
+function declareVarName(name, no_var) {
+    if (!SUPPORT.let || !no_var && rng(10)) return "var ";
+    block_vars.push(name);
+    return rng(2) ? "let " : "const ";
+}
+
 function createStatement(recurmax, canThrow, canBreak, canContinue, cannotReturn, stmtDepth, target) {
     ++stmtDepth;
     var loop = ++loops;
@@ -955,6 +961,8 @@ function createStatement(recurmax, canThrow, canBreak, canContinue, cannotReturn
         canContinue = label.continue || enableLoopControl(canContinue, CAN_CONTINUE);
         return label.target + "for (var brake" + loop + " = 5; " + createExpression(recurmax, COMMA_OK, stmtDepth, canThrow) + " && brake" + loop + " > 0; --brake" + loop + ")" + createStatement(recurmax, canThrow, canBreak, canContinue, cannotReturn, stmtDepth);
       case STMT_FOR_ENUM:
+        var block_len = block_vars.length;
+        var nameLenBefore = VAR_NAMES.length;
         var label = createLabel(canBreak, canContinue);
         canBreak = label.break || enableLoopControl(canBreak, CAN_BREAK);
         canContinue = label.continue || enableLoopControl(canContinue, CAN_CONTINUE);
@@ -963,12 +971,8 @@ function createStatement(recurmax, canThrow, canBreak, canContinue, cannotReturn
         var init = "";
         if (!/^key/.test(key)) {
             if (!(of && bug_for_of_var) && rng(10) == 0) init = "var ";
-        } else if (!SUPPORT.let || !(of && bug_for_of_var) && rng(10)) {
-            init = "var ";
-        } else if (rng(2)) {
-            init = "let ";
         } else {
-            init = "const ";
+            init = declareVarName(key, of && bug_for_of_var);
         }
         if (!SUPPORT.destructuring || of && !(canThrow && rng(20) == 0) || rng(10)) {
             init += key;
@@ -1003,8 +1007,15 @@ function createStatement(recurmax, canThrow, canBreak, canContinue, cannotReturn
             s += createExpression(recurmax, COMMA_OK, stmtDepth, canThrow) + "; ";
             s += label.target + " for (" + init + " in expr" + loop + ") {";
         }
-        if (rng(3)) s += "c = 1 + c; var " + createVarName(MANDATORY) + " = expr" + loop + "[" + key + "]; ";
+        if (/^key/.test(key)) VAR_NAMES.push(key);
+        if (rng(3)) {
+            s += "c = 1 + c; ";
+            var name = createVarName(MANDATORY);
+            s += declareVarName(name) + name + " = expr" + loop + "[" + key + "]; ";
+        }
         s += createStatement(recurmax, canThrow, canBreak, canContinue, cannotReturn, stmtDepth) + "}";
+        VAR_NAMES.length = nameLenBefore;
+        block_vars.length = block_len;
         return "{" + s + "}";
       case STMT_SEMI:
         return use_strict && rng(20) === 0 ? '"use strict";' : ";";