fix corner case in `unused` (#4559)

fixes #4558

2 files changed, 89 insertions, 36 deletions

diff --git a/lib/compress.js b/lib/compress.js
index 01ec7e60..86ce9385 100644
--- a/lib/compress.js
+++ b/lib/compress.js
@@ -5453,6 +5453,7 @@ merge(Compressor.prototype, {
         var in_use_ids = Object.create(null); // avoid expensive linear scans of in_use
         var value_read = Object.create(null);
         var value_modified = Object.create(null);
+        var var_defs = Object.create(null);
         if (self instanceof AST_Toplevel && compressor.top_retain) {
             self.variables.each(function(def) {
                 if (compressor.top_retain(def) && !(def.id in in_use_ids)) {
@@ -5462,7 +5463,6 @@ merge(Compressor.prototype, {
             });
         }
         var assignments = new Dictionary();
-        var var_defs_by_id = new Dictionary();
         var initializations = new Dictionary();
         // pass 1: find out which symbols are directly used in
         // this scope (not in nested scopes).
@@ -5495,10 +5495,10 @@ merge(Compressor.prototype, {
                         defn.name.mark_symbol(function(name) {
                             if (!(name instanceof AST_SymbolDeclaration)) return;
                             var def = name.definition();
-                            var_defs_by_id.add(def.id, defn);
+                            var_defs[def.id] = (var_defs[def.id] || 0) + 1;
                             if (node instanceof AST_Var && def.orig[0] instanceof AST_SymbolCatch) {
                                 var redef = def.redefined();
-                                if (redef) var_defs_by_id.add(redef.id, defn);
+                                if (redef) var_defs[redef.id] = (var_defs[redef.id] || 0) + 1;
                             }
                             if (!(def.id in in_use_ids) && (!drop_vars
                                 || (node instanceof AST_Const ? def.redefined() : def.const_redefs)
@@ -5518,7 +5518,7 @@ merge(Compressor.prototype, {
                 }
                 if (node instanceof AST_SymbolFunarg) {
                     var def = node.definition();
-                    var_defs_by_id.add(def.id, node);
+                    var_defs[def.id] = (var_defs[def.id] || 0) + 1;
                     assignments.add(def.id, node);
                     return true;
                 }
@@ -5577,6 +5577,7 @@ merge(Compressor.prototype, {
             });
         };
         // pass 3: we should drop declarations not in_use
+        var trim_defns = [];
         var unused_fn_names = [];
         var calls_to_drop_args = [];
         var fns_with_marked_args = [];
@@ -5743,8 +5744,9 @@ merge(Compressor.prototype, {
                 var is_var = node instanceof AST_Var;
                 node.definitions.forEach(function(def) {
                     if (def.value) def.value = def.value.transform(tt);
+                    var value = def.value;
                     if (def.name instanceof AST_Destructured) {
-                        var name = trim_destructured(def.name, def.value, function(node) {
+                        var name = trim_destructured(def.name, value, function(node) {
                             if (!drop_vars) return node;
                             if (node.definition().id in in_use_ids) return node;
                             if (is_catch(node)) return node;
@@ -5754,7 +5756,7 @@ merge(Compressor.prototype, {
                         if (name) {
                             flush();
                         } else {
-                            var value = def.value.drop_side_effect_free(compressor);
+                            value = value.drop_side_effect_free(compressor);
                             if (value) side_effects.push(value);
                         }
                         return;
@@ -5762,27 +5764,25 @@ merge(Compressor.prototype, {
                     var sym = def.name.definition();
                     var drop_sym = is_var ? can_drop_symbol(def.name) : is_safe_lexical(sym);
                     if (!drop_sym || !drop_vars || sym.id in in_use_ids) {
-                        if (def.value && indexOf_assign(sym, def) < 0) {
-                            var write_only = def.value.write_only;
-                            var value = def.value.drop_side_effect_free(compressor);
+                        if (value && indexOf_assign(sym, def) < 0) {
+                            var write_only = value.write_only;
+                            value = value.drop_side_effect_free(compressor);
                             if (def.value !== value) {
-                                sym.references.forEach(function(node) {
-                                    if (node.fixed === sym.fixed) node.fixed = def.value;
-                                });
-                                def.value = null;
                                 if (value) {
                                     AST_Node.warn("Side effects in last use of variable {name} [{file}:{line},{col}]", template(def.name));
                                     side_effects.push(value);
                                 }
-                            } else if (def.value.write_only !== write_only) {
-                                def.value.write_only = write_only;
+                                value = null;
+                                trim_defns.push(def);
+                            } else if (value.write_only !== write_only) {
+                                value.write_only = write_only;
                             }
                         }
-                        var old_def, var_defs = var_defs_by_id.get(sym.id);
-                        if (!def.value && !(node instanceof AST_Let)) {
-                            if (drop_sym && var_defs.length > 1) {
+                        var old_def;
+                        if (!value && !(node instanceof AST_Let)) {
+                            if (drop_sym && var_defs[sym.id] > 1) {
                                 AST_Node.info("Dropping declaration of variable {name} [{file}:{line},{col}]", template(def.name));
-                                remove(var_defs, def);
+                                var_defs[sym.id]--;
                                 sym.eliminated++;
                             } else {
                                 head.push(def);
@@ -5790,20 +5790,20 @@ merge(Compressor.prototype, {
                         } else if (compressor.option("functions")
                             && !compressor.option("ie8")
                             && !(node instanceof AST_Const || node instanceof AST_Let)
-                            && var_defs.length == 1
+                            && var_defs[sym.id] == 1
                             && sym.assignments == 0
-                            && def.value instanceof AST_Function
+                            && value instanceof AST_Function
                             && (sym.references.length ? all(sym.references, function(ref) {
-                                    return def.value === ref.fixed_value();
-                                }) : def.value === def.name.fixed_value())
-                            && (!def.value.name || (old_def = def.value.name.definition()).assignments == 0
+                                    return value === ref.fixed_value();
+                                }) : value === def.name.fixed_value())
+                            && (!value.name || (old_def = value.name.definition()).assignments == 0
                                 && (old_def.name == def.name.name || all(old_def.references, function(ref) {
                                     return ref.scope.find_variable(def.name.name) === def.name.definition();
                                 })))
                             && can_declare_defun()
-                            && can_rename(def.value, def.name.name)) {
+                            && can_rename(value, def.name.name)) {
                             AST_Node.warn("Declaring {name} as function [{file}:{line},{col}]", template(def.name));
-                            var defun = make_node(AST_Defun, def, def.value);
+                            var defun = make_node(AST_Defun, def, value);
                             defun.name = make_node(AST_SymbolDefun, def.name, def.name);
                             var name_def = def.name.scope.resolve().def_function(defun.name);
                             if (old_def) old_def.forEach(function(node) {
@@ -5813,26 +5813,25 @@ merge(Compressor.prototype, {
                             });
                             body.push(defun);
                         } else {
-                            if (drop_sym && var_defs.length > 1 && sym.orig.indexOf(def.name) > sym.eliminated) {
-                                remove(var_defs, def);
+                            if (drop_sym && var_defs[sym.id] > 1 && sym.orig.indexOf(def.name) > sym.eliminated) {
+                                var_defs[sym.id]--;
                                 duplicated++;
                             }
                             flush();
                         }
                     } else if (is_catch(def.name)) {
-                        var value = def.value && def.value.drop_side_effect_free(compressor);
+                        value = value && value.drop_side_effect_free(compressor);
                         if (value) side_effects.push(value);
-                        var var_defs = var_defs_by_id.get(sym.id);
-                        if (var_defs.length > 1) {
+                        if (var_defs[sym.id] > 1) {
                             AST_Node.warn("Dropping duplicated declaration of variable {name} [{file}:{line},{col}]", template(def.name));
-                            remove(var_defs, def);
+                            var_defs[sym.id]--;
                             sym.eliminated++;
                         } else {
                             def.value = null;
                             head.push(def);
                         }
                     } else {
-                        var value = def.value && !def.value.single_use && def.value.drop_side_effect_free(compressor);
+                        value = value && !value.single_use && value.drop_side_effect_free(compressor);
                         if (value) {
                             AST_Node.warn("Side effects in initialization of unused variable {name} [{file}:{line},{col}]", template(def.name));
                             side_effects.push(value);
@@ -5865,9 +5864,9 @@ merge(Compressor.prototype, {
                                 body.push(make_node(AST_SimpleStatement, node, {
                                     body: make_sequence(node, side_effects)
                                 }));
-                            } else if (def.value) {
-                                side_effects.push(def.value);
-                                def.value = make_sequence(def.value, side_effects);
+                            } else if (value) {
+                                side_effects.push(value);
+                                def.value = make_sequence(value, side_effects);
                             } else {
                                 def.value = make_node(AST_UnaryPrefix, def, {
                                     operator: "void",
@@ -6017,6 +6016,9 @@ merge(Compressor.prototype, {
             && self.body[0].value == "use strict") {
             self.body.length = 0;
         }
+        trim_defns.forEach(function(def) {
+            def.value = null;
+        });
         unused_fn_names.forEach(function(fn) {
             fn.name = null;
         });
diff --git a/test/compress/drop-unused.js b/test/compress/drop-unused.js
index bb47682c..d00deca2 100644
--- a/test/compress/drop-unused.js
+++ b/test/compress/drop-unused.js
@@ -3197,3 +3197,54 @@ issue_4464_3: {
         "function",
     ]
 }
+
+issue_4558_1: {
+    options = {
+        evaluate: true,
+        pure_getters: true,
+        reduce_vars: true,
+        sequences: true,
+        side_effects: true,
+        toplevel: true,
+        unused: true,
+    }
+    input: {
+        var a = 0;
+        var b = 1, b = c >>>= a;
+        var c = 0;
+        b && 0[a++],
+        console.log(a);
+    }
+    expect: {
+        var a = 0;
+        var b = c >>>= a;
+        var c;
+        b && a++,
+        console.log(a);
+    }
+    expect_stdout: "0"
+}
+
+issue_4558_2: {
+    options = {
+        evaluate: true,
+        ie8: true,
+        reduce_vars: true,
+        unused: true,
+    }
+    input: {
+        (function() {
+            var a = 1;
+            var b = (a = NaN) || (console.log("PASS"), 2);
+            return a;
+        })();
+    }
+    expect: {
+        (function() {
+            var a;
+            (a = NaN) || console.log("PASS");
+            return a;
+        })();
+    }
+    expect_stdout: "PASS"
+}