changeset 19141:67d9e635102f

Truffle/Instrumentation: refine checks for safe node replacement
author Michael Van De Vanter <michael.van.de.vanter@oracle.com>
date Tue, 03 Feb 2015 11:48:25 -0800
parents ccabd82be35c
children 64e6c7b83515
files graal/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/nodes/SafeReplaceTest.java graal/com.oracle.truffle.api/src/com/oracle/truffle/api/nodes/Node.java graal/com.oracle.truffle.api/src/com/oracle/truffle/api/nodes/NodeUtil.java
diffstat 3 files changed, 24 insertions(+), 42 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/nodes/SafeReplaceTest.java	Tue Feb 03 18:30:07 2015 +0100
+++ b/graal/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/nodes/SafeReplaceTest.java	Tue Feb 03 11:48:25 2015 -0800
@@ -39,12 +39,11 @@
     public void testCorrectReplacement() {
         TestRootNode root = new TestRootNode();
         final TestNode oldChild = new TestNode();
+        final TestNode newChild = new TestNode();
         root.child = oldChild;
-        assertFalse(oldChild.isReplaceable());  // No parent node
+        assertFalse(oldChild.isSafelyReplaceableBy(newChild));  // No parent node
         root.adoptChildren();
-        assertTrue(oldChild.isReplaceable());   // Now adopted by parent
-        final TestNode newChild = new TestNode();
-        assertTrue(oldChild.isSafelyReplaceableBy(newChild));  // Parent field type is assignable by
+        assertTrue(oldChild.isSafelyReplaceableBy(newChild));   // Now adopted by parent
         // new node
         oldChild.replace(newChild);
         root.execute(null);
@@ -59,10 +58,11 @@
         final TestNode oldChild = new TestNode();
         root.child = oldChild;
         root.adoptChildren();
-        final WrongTestNode newChild = new WrongTestNode();
-        assertFalse(oldChild.isSafelyReplaceableBy(newChild));
-        // Can't test: oldChild.replace(newChild);
-        // Fails if assertions checked; else unsafe assignment will eventually crash the VM
+        final TestNode newChild = new TestNode();
+        final TestNode strayChild = new TestNode();
+        assertFalse(strayChild.isSafelyReplaceableBy(newChild)); // Stray not a child of parent
+        final WrongTestNode wrongTypeNewChild = new WrongTestNode();
+        assertFalse(oldChild.isSafelyReplaceableBy(wrongTypeNewChild));
     }
 
     private static class TestNode extends Node {
--- a/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/nodes/Node.java	Tue Feb 03 18:30:07 2015 +0100
+++ b/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/nodes/Node.java	Tue Feb 03 11:48:25 2015 -0800
@@ -290,26 +290,10 @@
     }
 
     /**
-     * Checks if this node is properly adopted by its parent.
-     *
-     * @return {@code true} if it is structurally safe to replace this node.
-     */
-    public final boolean isReplaceable() {
-        if (getParent() != null) {
-            for (Node sibling : getParent().getChildren()) {
-                if (sibling == this) {
-                    return true;
-                }
-            }
-        }
-        return false;
-    }
-
-    /**
-     * Checks if this node can be replaced by another node, both structurally and with type safety.
+     * Checks if this node can be replaced by another node: tree structure & type.
      */
     public final boolean isSafelyReplaceableBy(Node newNode) {
-        return isReplaceable() && NodeUtil.isReplacementSafe(getParent(), this, newNode);
+        return NodeUtil.isReplacementSafe(getParent(), this, newNode);
     }
 
     private void reportReplace(Node oldNode, Node newNode, CharSequence reason) {
--- a/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/nodes/NodeUtil.java	Tue Feb 03 18:30:07 2015 +0100
+++ b/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/nodes/NodeUtil.java	Tue Feb 03 11:48:25 2015 -0800
@@ -676,26 +676,24 @@
     }
 
     /**
-     * Determines whether a proposed child replacement would be type safe.
-     *
-     * @param parent non-null node
-     * @param oldChild non-null existing child of parent
-     * @param newChild non-null proposed replacement for existing child
+     * Determines whether a proposed child replacement would be safe: structurally and type.
      */
     public static boolean isReplacementSafe(Node parent, Node oldChild, Node newChild) {
         assert newChild != null;
-        final NodeField field = findChildField(parent, oldChild);
-        if (field == null) {
-            throw new IllegalArgumentException();
+        if (parent != null) {
+            final NodeField field = findChildField(parent, oldChild);
+            if (field != null) {
+                switch (field.getKind()) {
+                    case CHILD:
+                        return field.getType().isAssignableFrom(newChild.getClass());
+                    case CHILDREN:
+                        return field.getType().getComponentType().isAssignableFrom(newChild.getClass());
+                    default:
+                        throw new IllegalStateException();
+                }
+            }
         }
-        switch (field.getKind()) {
-            case CHILD:
-                return field.getType().isAssignableFrom(newChild.getClass());
-            case CHILDREN:
-                return field.getType().getComponentType().isAssignableFrom(newChild.getClass());
-            default:
-                throw new IllegalArgumentException();
-        }
+        return false;
     }
 
     /** Returns all declared fields in the class hierarchy. */