# HG changeset patch # User Michael Van De Vanter # Date 1422992905 28800 # Node ID 67d9e635102fe17ed1c7571b27059b736817c010 # Parent ccabd82be35cc79a6dcb2f752127527227c4a1d5 Truffle/Instrumentation: refine checks for safe node replacement diff -r ccabd82be35c -r 67d9e635102f graal/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/nodes/SafeReplaceTest.java --- 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 { diff -r ccabd82be35c -r 67d9e635102f graal/com.oracle.truffle.api/src/com/oracle/truffle/api/nodes/Node.java --- 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) { diff -r ccabd82be35c -r 67d9e635102f graal/com.oracle.truffle.api/src/com/oracle/truffle/api/nodes/NodeUtil.java --- 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. */