fix for js optimizer not noticing globals are modified

1 files changed, 56 insertions, 42 deletions

diff --git a/tools/js-optimizer.js b/tools/js-optimizer.js
index f8e65f55..aa4ba39b 100644
--- a/tools/js-optimizer.js
+++ b/tools/js-optimizer.js
@@ -35,7 +35,7 @@ var FUNCTION = set('defun', 'function');
 //     it replaces the passed node in the tree.
 //   @arg post: A callback to call after traversing all children.
 //   @arg stack: If true, a stack will be implemented: If pre does not push on
-//               the stack, we push a null. We pop when we leave the node. The
+//               the stack, we push a 0. We pop when we leave the node. The
 //               stack is passed as a third parameter to the callbacks.
 //   @returns: If the root node was replaced, the new root node. If the traversal
 //     was stopped, true. Otherwise undefined.
@@ -46,7 +46,7 @@ function traverse(node, pre, post, stack) {
     if (stack) len = stack.length;
     var result = pre(node, type, stack);
     if (result == true) return true;
-    if (stack && len == stack.length) stack.push(null);
+    if (stack && len == stack.length) stack.push(0);
   }
   for (var i = 0; i < node.length; i++) {
     var subnode = node[i];
@@ -66,6 +66,32 @@ function traverse(node, pre, post, stack) {
   return result;
 }
 
+// Walk the ast in a simple way, with an understanding of which JS variables are defined)
+function traverseWithVariables(ast, callback) {
+  traverse(ast, function(node, type, stack) {
+    if (type in FUNCTION) {
+      stack.push({ type: 'function', vars: node[2] });
+    } else if (type == 'var') {
+      // Find our function, add our vars
+      var func = stack[stack.length-1];
+      if (func) {
+        func.vars = func.vars.concat(node[1].map(function(varItem) { return varItem[0] }));
+      }
+    }
+  }, function(node, type, stack) {
+    if (type in FUNCTION) {
+      // We know all of the variables that are seen here, proceed to do relevant replacements
+      var allVars = stack.map(function(item) { return item ? item.vars : [] }).reduce(concatenator, []); // FIXME dictionary for speed?
+      traverse(node, function(node, type, stack) {
+        // Be careful not to look into our inner functions. They have already been processed.
+        if (sum(stack) > 1) return;
+        if (type in FUNCTION) stack.push(1);
+        return callback(node, type, allVars);
+      }, null, []);
+    }
+  }, []);
+}
+
 function makeEmptyNode() {
   return ['toplevel', []]
 }
@@ -81,59 +107,47 @@ function unGlobalize(ast) {
   // Find global renamings of the relevant values
   ast[1].forEach(function(node, i) {
     if (node[0] != 'var') return;
-    node[1] = node[1].filter(function(varItem) {
-      var key = varItem[0];
+    node[1] = node[1].filter(function(varItem, j) {
+      var ident = varItem[0];
       var value = varItem[1];
       if (!value) return true;
+      var possible = false;
       if (value[0] == 'name' && value[1] == 'null') {
-        values[key] = ['name','null'];
-        return false;
-      } else if (value[0] == 'unary-prefix' && value[1] == 'void' && value[2] && value[2].length == 2 && value[2][0] == 'num' && value[2][1] == 0){
-        values[key] = ['unary-prefix','void',['num',0]];
-        return false;
-      } else if (value[0] == 'unary-prefix' && value[1] == '!' && value[2] && value[2].length == 2 && value[2][0] == 'num' && value[2][1] == 0){
-        values[key] = ['unary-prefix','!',['num',0]];
-        return false;
-      } else if (value[0] == 'unary-prefix' && value[1] == '!' && value[2] && value[2].length == 2 && value[2][0] == 'num' && value[2][1] == 1){
-        values[key] = ['unary-prefix','!',['num',1]];
+        possible = true;
+      } else if (value[0] == 'unary-prefix' && value[1] == 'void' && value[2] && value[2].length == 2 && value[2][0] == 'num' && value[2][1] == 0) {
+        possible = true;
+      } else if (value[0] == 'unary-prefix' && value[1] == '!' && value[2] && value[2].length == 2 && value[2][0] == 'num' && value[2][1] == 0) {
+        possible = true;
+      } else if (value[0] == 'unary-prefix' && value[1] == '!' && value[2] && value[2].length == 2 && value[2][0] == 'num' && value[2][1] == 1) {
+        possible = true;
+      }
+      if (!possible) return true;
+      // Make sure there are no assignments to this variable. (This isn't fast, we traverse many times..)
+      ast[1][i][1][j] = makeEmptyNode();
+      var assigned = false;
+      traverseWithVariables(ast, function(node, type, allVars) {
+        if (type == 'assign' && node[2][0] == 'name' && node[2][1] == ident) assigned = true;
+      });
+      ast[1][i][1][j] = [ident, value];
+      if (!assigned) {
+        values[ident] = value;
         return false;
       }
       return true;
     });
+
     if (node[1].length == 0) {
       ast[1][i] = makeEmptyNode();
     }
   });
-  // Walk the ast, with an understanding of JS variable scope (i.e., function-level scoping)
-  traverse(ast, function(node, type, stack) {
-    if (type in FUNCTION) {
-      stack.push({ type: 'function', vars: node[2] });
-    } else if (type == 'var') {
-      // Find our function, add our vars
-      var func = stack[stack.length-1];
-      if (func) {
-        func.vars = func.vars.concat(node[1].map(function(varItem) { return varItem[0] }));
+  traverseWithVariables(ast, function(node, type, allVars) {
+    if (type == 'name') {
+      var ident = node[1];
+      if (ident in values && allVars.indexOf(ident) < 0) {
+        return copy(values[ident]);
       }
     }
-  }, function(node, type, stack) {
-    if (type in FUNCTION) {
-      // We know all of the variables that are seen here, proceed to do relevant replacements
-      var allVars = stack.map(function(item) { return item ? item.vars : [] }).reduce(concatenator, []); // FIXME dictionary for speed?
-      traverse(node, function(node, type, stack) {
-        // Be careful not to look into our inner functions. They have already been processed.
-        if (type == 'name' && sum(stack) == 1) {
-          var ident = node[1];
-          if (ident in values && allVars.indexOf(ident) < 0) {
-            return copy(values[ident]);
-          }
-        } else if (type in FUNCTION) {
-          stack.push(1);
-        } else {
-          stack.push(0);
-        }
-      }, null, []);
-    }
-  }, []);
+  });
 }
 
 // Main