fix deleting constants out of the scope they're used

lib/compress/common.js

@@ -105,34 +105,6 @@ export function make_sequence(orig, expressions) {
     });
 }
 
-export function to_statement(expr) {
-    if (expr instanceof AST_Statement) return expr;
-
-    // these do not inherit AST_Statement but are statements
-    if (expr instanceof AST_Defun || expr instanceof AST_DefClass) {
-        return expr;
-    }
-
-    return make_node(AST_SimpleStatement, expr, { body: expr });
-}
-
-export function merge_statements(array, node) {
-    if (node instanceof AST_BlockStatement && node.body.every(can_be_evicted_from_block)) {
-        array.push(...node.body);
-    } else {
-        array.push(to_statement(node));
-    }
-    return array;
-}
-
-export function make_statements(orig, array) {
-    if (array.length === 1) return to_statement(array[0]);
-    if (array.length === 0) return make_node(AST_EmptyStatement, orig);
-    return make_node(AST_BlockStatement, orig, {
-        body: array.reduce(merge_statements, [])
-    });
-}
-
 export function make_node_from_constant(val, orig) {
     switch (typeof val) {
       case "string":

lib/flow/dce.js

@@ -1,28 +1,28 @@
-import { TreeTransformer, AST_If, AST_Definitions, AST_Number, AST_Conditional } from "../ast.js";
+import { TreeTransformer, AST_If, AST_Definitions, AST_Number, AST_Conditional, AST_EmptyStatement, AST_BlockStatement } from "../ast.js";
 import { MAP, make_node } from "../utils/index.js";
 import { KNOWN_ELSE, KNOWN_THEN } from "./conditionals.js";
 import { World } from "./tools.js";
 
 // TODO move to utils?
-import { make_sequence, make_statements } from "../compress/common.js";
+import { make_sequence } from "../compress/common.js";
 
 export function flow_drop_dead_code(world = new World(), toplevel) {
     let tt = new TreeTransformer((node, descend, in_list) => {
         if (node instanceof AST_Conditional) {
             if (KNOWN_ELSE.has(node)) {
-                return keep_expression(tt, node, node.alternative);
+                return keep_expression(tt, node, node.alternative, in_list);
             }
             if (KNOWN_THEN.has(node)) {
-                return keep_expression(tt, node, node.consequent);
+                return keep_expression(tt, node, node.consequent, in_list);
             }
             return;
         }
         if (node instanceof AST_If) {
             if (KNOWN_ELSE.has(node)) {
-                return keep_statement(tt, node, node.alternative);
+                return keep_statement(tt, node, node.alternative, in_list);
             }
             if (KNOWN_THEN.has(node)) {
-                return keep_statement(tt, node, node.body);
+                return keep_statement(tt, node, node.body, in_list);
             }
             return;
         }
@@ -34,9 +34,13 @@ export function flow_drop_dead_code(world = new World(), toplevel) {
             for (const def of definitions) {
                 tt.push(def);
                 const binding = world.get_binding(def.name.name);
+
                 if (binding && binding.reads === 0) {
                     world.delete_binding(def.name.name);
-                    out_definitions.push(keep_statement(tt, def, def.value));
+                    const keep = def.value && def.value.drop_side_effect_free(tt);
+                    if (def.value) {
+                        out_definitions.push(keep_statement(tt, def, keep));
+                    }
                 } else {
                     const last = out_definitions[out_definitions.length - 1];
                     if (last instanceof AST_Definitions) {
@@ -47,6 +51,7 @@ export function flow_drop_dead_code(world = new World(), toplevel) {
                         }));
                     }
                 }
+
                 tt.pop();
             }
 
@@ -58,31 +63,21 @@ export function flow_drop_dead_code(world = new World(), toplevel) {
 }
 
 /** keep a statement around, but go into it */
-function keep_expression(tt, orig, node) {
-    if (!node) node = [];
-    if (!Array.isArray(node)) node = [node];
-    node = node.reduce((accum, node) => {
-        node = node && node.transform(tt);
-        if (node) accum.push(node);
-        return accum;
-    }, []);
+function keep_expression(tt, orig, node, in_list) {
+    const nodes = node ? MAP([node], tt) : [];
 
-    node = MAP(node, tt);
+    if (!nodes.length && in_list) return MAP.skip;
 
-    return node.length ? make_sequence(orig, node) : make_node(AST_Number, orig, { value: 0 });
+    return nodes.length ? make_sequence(orig, nodes) : make_node(AST_Number, orig, { value: 0 });
 }
 
 /** keep a statement around, but go into it */
-function keep_statement(tt, orig, node) {
-    if (!node) node = [];
-    if (!Array.isArray(node)) node = [node];
-    node = node.reduce((accum, node) => {
-        node = node && node.transform(tt).drop_side_effect_free(tt);
-        if (node) accum.push(node);
-        return accum;
-    }, []);
-
-    node = MAP(node, tt);
+function keep_statement(tt, orig, node, in_list) {
+    const nodes = node ? MAP([node], tt) : [];
 
-    return make_statements(orig, node);
+    switch (nodes.length) {
+        case 0: return in_list ? MAP.skip : make_node(AST_EmptyStatement, orig);
+        case 1: return nodes[0];
+        default: return make_node(AST_BlockStatement, orig, { body: nodes });
+    }
 }

lib/flow/tools.js

@@ -292,27 +292,29 @@ export class World {
         this.track_rw = track;
     }
 
-    write_variable(name, type, {is_definition = true} = {}) {
+    write_variable(name, type, {is_definition = true, track = this.track_rw} = {}) {
         let binding = this.variables.get(name);
 
         if (!binding) {
             if (!this.parent_scope) throw NOPE;
 
-            const written_type = this.parent_scope.write_variable(name, type, {is_definition});
-            return written_type;
+            return this.parent_scope.write_variable(name, type, {
+                is_definition,
+                track
+            });
         } else {
             if (this.track_rw) binding = binding.write(type, {is_definition});
             this.variables.set(binding.name, binding);
             return binding.type;
         }
     }
-    read_binding(name) {
+    read_binding(name, { track = this.track_rw } = {}) {
         if (!this.variables.has(name)) {
             if (!this.parent_scope) throw NOPE;
-            return this.parent_scope.read_binding(name);
+            return this.parent_scope.read_binding(name, { track });
         } else {
             let binding = this.variables.get(name);
-            if (this.track_rw) binding = binding.read();
+            if (track) binding = binding.read();
             this.variables.set(binding.name, binding);
             return binding;
         }

test/mocha/flow.js

@@ -282,7 +282,6 @@ describe('Flow analysis: powering DCE', () => {
             function fn() { return DEBUG ? 42 : -1 }
             fn()
         `, `
-            var DEBUG = 1
             function fn() { return 42 }
             fn()
         `);
@@ -292,7 +291,6 @@ describe('Flow analysis: powering DCE', () => {
             function fn() { if (DEBUG) { return 42 } else { return -1 } }
             fn()
         `, `
-            var DEBUG = 1
             function fn() { { return 42 } }
             fn()
         `);