checkpoint

- discard statements with no side effects (unsafe? could be)
- safer hoist_vars (needs some revamping of scope/mangling)

4 files changed, 110 insertions, 31 deletions

diff --git a/bin/uglifyjs2 b/bin/uglifyjs2
index 8009de5f..b3788364 100755
--- a/bin/uglifyjs2
+++ b/bin/uglifyjs2
@@ -69,6 +69,7 @@ var output = UglifyJS.OutputStream({
 
 files = files.map(do_file_1);
 files = files.map(do_file_2);
+UglifyJS.base54.sort();
 files.forEach(do_file_3);
 if (ARGS.v) {
     sys.error("BASE54 digits: " + UglifyJS.base54.get());
@@ -124,7 +125,7 @@ function do_file_1(file) {
 
 function do_file_2(ast) {
     time_it("scope", function(){
-        //ast.figure_out_scope();
+        ast.figure_out_scope();
         ast.compute_char_frequency();
     });
     return ast;
diff --git a/lib/compress.js b/lib/compress.js
index d071d211..f401b930 100644
--- a/lib/compress.js
+++ b/lib/compress.js
@@ -64,8 +64,9 @@ function Compressor(options, false_by_default) {
         evaluate      : !false_by_default,
         booleans      : !false_by_default,
         loops         : !false_by_default,
+        unused_func   : !false_by_default,
         hoist_funs    : !false_by_default,
-        //hoist_vars    : !false_by_default, // XXX: turns out, this is really bad
+        hoist_vars    : !false_by_default,
 
         warnings      : true
     });
@@ -461,6 +462,36 @@ function Compressor(options, false_by_default) {
         node.DEFMETHOD("negate", func);
     });
 
+    // determine if expression has side effects
+    (function(def){
+        def(AST_Node, function(){ return true });
+
+        def(AST_EmptyStatement, function(){ return false });
+        def(AST_Constant, function(){ return false });
+        def(AST_This, function(){ return false });
+        def(AST_Function, function(){ return false });
+
+        def(AST_SimpleStatement, function(){
+            return this.body.has_side_effects();
+        });
+        def(AST_Binary, function(){
+            return this.left.has_side_effects()
+                || this.right.has_side_effects ();
+        });
+        def(AST_Conditional, function(){
+            return this.condition.has_side_effects()
+                || this.consequent.has_side_effects()
+                || this.alternative.has_side_effects();
+        });
+        def(AST_Unary, function(){
+            return this.operator == "delete"
+                || this.operator == "++"
+                || this.operator == "--";
+        });
+    })(function(node, func){
+        node.DEFMETHOD("has_side_effects", func);
+    });
+
     /* -----[ node squeezers ]----- */
 
     SQUEEZE(AST_Debugger, function(self, compressor){
@@ -508,49 +539,55 @@ function Compressor(options, false_by_default) {
             var self = this;
             var hoisted = [];
             var defuns = {};
-            var vars = {}, vars_found = 0;
+            var vars = {}, vars_found = 0, vardecl = [];
             var tw = new TreeWalker(function(node){
                 if (node !== self) {
-                    if (node instanceof AST_Defun && hoist_funs) {
+                    if (node instanceof AST_Defun && hoist_funs && !node.hoisted) {
                         hoisted.push(node.clone());
                         node.hoisted = true;
                         defuns[node.name.name] = true;
                     }
-                    if (node instanceof AST_Var && hoist_vars) {
+                    if (node instanceof AST_Var && hoist_vars && !node.hoisted) {
                         node.definitions.forEach(function(def){
                             vars[def.name.name] = def;
                             ++vars_found;
                         });
-                        node.hoisted = true;
+                        vardecl.push(node);
                     }
                     if (node instanceof AST_Scope)
                         return true;
                 }
             });
             self.walk(tw);
-            if (vars_found > 0) {
-                if (self instanceof AST_Lambda && !self.uses_arguments) {
-                    for (var i in vars) if (HOP(vars, i)) {
-                        var sym = vars[i].name;
-                        if (!find_if(function(arg){ return arg.name == sym.name }, self.argnames)) {
-                            self.argnames.push(sym);
-                        }
-                    }
-                } else {
-                    var node = make_node(AST_Var, self, {
-                        definitions: Object.keys(vars).map(function(name){
-                            var def = vars[name].clone();
-                            def.value = null;
-                            return def;
-                        })
-                    });
-                    hoisted.unshift(node);
-                }
+            if (vars_found > 0 && vardecl.length > 1) {
+                vardecl.forEach(function(v){ v.hoisted = true });
+                var node = make_node(AST_Var, self, {
+                    definitions: Object.keys(vars).map(function(name){
+                        var def = vars[name].clone();
+                        def.value = null;
+                        return def;
+                    })
+                });
+                hoisted.unshift(node);
             }
             self.body = hoisted.concat(self.body);
         }
     });
 
+    SQUEEZE(AST_SimpleStatement, function(self, compressor){
+        self = self.clone();
+        self.body = self.body.squeeze(compressor);
+        return self.optimize(compressor);
+    });
+
+    AST_SimpleStatement.DEFMETHOD("optimize", function(compressor){
+        if (!this.body.has_side_effects()) {
+            AST_Node.warn("Dropping side-effect-free statement [{line},{col}]", this.start);
+            return make_node(AST_EmptyStatement, this);
+        }
+        return this;
+    });
+
     SQUEEZE(AST_EmptyStatement, function(self, compressor){
         return self;
     });
@@ -790,9 +827,10 @@ function Compressor(options, false_by_default) {
     AST_Definitions.DEFMETHOD("to_assignments", function(){
         var assignments = this.definitions.reduce(function(a, def){
             if (def.value) {
+                var name = make_node(AST_SymbolRef, def.name, def.name);
                 a.push(make_node(AST_Assign, def, {
                     operator : "=",
-                    left     : def.name,
+                    left     : name,
                     right    : def.value
                 }));
             }
@@ -842,7 +880,31 @@ function Compressor(options, false_by_default) {
         self.argnames = do_list(self.argnames, compressor);
         self.hoist_declarations(compressor);
         self.body = tighten_body(self.body, compressor);
-        return self;
+        return self.optimize(compressor);
+    });
+
+    AST_Lambda.DEFMETHOD("optimize", function(compressor){
+        if (compressor.option("unused_func")) {
+            if (this.name && this.name.unreferenced()) {
+                this.name = null;
+            }
+        }
+        return this;
+    });
+
+    AST_Defun.DEFMETHOD("optimize", function(compressor){
+        if (compressor.option("unused_func")) {
+            if (this.name.unreferenced()
+                && !(this.parent_scope instanceof AST_Toplevel)) {
+                AST_Node.warn("Dropping unused function {name} [{line},{col}]", {
+                    name: this.name.name,
+                    line: this.start.line,
+                    col: this.start.col
+                });
+                return make_node(AST_EmptyStatement, this);
+            }
+        }
+        return this;
     });
 
     SQUEEZE(AST_Call, function(self, compressor){
diff --git a/lib/scope.js b/lib/scope.js
index e3433340..1224e604 100644
--- a/lib/scope.js
+++ b/lib/scope.js
@@ -50,7 +50,8 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(){
     // times on the same tree.
 
     // pass 1: setup scope chaining and handle definitions
-    var scope = this.parent_scope;
+    var self = this;
+    var scope = self.parent_scope = null;
     var labels = {};
     var tw = new TreeWalker(function(node, descend){
         if (node instanceof AST_Scope) {
@@ -110,7 +111,7 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(){
             node.reference(sym);
         }
     });
-    this.walk(tw);
+    self.walk(tw);
 
     // pass 2: find back references and eval
     var func = null;
@@ -137,7 +138,7 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(){
             }
         }
     });
-    this.walk(tw);
+    self.walk(tw);
 });
 
 AST_Scope.DEFMETHOD("init_scope_vars", function(){
@@ -364,6 +365,14 @@ AST_Toplevel.DEFMETHOD("compute_char_frequency", function(){
     var tw = new TreeWalker(function(node){
         if (node instanceof AST_Constant)
             base54.consider(node.print_to_string());
+        else if (node instanceof AST_Return)
+            base54.consider("return");
+        else if (node instanceof AST_Throw)
+            base54.consider("throw");
+        else if (node instanceof AST_Continue)
+            base54.consider("continue");
+        else if (node instanceof AST_Break)
+            base54.consider("break");
         else if (node instanceof AST_Debugger)
             base54.consider("debugger");
         else if (node instanceof AST_Directive)
@@ -420,7 +429,6 @@ AST_Toplevel.DEFMETHOD("compute_char_frequency", function(){
             base54.consider(node.property);
     });
     this.walk(tw);
-    base54.sort();
 });
 
 var base54 = (function() {
diff --git a/tmp/test-node.js b/tmp/test-node.js
index 85dc1b1a..4da5e9ff 100755
--- a/tmp/test-node.js
+++ b/tmp/test-node.js
@@ -10,5 +10,13 @@ var code = fs.readFileSync(filename, "utf8");
 
 var ast = UglifyJS.parse(code);
 ast.figure_out_scope();
+ast = ast.squeeze(UglifyJS.Compressor());
+
 ast.compute_char_frequency();
-console.log(UglifyJS.base54.get().join(","));
+UglifyJS.base54.sort();
+
+ast.figure_out_scope();
+ast.mangle_names();
+
+sys.error(UglifyJS.base54.get());
+sys.print(ast.print_to_string({ beautify: true }));