# HG changeset patch # User Doug Simon # Date 1417424433 -3600 # Node ID d24328ea36a7ee0a96634f3f1a1eeb6460a536c7 # Parent b76489300efa2287ff1521c1aec3c44e94bcb9ac fixed bug in VerifyUsageWithEquals and improved javadoc diff -r b76489300efa -r d24328ea36a7 graal/com.oracle.graal.phases/src/com/oracle/graal/phases/verify/VerifyUsageWithEquals.java --- a/graal/com.oracle.graal.phases/src/com/oracle/graal/phases/verify/VerifyUsageWithEquals.java Sun Nov 30 21:14:13 2014 +0100 +++ b/graal/com.oracle.graal.phases/src/com/oracle/graal/phases/verify/VerifyUsageWithEquals.java Mon Dec 01 10:00:33 2014 +0100 @@ -31,24 +31,30 @@ import com.oracle.graal.phases.tiers.*; /** - * For certain types object identity should not be used for object equality check. This phase checks - * the correct usage of the given type. Equality checks with == or != (except null checks) results - * in an {@link AssertionError}. + * For certain types, object identity should not be used for object equality check. This phase + * checks the correct usage of the given type. Equality checks with == or != (except null checks) + * results in an {@link AssertionError}. */ public class VerifyUsageWithEquals extends VerifyPhase { - private final Class klass; + /** + * The type of values that must not use identity for testing object equality. + */ + private final Class restrictedClass; public VerifyUsageWithEquals(Class klass) { - this.klass = klass; + this.restrictedClass = klass; } - private boolean isAssignableType(ValueNode node, MetaAccessProvider metaAccess) { + /** + * Determines whether the type of {@code node} is assignable to the {@link #restrictedClass}. + */ + private boolean isAssignableToRestrictedType(ValueNode node, MetaAccessProvider metaAccess) { if (node.stamp() instanceof ObjectStamp) { - ResolvedJavaType valueType = metaAccess.lookupJavaType(klass); + ResolvedJavaType restrictedType = metaAccess.lookupJavaType(restrictedClass); ResolvedJavaType nodeType = StampTool.typeOrNull(node); - if (nodeType != null && valueType.isAssignableFrom(nodeType)) { + if (nodeType != null && restrictedType.isAssignableFrom(nodeType)) { return true; } } @@ -59,16 +65,20 @@ return node.isConstant() && node.isNullConstant(); } - private boolean checkUsage(ValueNode x, ValueNode y, MetaAccessProvider metaAccess) { - return isAssignableType(x, metaAccess) && !isNullConstant(y); + /** + * Checks whether the type of {@code x} is assignable to the restricted type and that {@code y} + * is not a null constant. + */ + private boolean isIllegalUsage(ValueNode x, ValueNode y, MetaAccessProvider metaAccess) { + return isAssignableToRestrictedType(x, metaAccess) && !isNullConstant(y); } @Override protected boolean verify(StructuredGraph graph, PhaseContext context) { for (ObjectEqualsNode cn : graph.getNodes().filter(ObjectEqualsNode.class)) { // bail out if we compare an object of type klass with == or != (except null checks) - if (checkUsage(cn.getX(), cn.getY(), context.getMetaAccess()) && checkUsage(cn.getY(), cn.getX(), context.getMetaAccess())) { - throw new VerificationError("Verification of " + klass.getName() + " usage failed: Comparing " + cn.getX() + " and " + cn.getY() + " in " + graph.method() + + if (isIllegalUsage(cn.getX(), cn.getY(), context.getMetaAccess()) || isIllegalUsage(cn.getY(), cn.getX(), context.getMetaAccess())) { + throw new VerificationError("Verification of " + restrictedClass.getName() + " usage failed: Comparing " + cn.getX() + " and " + cn.getY() + " in " + graph.method() + " must use .equals() for object equality, not '==' or '!='"); } }