Mercurial > hg > truffle
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. */