8005842: Loops in ASTWriter. Corrected @Reference and @Ignore node annotation for IR nodes

Reviewed-by: hannesw, sundar

nashorn/src/jdk/nashorn/internal/ir/Block.java

@@ -44,7 +44,6 @@ import jdk.nashorn.internal.codegen.Frame;
 import jdk.nashorn.internal.codegen.MethodEmitter.Label;
 import jdk.nashorn.internal.ir.annotations.Ignore;
 import jdk.nashorn.internal.ir.annotations.ParentNode;
-import jdk.nashorn.internal.ir.annotations.Reference;
 import jdk.nashorn.internal.ir.visitor.NodeVisitor;
 import jdk.nashorn.internal.runtime.Source;
 
@@ -55,11 +54,11 @@ import jdk.nashorn.internal.runtime.Source;
  */
 public class Block extends Node {
     /** Parent context */
-    @ParentNode @Reference
+    @ParentNode @Ignore
     private Block parent;
 
     /** Owning function. */
-    @Ignore @Reference
+    @Ignore //don't print it, it is apparent in the tree
     protected FunctionNode function;
 
     /** List of statements */

nashorn/src/jdk/nashorn/internal/ir/FunctionNode.java

@@ -46,7 +46,6 @@ import jdk.nashorn.internal.codegen.Splitter;
 import jdk.nashorn.internal.codegen.Transform;
 import jdk.nashorn.internal.codegen.types.Type;
 import jdk.nashorn.internal.ir.annotations.Ignore;
-import jdk.nashorn.internal.ir.annotations.Reference;
 import jdk.nashorn.internal.ir.visitor.NodeVisitor;
 import jdk.nashorn.internal.parser.Parser;
 import jdk.nashorn.internal.runtime.Source;
@@ -142,11 +141,11 @@ public class FunctionNode extends Block {
     private final List<VarNode> declarations;
 
     /** VarNode for this function statement */
-    @Reference @Ignore //this is explicit code anyway and should not be traverseda after lower
+    @Ignore //this is explicit code anyway and should not be traversed after lower
     private VarNode funcVarNode;
 
     /** Line number for function declaration */
-    @Reference @Ignore
+    @Ignore
     private LineNumberNode funcVarLineNumberNode;
 
     /** Function flags. */

nashorn/src/jdk/nashorn/internal/ir/LabelNode.java

@@ -25,7 +25,7 @@
 
 package jdk.nashorn.internal.ir;
 
-import jdk.nashorn.internal.ir.annotations.Reference;
+import jdk.nashorn.internal.ir.annotations.Ignore;
 import jdk.nashorn.internal.ir.visitor.NodeVisitor;
 import jdk.nashorn.internal.runtime.Source;
 
@@ -42,11 +42,11 @@ public class LabelNode extends Node {
     private Block body;
 
     /** Node to break from. */
-    @Reference
+    @Ignore
     private Node breakNode;
 
     /** Node to continue. */
-    @Reference
+    @Ignore
     private Node continueNode;
 
     /**

nashorn/src/jdk/nashorn/internal/ir/LabeledNode.java

@@ -26,7 +26,6 @@
 package jdk.nashorn.internal.ir;
 
 import jdk.nashorn.internal.ir.annotations.Ignore;
-import jdk.nashorn.internal.ir.annotations.Reference;
 import jdk.nashorn.internal.runtime.Source;
 
 /**
@@ -39,11 +38,11 @@ public abstract class LabeledNode extends Node {
     protected final LabelNode labelNode;
 
     /** Target control node. */
-    @Reference @Ignore
+    @Ignore
     protected final Node targetNode;
 
     /** Try chain. */
-    @Reference
+    @Ignore
     protected final TryNode tryChain;
 
     /** scope nesting level */

nashorn/src/jdk/nashorn/internal/ir/ObjectNode.java

@@ -29,7 +29,6 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import jdk.nashorn.internal.ir.annotations.Ignore;
-import jdk.nashorn.internal.ir.annotations.Reference;
 import jdk.nashorn.internal.ir.visitor.NodeVisitor;
 import jdk.nashorn.internal.runtime.Source;
 
@@ -38,7 +37,7 @@ import jdk.nashorn.internal.runtime.Source;
  */
 public class ObjectNode extends Node {
     /** Literal context. */
-    @Reference @Ignore
+    @Ignore
     private Block context;
 
     /** Literal elements. */

nashorn/src/jdk/nashorn/internal/ir/ReferenceNode.java

@@ -25,7 +25,6 @@
 
 package jdk.nashorn.internal.ir;
 
-import jdk.nashorn.internal.ir.annotations.Ignore;
 import jdk.nashorn.internal.ir.annotations.Reference;
 import jdk.nashorn.internal.ir.visitor.NodeVisitor;
 import jdk.nashorn.internal.runtime.Source;
@@ -35,7 +34,7 @@ import jdk.nashorn.internal.runtime.Source;
  */
 public class ReferenceNode extends Node {
     /** Node referenced. */
-    @Reference @Ignore
+    @Reference
     private final FunctionNode reference;
 
     /**

nashorn/src/jdk/nashorn/internal/ir/ReturnNode.java

@@ -29,7 +29,6 @@ import static jdk.nashorn.internal.parser.TokenType.RETURN;
 import static jdk.nashorn.internal.parser.TokenType.YIELD;
 
 import jdk.nashorn.internal.ir.annotations.Ignore;
-import jdk.nashorn.internal.ir.annotations.Reference;
 import jdk.nashorn.internal.ir.visitor.NodeVisitor;
 import jdk.nashorn.internal.runtime.Source;
 
@@ -42,7 +41,7 @@ public class ReturnNode extends Node {
     private Node expression;
 
     /** Try chain. */
-    @Reference @Ignore
+    @Ignore
     private final TryNode tryChain;
 
     /**

nashorn/src/jdk/nashorn/internal/ir/SplitNode.java

@@ -30,6 +30,8 @@ import java.util.Collections;
 import java.util.List;
 import jdk.nashorn.internal.codegen.CompileUnit;
 import jdk.nashorn.internal.codegen.MethodEmitter;
+import jdk.nashorn.internal.ir.annotations.Ignore;
+import jdk.nashorn.internal.ir.annotations.Reference;
 import jdk.nashorn.internal.ir.visitor.NodeVisitor;
 
 
@@ -50,15 +52,18 @@ public class SplitNode extends Node {
     private MethodEmitter caller;
 
     /** Containing function. */
+    @Reference
     private final FunctionNode functionNode;
 
     /** A list of target labels in parent methods this split node may encounter. */
+    @Ignore
     private final List<MethodEmitter.Label> externalTargets;
 
     /** True if this split node or any of its children contain a return statement. */
     private boolean hasReturn;
 
     /** Body of split code. */
+    @Ignore
     private Node body;
 
     /**

nashorn/src/jdk/nashorn/internal/ir/SwitchNode.java

@@ -30,7 +30,6 @@ import java.util.Collections;
 import java.util.List;
 import jdk.nashorn.internal.codegen.MethodEmitter.Label;
 import jdk.nashorn.internal.ir.annotations.Ignore;
-import jdk.nashorn.internal.ir.annotations.Reference;
 import jdk.nashorn.internal.ir.visitor.NodeVisitor;
 import jdk.nashorn.internal.runtime.Source;
 
@@ -48,7 +47,7 @@ public class SwitchNode extends BreakableNode {
     private List<CaseNode> cases;
 
     /** Switch default. */
-    @Reference @Ignore //points to one of the members in the list above, don't traverse twice
+    @Ignore //points to one of the members in the list above, don't traverse twice
     private CaseNode defaultCase;
 
     /**

nashorn/src/jdk/nashorn/internal/ir/ThrowNode.java

@@ -26,7 +26,6 @@
 package jdk.nashorn.internal.ir;
 
 import jdk.nashorn.internal.ir.annotations.Ignore;
-import jdk.nashorn.internal.ir.annotations.Reference;
 import jdk.nashorn.internal.ir.visitor.NodeVisitor;
 import jdk.nashorn.internal.runtime.Source;
 
@@ -38,7 +37,7 @@ public class ThrowNode extends Node {
     private Node expression;
 
     /** Try chain. */
-    @Reference @Ignore
+    @Ignore
     private final TryNode tryChain;
 
     /**

nashorn/src/jdk/nashorn/internal/ir/TryNode.java

@@ -29,7 +29,7 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import jdk.nashorn.internal.codegen.MethodEmitter.Label;
-import jdk.nashorn.internal.ir.annotations.Reference;
+import jdk.nashorn.internal.ir.annotations.Ignore;
 import jdk.nashorn.internal.ir.visitor.NodeVisitor;
 import jdk.nashorn.internal.runtime.Source;
 
@@ -38,7 +38,7 @@ import jdk.nashorn.internal.runtime.Source;
  */
 public class TryNode extends Node {
     /** Try chain. */
-    @Reference
+    @Ignore //don't print, will be apparent from the AST
     private TryNode next;
 
     /** Try statements. */

nashorn/src/jdk/nashorn/internal/ir/debug/ASTWriter.java

@@ -71,12 +71,12 @@ public final class ASTWriter {
     @Override
     public String toString() {
         final StringBuilder sb = new StringBuilder();
-        printAST(sb, "root", root, 0);
+        printAST(sb, null, "root", root, 0);
         return sb.toString();
     }
 
     @SuppressWarnings("unchecked")
-    private void printAST(final StringBuilder sb, final String name, final Node node, final int indent) {
+    private void printAST(final StringBuilder sb, final Field field, final String name, final Node node, final int indent) {
         ASTWriter.indent(sb, indent);
         if (node == null) {
             sb.append("[Object ");
@@ -85,59 +85,18 @@ public final class ASTWriter {
             return;
         }
 
+        final boolean isReference = field != null && field.getAnnotation(Reference.class) != null;
+
         Class<?> clazz = node.getClass();
-        String type    = clazz.getName();
+        String   type  = clazz.getName();
+
         type = type.substring(type.lastIndexOf('.') + 1, type.length());
- //       type += "@" + Debug.id(node) + "#" + node.getSymbol();
+//        type += "@" + Debug.id(node) + "#" + node.getSymbol();
 
         final List<Field> children = new LinkedList<>();
-        final Deque<Class<?>> stack = new ArrayDeque<>();
 
-        /**
+        if (!isReference) {
-         * Here is some ugliness that can be overcome by proper ChildNode annotations
+            enqueueChildren(node, clazz, children);
-         * with proper orders. Right now we basically sort all classes up to Node
-         * with super class first, as this often is the natural order, e.g. base
-         * before index for an IndexNode.
-         *
-         * Also there are special cases as this is not true for UnaryNodes(lhs) and
-         * BinaryNodes extends UnaryNode (with lhs), and TernaryNodes.
-         *
-         * TODO - generalize traversal with an order built on annotations and this
-         * will go away.
-         */
-        do {
-            stack.push(clazz);
-            clazz = clazz.getSuperclass();
-        } while (clazz != null);
-
-        if (node instanceof TernaryNode) {
-            // HACK juggle "third"
-            stack.push(stack.removeLast());
-        }
-        // HACK change operator order for BinaryNodes to get lhs first.
-        final Iterator<Class<?>> iter = node instanceof BinaryNode ? stack.descendingIterator() : stack.iterator();
-
-        while (iter.hasNext()) {
-            final Class<?> c = iter.next();
-            for (final Field f : c.getDeclaredFields()) {
-                try {
-                    f.setAccessible(true);
-                    final Object child = f.get(node);
-                    if (child == null) {
-                        continue;
-                    }
-
-                    if (child instanceof Node) {
-                        children.add(f);
-                    } else if (child instanceof Collection) {
-                        if (!((Collection<?>)child).isEmpty()) {
-                            children.add(f);
-                        }
-                    }
-                } catch (final IllegalArgumentException | IllegalAccessException e) {
-                    return;
-                }
-            }
         }
 
         String status = "";
@@ -193,52 +152,98 @@ public final class ASTWriter {
                 append("]").
                 append('\n');
 
-            for (final Field field : children) {
+            for (final Field child : children) {
-
+                if (child.getAnnotation(ParentNode.class) != null) {
-                if (field.getAnnotation(ParentNode.class) != null) {
                     continue;
-                }
+                } else if (child.getAnnotation(Ignore.class) != null) {
-                if (field.getAnnotation(Ignore.class) != null) {
-                    continue;
-                }
-                if (field.getAnnotation(Reference.class) != null) {
                     continue;
                 }
 
                 Object value;
                 try {
-                    value = field.get(node);
+                    value = child.get(node);
                 } catch (final IllegalArgumentException | IllegalAccessException e) {
                     Context.printStackTrace(e);
                     return;
                 }
 
                 if (value instanceof Node) {
-                    printAST(sb, field.getName(), (Node)value, indent + 1);
+                    printAST(sb, child, child.getName(), (Node)value, indent + 1);
                 } else if (value instanceof Collection) {
                     int pos = 0;
                     ASTWriter.indent(sb, indent + 1);
                     sb.append("[Collection ").
-                        append(field.getName()).
+                        append(child.getName()).
                         append("[0..").
                         append(((Collection<Node>)value).size()).
                         append("]]").
                         append('\n');
 
-                    for (final Node child : (Collection<Node>)value) {
+                    for (final Node member : (Collection<Node>)value) {
-                        printAST(sb, field.getName() + "[" + pos++ + "]", child, indent + 2);
+                        printAST(sb, child, child.getName() + "[" + pos++ + "]", member, indent + 2);
                     }
                 }
             }
         }
     }
 
+    private static void enqueueChildren(final Node node, final Class<?> nodeClass, final List<Field> children) {
+        final Deque<Class<?>> stack = new ArrayDeque<>();
+
+        /**
+         * Here is some ugliness that can be overcome by proper ChildNode annotations
+         * with proper orders. Right now we basically sort all classes up to Node
+         * with super class first, as this often is the natural order, e.g. base
+         * before index for an IndexNode.
+         *
+         * Also there are special cases as this is not true for UnaryNodes(lhs) and
+         * BinaryNodes extends UnaryNode (with lhs), and TernaryNodes.
+         *
+         * TODO - generalize traversal with an order built on annotations and this
+         * will go away.
+         */
+        Class<?> clazz = nodeClass;
+        do {
+            stack.push(clazz);
+            clazz = clazz.getSuperclass();
+        } while (clazz != null);
+
+        if (node instanceof TernaryNode) {
+            // HACK juggle "third"
+            stack.push(stack.removeLast());
+        }
+        // HACK change operator order for BinaryNodes to get lhs first.
+        final Iterator<Class<?>> iter = node instanceof BinaryNode ? stack.descendingIterator() : stack.iterator();
+
+        while (iter.hasNext()) {
+            final Class<?> c = iter.next();
+            for (final Field f : c.getDeclaredFields()) {
+                try {
+                    f.setAccessible(true);
+                    final Object child = f.get(node);
+                    if (child == null) {
+                        continue;
+                    }
+
+                    if (child instanceof Node) {
+                        children.add(f);
+                    } else if (child instanceof Collection) {
+                        if (!((Collection<?>)child).isEmpty()) {
+                            children.add(f);
+                        }
+                    }
+                } catch (final IllegalArgumentException | IllegalAccessException e) {
+                    return;
+                }
+            }
+        }
+    }
+
     private static void indent(final StringBuilder sb, final int indent) {
         for (int i = 0; i < indent; i++) {
-            for (int j = 0 ; j < TABWIDTH ; j++) {
+            for (int j = 0; j < TABWIDTH; j++) {
                 sb.append(' ');
             }
         }
     }
-
 }