compress duplicated variable definitions (#1817)

These are surprisingly common, as people reuse the same variable name within loops or switch branches.

diff --git a/lib/compress.js b/lib/compress.js
index cc42c46..7324fe0 100644 (file)
--- a/lib/compress.js
+++ b/lib/compress.js
@@ -1813,6 +1813,7 @@ merge(Compressor.prototype, {
                     }
                 });
             }
+            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).
@@ -1832,8 +1833,11 @@ merge(Compressor.prototype, {
                     }
                     if (node instanceof AST_Definitions && scope === self) {
                         node.definitions.forEach(function(def){
+                            var node_def = def.name.definition();
+                            if (def.name instanceof AST_SymbolVar) {
+                                var_defs_by_id.add(node_def.id, def);
+                            }
                             if (!drop_vars) {
-                                var node_def = def.name.definition();
                                 if (!(node_def.id in in_use_ids)) {
                                     in_use_ids[node_def.id] = true;
                                     in_use.push(node_def);
@@ -1943,19 +1947,29 @@ merge(Compressor.prototype, {
                     }
                     if (drop_vars && node instanceof AST_Definitions && !(tt.parent() instanceof AST_ForIn)) {
                         var def = node.definitions.filter(function(def){
-                            if (def.value) def.value = def.value.transform(tt);
-                            var sym = def.name.definition();
-                            if (sym.id in in_use_ids) return true;
-                            if (sym.orig[0] instanceof AST_SymbolCatch) {
-                                def.value = def.value && def.value.drop_side_effect_free(compressor);
-                                return true;
-                            }
                             var w = {
                                 name : def.name.name,
                                 file : def.name.start.file,
                                 line : def.name.start.line,
                                 col  : def.name.start.col
                             };
+                            if (def.value) def.value = def.value.transform(tt);
+                            var sym = def.name.definition();
+                            if (sym.id in in_use_ids) {
+                                if (def.name instanceof AST_SymbolVar) {
+                                    var var_defs = var_defs_by_id.get(sym.id);
+                                    if (var_defs.length > 1 && !def.value) {
+                                        compressor.warn("Dropping duplicated definition of variable {name} [{file}:{line},{col}]", w);
+                                        var_defs.splice(var_defs.indexOf(def), 1);
+                                        return false;
+                                    }
+                                }
+                                return true;
+                            }
+                            if (sym.orig[0] instanceof AST_SymbolCatch) {
+                                def.value = def.value && def.value.drop_side_effect_free(compressor);
+                                return true;
+                            }
                             if (def.value && (def._unused_side_effects = def.value.drop_side_effect_free(compressor))) {
                                 compressor.warn("Side effects in initialization of unused variable {name} [{file}:{line},{col}]", w);
                                 return true;
@@ -1963,6 +1977,28 @@ merge(Compressor.prototype, {
                             compressor[def.name.unreferenced() ? "warn" : "info"]("Dropping unused variable {name} [{file}:{line},{col}]", w);
                             return false;
                         });
+                        if (def.length == 1
+                            && def[0].value
+                            && !def[0]._unused_side_effects
+                            && def[0].name instanceof AST_SymbolVar) {
+                            var var_defs = var_defs_by_id.get(def[0].name.definition().id);
+                            if (var_defs.length > 1) {
+                                compressor.warn("Converting duplicated definition of variable {name} to assignment [{file}:{line},{col}]", {
+                                    name : def[0].name.name,
+                                    file : def[0].name.start.file,
+                                    line : def[0].name.start.line,
+                                    col  : def[0].name.start.col
+                                });
+                                var_defs.splice(var_defs.indexOf(def[0]), 1);
+                                return make_node(AST_SimpleStatement, node, {
+                                    body: make_node(AST_Assign, def[0], {
+                                        operator: "=",
+                                        left: make_node(AST_SymbolRef, def[0].name, def[0].name),
+                                        right: def[0].value
+                                    })
+                                });
+                            }
+                        }
                         // place uninitialized names at the start
                         def = mergeSort(def, function(a, b){
                             if (!a.value && b.value) return -1;

diff --git a/lib/scope.js b/lib/scope.js
index 2ffca25..c8d7c6b 100644 (file)
--- a/lib/scope.js
+++ b/lib/scope.js
@@ -268,7 +268,7 @@ AST_Scope.DEFMETHOD("init_scope_vars", function(parent_scope){
 AST_Lambda.DEFMETHOD("init_scope_vars", function(){
     AST_Scope.prototype.init_scope_vars.apply(this, arguments);
     this.uses_arguments = false;
-    this.def_variable(new AST_SymbolVar({
+    this.def_variable(new AST_SymbolConst({
         name: "arguments",
         start: this.start,
         end: this.end

diff --git a/test/compress/drop-unused.js b/test/compress/drop-unused.js
index 99d9cac..2eefbe8 100644 (file)
--- a/test/compress/drop-unused.js
+++ b/test/compress/drop-unused.js
@@ -1029,3 +1029,30 @@ delete_assign_2: {
     }
     expect_stdout: true
 }
+
+drop_var: {
+    options = {
+        toplevel: true,
+        unused: true,
+    }
+    input: {
+        var a;
+        console.log(a, b);
+        var a = 1, b = 2;
+        console.log(a, b);
+        var a = 3;
+        console.log(a, b);
+    }
+    expect: {
+        console.log(a, b);
+        var a = 1, b = 2;
+        console.log(a, b);
+        a = 3;
+        console.log(a, b);
+    }
+    expect_stdout: [
+        "undefined undefined",
+        "1 2",
+        "3 2",
+    ]
+}

diff --git a/test/compress/reduce_vars.js b/test/compress/reduce_vars.js
index 405dbc2..82b0021 100644 (file)
--- a/test/compress/reduce_vars.js
+++ b/test/compress/reduce_vars.js
@@ -1639,7 +1639,7 @@ redefine_arguments_1: {
             return typeof arguments;
         }
         function g() {
-            return"number";
+            return "number";
         }
         function h(x) {
             var arguments = x;
@@ -1951,7 +1951,6 @@ pure_getters_2: {
         var a = a && a.b;
     }
     expect: {
-        var a;
         var a = a && a.b;
     }
 }