Fix Scope.dup mixing up edge orders

Fix GraphVix node edge ordering

3 files changed, 19 insertions, 43 deletions

diff --git a/chapter05/src/main/java/com/seaofnodes/simple/GraphVisualizer.java b/chapter05/src/main/java/com/seaofnodes/simple/GraphVisualizer.java
index d277a980..b7e561e1 100644
--- a/chapter05/src/main/java/com/seaofnodes/simple/GraphVisualizer.java
+++ b/chapter05/src/main/java/com/seaofnodes/simple/GraphVisualizer.java
@@ -29,6 +29,9 @@ public class GraphVisualizer {
         // scopes and nodes.  If we try to cross subgraph cluster borders while
         // still making the subgraphs DOT gets confused.
         sb.append("\trankdir=BT;\n"); // Force Nodes before Scopes
+
+        // Preserve node input order
+        sb.append("\tordering=\"in\";\n");
         
         // Just the Nodes first, in a cluster no edges
         nodes(sb, all);
diff --git a/chapter05/src/main/java/com/seaofnodes/simple/node/Node.java b/chapter05/src/main/java/com/seaofnodes/simple/node/Node.java
index f2fddd1c..382d41ae 100644
--- a/chapter05/src/main/java/com/seaofnodes/simple/node/Node.java
+++ b/chapter05/src/main/java/com/seaofnodes/simple/node/Node.java
@@ -2,9 +2,7 @@ package com.seaofnodes.simple.node;
 
 import com.seaofnodes.simple.type.Type;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Objects;
+import java.util.*;
 
 /**
  * All Nodes in the Sea of Nodes IR inherit from the Node class.
@@ -187,14 +185,16 @@ public abstract class Node {
     Node set_def(int idx, Node new_def ) {
         Node old_def = in(idx);
         if( old_def == new_def ) return this; // No change
+        // If new def is not null, add the corresponding def->use edge
+        // This needs to happen before removing the old node's def->use edge as
+        // the new_def might get killed if the old node kills it recursively.
+        if( new_def != null )
+            new_def._outputs.add(this);
         if( old_def != null &&  // If the old def exists, remove a def->use edge
             old_def.delUse(this) ) // If we removed the last use, the old def is now dead
             old_def.kill();     // Kill old def
         // Set the new_def over the old (killed) edge
         _inputs.set(idx,new_def);
-        // If new def is not null, add the corresponding def->use edge
-        if( new_def != null )
-            new_def._outputs.add(this);
         // Return self for easy flow-coding
         return this;
     }
@@ -329,21 +329,4 @@ public abstract class Node {
      * Node unique id generator, and is done as part of making a new Parser.
      */
     public static void reset() { UNIQUE_ID = 1; }
-  
-    /*
-     * hashCode and equals implementation to be added in later chapter.
-     */
-
-    @Override
-    public boolean equals(Object o) {
-        if (this == o) return true;
-        if (o == null || getClass() != o.getClass()) return false;
-        Node node = (Node) o;
-        return _nid == node._nid && Objects.equals(_inputs, node._inputs) && Objects.equals(_type, node._type);
-    }
-
-    @Override
-    public int hashCode() {
-        return Objects.hash(_nid, _inputs, _type);
-    }
 }
diff --git a/chapter05/src/main/java/com/seaofnodes/simple/node/ScopeNode.java b/chapter05/src/main/java/com/seaofnodes/simple/node/ScopeNode.java
index ba4758e4..f09eeef5 100644
--- a/chapter05/src/main/java/com/seaofnodes/simple/node/ScopeNode.java
+++ b/chapter05/src/main/java/com/seaofnodes/simple/node/ScopeNode.java
@@ -19,7 +19,7 @@ public class ScopeNode extends Node {
     public final Stack<HashMap<String, Integer>> _scopes = new Stack<>();
 
     @Override
-    public String label() { return "Scope" + _nid; }
+    public String label() { return "Scope"; }
 
     @Override
     StringBuilder _print1(StringBuilder sb) {
@@ -47,8 +47,8 @@ public class ScopeNode extends Node {
     public Node idealize() { return null; }
 
     // Add an empty lexical scope
-    public void push() {
-        _scopes.push(new HashMap<>());
+    public HashMap<String, Integer> push() {
+        return _scopes.push(new HashMap<>());
     }
 
     // Remove the current lexical scope, killing all unused nodes.
@@ -97,19 +97,17 @@ public class ScopeNode extends Node {
     /**
      * Duplicate a ScopeNode; including all levels, up to Nodes.  So this is
      * neither shallow (would dup the Scope but not the internal HashMap
-     * tables), nor deep (would dup the Scope, the HashMap tables, but then the
-     * also program Nodes).
-     *
+     * tables), nor deep (would dup the Scope, the HashMap tables, but then
+     * also the program Nodes).
+     * <p>
      * The new Scope is a full-fledged Node with proper use<->def edges.
      */
     public ScopeNode dup() {
         ScopeNode dup = new ScopeNode();
-        for( HashMap<String, Integer> tab : _scopes ) {
-            dup.push();    // Create same level in the duplicate
-            for( String name : tab.keySet() )
-                // Ensure that dup is a user of n
-                dup.define(name, in(tab.get(name)));
-        }
+        for( HashMap<String, Integer> tab : _scopes )
+            dup._scopes.push((HashMap<String, Integer>) tab.clone());
+        for( int i=0; i<nIns(); i++ )
+            dup.add_def(in(i));
         return dup;
     }
 
@@ -122,12 +120,4 @@ public class ScopeNode extends Node {
         return r;
     }
 
-    /**
-     * Cleanup the scope and all its levels.
-     * Ensure that the scope is removed as user of all the defs.
-     */
-    public void clear() {
-        while (!_scopes.empty())
-            pop();
-    }
 }