changeset 18581:d24328ea36a7

fixed bug in VerifyUsageWithEquals and improved javadoc
author Doug Simon <doug.simon@oracle.com>
date Mon, 01 Dec 2014 10:00:33 +0100
parents b76489300efa
children 1b918c6ff5eb
files graal/com.oracle.graal.phases/src/com/oracle/graal/phases/verify/VerifyUsageWithEquals.java
diffstat 1 files changed, 22 insertions(+), 12 deletions(-) [+]
line wrap: on
line diff
--- 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<PhaseContext> {
 
-    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 '!='");
             }
         }