[Truffle] Fix conversions to boolean.

* The Ruby rules are fairly simple so jsut use a simple method. A node could potentially avoid the explicit instanceof check though. * Remove extra #! methods on true and false as well as unused yieldBoolean().
jruby · Nov 3, 2014 · 2bdcc68 · 2bdcc68
1 parent 1495127
commit 2bdcc68
Showing 16 changed files with 57 additions and 125 deletions.
diff --git a/core/src/main/java/org/jruby/truffle/nodes/control/WhenSplatNode.java b/core/src/main/java/org/jruby/truffle/nodes/control/WhenSplatNode.java
@@ -45,9 +45,7 @@ public boolean executeBoolean(VirtualFrame frame) {
         }
 
         for (Object value : array.slowToArray()) {
-            // TODO(CS): how to cast this to a boolean?
-
-            if ((boolean) dispatchCaseEqual.call(frame, caseExpression, "===", null, value)) {
+            if (dispatchCaseEqual.callIsTruthy(frame, caseExpression, "===", null, value)) {
                 return true;
             }
         }

diff --git a/core/src/main/java/org/jruby/truffle/nodes/core/ArrayNodes.java b/core/src/main/java/org/jruby/truffle/nodes/core/ArrayNodes.java
@@ -499,7 +499,7 @@ public boolean equal(VirtualFrame frame, RubyArray a, RubyArray b) {
             final Object[] bs = b.slowToArray();
 
             for (int n = 0; n < a.getSize(); n++) {
-                if (!(boolean)equals.call(frame, as[n], "==", null, bs[n])) {
+                if (!equals.callIsTruthy(frame, as[n], "==", null, bs[n])) {
                     return false;
                 }
             }
@@ -911,7 +911,7 @@ public boolean allIntegerFixnum(VirtualFrame frame, RubyArray array, RubyProc bl
             notDesignedForCompilation();
 
             for (int n = 0; n < array.getSize(); n++) {
-                if (!yieldBoolean(frame, block, ((int[]) array.getStore())[n])) {
+                if (!yieldIsTruthy(frame, block, ((int[]) array.getStore())[n])) {
                     return false;
                 }
             }
@@ -924,7 +924,7 @@ public boolean allLongFixnum(VirtualFrame frame, RubyArray array, RubyProc block
             notDesignedForCompilation();
 
             for (int n = 0; n < array.getSize(); n++) {
-                if (!yieldBoolean(frame, block, ((long[]) array.getStore())[n])) {
+                if (!yieldIsTruthy(frame, block, ((long[]) array.getStore())[n])) {
                     return false;
                 }
             }
@@ -937,7 +937,7 @@ public boolean allFloat(VirtualFrame frame, RubyArray array, RubyProc block) {
             notDesignedForCompilation();
 
             for (int n = 0; n < array.getSize(); n++) {
-                if (!yieldBoolean(frame, block, ((double[]) array.getStore())[n])) {
+                if (!yieldIsTruthy(frame, block, ((double[]) array.getStore())[n])) {
                     return false;
                 }
             }
@@ -950,7 +950,7 @@ public boolean allObject(VirtualFrame frame, RubyArray array, RubyProc block) {
             notDesignedForCompilation();
 
             for (int n = 0; n < array.getSize(); n++) {
-                if (!yieldBoolean(frame, block, ((Object[]) array.getStore())[n])) {
+                if (!yieldIsTruthy(frame, block, ((Object[]) array.getStore())[n])) {
                     return false;
                 }
             }
@@ -981,7 +981,7 @@ public boolean allIntegerFixnum(VirtualFrame frame, RubyArray array, RubyProc bl
             notDesignedForCompilation();
 
             for (int n = 0; n < array.getSize(); n++) {
-                if (yieldBoolean(frame, block, ((int[]) array.getStore())[n])) {
+                if (yieldIsTruthy(frame, block, ((int[]) array.getStore())[n])) {
                     return true;
                 }
             }
@@ -994,7 +994,7 @@ public boolean anyLongFixnum(VirtualFrame frame, RubyArray array, RubyProc block
             notDesignedForCompilation();
 
             for (int n = 0; n < array.getSize(); n++) {
-                if (yieldBoolean(frame, block, ((long[]) array.getStore())[n])) {
+                if (yieldIsTruthy(frame, block, ((long[]) array.getStore())[n])) {
                     return true;
                 }
             }
@@ -1007,7 +1007,7 @@ public boolean anyFloat(VirtualFrame frame, RubyArray array, RubyProc block) {
             notDesignedForCompilation();
 
             for (int n = 0; n < array.getSize(); n++) {
-                if (yieldBoolean(frame, block, ((double[]) array.getStore())[n])) {
+                if (yieldIsTruthy(frame, block, ((double[]) array.getStore())[n])) {
                     return true;
                 }
             }
@@ -1020,7 +1020,7 @@ public boolean anyObject(VirtualFrame frame, RubyArray array, RubyProc block) {
             notDesignedForCompilation();
 
             for (int n = 0; n < array.getSize(); n++) {
-                if (yieldBoolean(frame, block, ((Object[]) array.getStore())[n])) {
+                if (yieldIsTruthy(frame, block, ((Object[]) array.getStore())[n])) {
                     return true;
                 }
             }
@@ -1599,7 +1599,7 @@ public Object findIntegerFixnum(VirtualFrame frame, RubyArray array, RubyProc bl
                 try {
                     final Object value = store[n];
 
-                    if (yieldBoolean(frame, block, value)) {
+                    if (yieldIsTruthy(frame, block, value)) {
                         return value;
                     }
                 } catch (BreakException e) {
@@ -1620,7 +1620,7 @@ public Object findLongFixnum(VirtualFrame frame, RubyArray array, RubyProc block
                 try {
                     final Object value = store[n];
 
-                    if (yieldBoolean(frame, block, value)) {
+                    if (yieldIsTruthy(frame, block, value)) {
                         return value;
                     }
                 } catch (BreakException e) {
@@ -1641,7 +1641,7 @@ public Object findFloat(VirtualFrame frame, RubyArray array, RubyProc block) {
                 try {
                     final Object value = store[n];
 
-                    if (yieldBoolean(frame, block, value)) {
+                    if (yieldIsTruthy(frame, block, value)) {
                         return value;
                     }
                 } catch (BreakException e) {
@@ -1662,7 +1662,7 @@ public Object findObject(VirtualFrame frame, RubyArray array, RubyProc block) {
                 try {
                     final Object value = store[n];
 
-                    if (yieldBoolean(frame, block, value)) {
+                    if (yieldIsTruthy(frame, block, value)) {
                         return value;
                     }
                 } catch (BreakException e) {
@@ -1761,7 +1761,6 @@ public boolean includeFixnum(VirtualFrame frame, RubyArray array, Object value)
             for (int n = 0; n < array.getSize(); n++) {
                 final Object stored = store[n];
 
-                // TODO(CS): cast node around the dispatch
                 notDesignedForCompilation();
 
                 if (equalNode.executeSameOrEqual(frame, stored, value)) {
@@ -2817,7 +2816,7 @@ public Object rejectInPlaceObject(VirtualFrame frame, RubyArray array, RubyProc
             int i = 0;
 
             for (int n = 0; n < array.getSize(); n++) {
-                if (yieldBoolean(frame, block, store[n])) {
+                if (yieldIsTruthy(frame, block, store[n])) {
                     continue;
                 }
 
@@ -2924,10 +2923,9 @@ public Object selectObject(VirtualFrame frame, RubyArray array, RubyProc block)
 
                     final Object value = store[n];
 
-                    // TODO(CS): cast to boolean?
                     notDesignedForCompilation();
 
-                    if (yieldBoolean(frame, block, new Object[]{value})) {
+                    if (yieldIsTruthy(frame, block,  new Object[]{value})) {
                         selectedStore = arrayBuilder.append(selectedStore, selectedSize, value);
                         selectedSize++;
                     }
@@ -2958,10 +2956,9 @@ public Object selectFixnumInteger(VirtualFrame frame, RubyArray array, RubyProc
 
                     final Object value = store[n];
 
-                    // TODO(CS): cast to boolean?
                     notDesignedForCompilation();
 
-                    if ((boolean) yield(frame, block, value)) {
+                    if (yieldIsTruthy(frame, block, value)) {
                         selectedStore = arrayBuilder.append(selectedStore, selectedSize, value);
                         selectedSize++;
                     }

diff --git a/core/src/main/java/org/jruby/truffle/nodes/core/BasicObjectNodes.java b/core/src/main/java/org/jruby/truffle/nodes/core/BasicObjectNodes.java
@@ -27,7 +27,7 @@
 @CoreClass(name = "BasicObject")
 public abstract class BasicObjectNodes {
 
-    @CoreMethod(names = "!", needsSelf = false)
+    @CoreMethod(names = "!")
     public abstract static class NotNode extends CoreMethodNode {
 
         public NotNode(RubyContext context, SourceSection sourceSection) {
@@ -39,8 +39,8 @@ public NotNode(NotNode prev) {
         }
 
         @Specialization
-        public boolean not() {
-            return false;
+        public boolean not(Object value) {
+            return !getContext().getCoreLibrary().isTruthy(value);
         }
 
     }
@@ -62,8 +62,7 @@ public NotEqualNode(NotEqualNode prev) {
 
         @Specialization
         public boolean equal(VirtualFrame frame, Object a, Object b) {
-            // TODO(CS): cast
-            return !(boolean) equalNode.call(frame, a, "==", null, b);
+            return !equalNode.callIsTruthy(frame, a, "==", null, b);
         }
 
     }

diff --git a/core/src/main/java/org/jruby/truffle/nodes/core/FalseClassNodes.java b/core/src/main/java/org/jruby/truffle/nodes/core/FalseClassNodes.java
@@ -17,24 +17,6 @@
 @CoreClass(name = "FalseClass")
 public abstract class FalseClassNodes {
 
-    @CoreMethod(names = "!", needsSelf = false)
-    public abstract static class NotNode extends CoreMethodNode {
-
-        public NotNode(RubyContext context, SourceSection sourceSection) {
-            super(context, sourceSection);
-        }
-
-        public NotNode(NotNode prev) {
-            super(prev);
-        }
-
-        @Specialization
-        public boolean not() {
-            return true;
-        }
-
-    }
-
     @CoreMethod(names = {"==", "=~"}, needsSelf = false, required = 1)
     public abstract static class EqualNode extends CoreMethodNode {
 

diff --git a/core/src/main/java/org/jruby/truffle/nodes/core/HashNodes.java b/core/src/main/java/org/jruby/truffle/nodes/core/HashNodes.java
@@ -67,8 +67,7 @@ public boolean equalObjectArray(VirtualFrame frame, RubyHash a, RubyHash b) {
             }
 
             for (int n = 0; n < aSize * 2; n++) {
-                // TODO(CS): cast
-                if (!(boolean) equalNode.call(frame, aStore[n], "==", null, bStore[n])) {
+                if (!equalNode.callIsTruthy(frame, aStore[n], "==", null, bStore[n])) {
                     return false;
                 }
             }
@@ -231,8 +230,7 @@ public Object getObjectArray(VirtualFrame frame, RubyHash hash, Object key) {
             final int size = hash.getStoreSize();
 
             for (int n = 0; n < RubyHash.HASHES_SMALL; n++) {
-                // TODO(CS): cast
-                if (n < size && (boolean) eqlNode.call(frame, store[n * 2], "eql?", null, key)) {
+                if (n < size && eqlNode.callIsTruthy(frame, store[n * 2], "eql?", null, key)) {
                     return store[n * 2 + 1];
                 }
             }
@@ -309,8 +307,7 @@ public Object setObjectArray(VirtualFrame frame, RubyHash hash, Object key, Obje
             final int size = hash.getStoreSize();
 
             for (int n = 0; n < RubyHash.HASHES_SMALL; n++) {
-                // TODO(CS): cast
-                if (n < size && (boolean) eqlNode.call(frame, store[n * 2], "eql?", null, key)) {
+                if (n < size && eqlNode.callIsTruthy(frame, store[n * 2], "eql?", null, key)) {
                     store[n * 2 + 1] = value;
                     return value;
                 }
@@ -837,8 +834,7 @@ public RubyHash mergeObjectArrayObjectArray(VirtualFrame frame, RubyHash hash, R
 
                     for (int b = 0; b < RubyHash.HASHES_SMALL; b++) {
                         if (b < storeBSize) {
-                            // TODO(CS): cast
-                            if ((boolean) eqlNode.call(frame, storeA[a * 2], "eql?", null, storeB[b * 2])) {
+                            if (eqlNode.callIsTruthy(frame, storeA[a * 2], "eql?", null, storeB[b * 2])) {
                                 merge = false;
                                 break;
                             }
@@ -926,8 +922,7 @@ public boolean keyObjectArray(VirtualFrame frame, RubyHash hash, Object key) {
             final Object[] store = (Object[]) hash.getStore();
 
             for (int n = 0; n < store.length; n += 2) {
-                // TODO(CS): cast
-                if ((boolean) eqlNode.call(frame, store[n], "eql?", null, key)) {
+                if (eqlNode.callIsTruthy(frame, store[n], "eql?", null, key)) {
                     return true;
                 }
             }

diff --git a/core/src/main/java/org/jruby/truffle/nodes/core/KernelNodes.java b/core/src/main/java/org/jruby/truffle/nodes/core/KernelNodes.java
@@ -67,8 +67,7 @@ public SameOrEqualNode(SameOrEqualNode prev) {
         public boolean sameOrEqual(VirtualFrame frame, Object a, Object b) {
             if (referenceEqualNode.executeEqual(frame, a, b))
                 return true;
-            // TODO(CS): cast
-            return (boolean) equalNode.call(frame, a, "==", null, b);
+            return equalNode.callIsTruthy(frame, a, "==", null, b);
         }
 
     }

diff --git a/core/src/main/java/org/jruby/truffle/nodes/core/RangeNodes.java b/core/src/main/java/org/jruby/truffle/nodes/core/RangeNodes.java
@@ -234,16 +234,16 @@ public boolean include(RubyRange.IntegerFixnumRange range, int value) {
         public boolean include(VirtualFrame frame, RubyRange.ObjectRange range, Object value) {
             notDesignedForCompilation();
 
-            if ((boolean) callLess.call(frame, value, "<", null, range.getBegin())) {
+            if (callLess.callIsTruthy(frame, value, "<", null, range.getBegin())) {
                 return false;
             }
 
             if (range.doesExcludeEnd()) {
-                if ((boolean) callGreaterEqual.call(frame, value, ">=", null, range.getEnd())) {
+                if (callGreaterEqual.callIsTruthy(frame, value, ">=", null, range.getEnd())) {
                     return false;
                 }
             } else {
-                if ((boolean) callGreater.call(frame, value, ">", null, range.getEnd())) {
+                if (callGreater.callIsTruthy(frame, value, ">", null, range.getEnd())) {
                     return false;
                 }
             }

diff --git a/core/src/main/java/org/jruby/truffle/nodes/core/TrueClassNodes.java b/core/src/main/java/org/jruby/truffle/nodes/core/TrueClassNodes.java
@@ -17,24 +17,6 @@
 @CoreClass(name = "TrueClass")
 public abstract class TrueClassNodes {
 
-    @CoreMethod(names = "!", needsSelf = false)
-    public abstract static class NotNode extends CoreMethodNode {
-
-        public NotNode(RubyContext context, SourceSection sourceSection) {
-            super(context, sourceSection);
-        }
-
-        public NotNode(NotNode prev) {
-            super(prev);
-        }
-
-        @Specialization
-        public boolean not() {
-            return false;
-        }
-
-    }
-
     @CoreMethod(names = {"==", "=~"}, needsSelf = false, required = 1)
     public abstract static class EqualNode extends CoreMethodNode {
 

diff --git a/core/src/main/java/org/jruby/truffle/nodes/core/YieldingArrayCoreMethodNode.java b/core/src/main/java/org/jruby/truffle/nodes/core/YieldingArrayCoreMethodNode.java
@@ -36,10 +36,8 @@ public Object yield(VirtualFrame frame, RubyProc block, Object... arguments) {
         return dispatchNode.dispatch(frame, block, arguments);
     }
 
-    public boolean yieldBoolean(VirtualFrame frame, RubyProc block, Object... arguments) {
-        // TODO(CS): this should be a node!
-        RubyNode.notDesignedForCompilation();
-        return RubyTrueClass.toBoolean(yield(frame, block, arguments));
+    public boolean yieldIsTruthy(VirtualFrame frame, RubyProc block, Object... arguments) {
+        return getContext().getCoreLibrary().isTruthy(yield(frame, block, arguments));
     }
 
 }
diff --git a/core/src/main/java/org/jruby/truffle/nodes/core/YieldingCoreMethodNode.java b/core/src/main/java/org/jruby/truffle/nodes/core/YieldingCoreMethodNode.java
@@ -34,9 +34,4 @@ public Object yield(VirtualFrame frame, RubyProc block, Object... arguments) {
         return dispatchNode.dispatch(frame, block, arguments);
     }
 
-    public boolean yieldBoolean(VirtualFrame frame, RubyProc block, Object... arguments) {
-        // TODO(CS): this should be a node!
-        return RubyTrueClass.toBoolean(dispatchNode.dispatch(frame, block, arguments));
-    }
-
 }
diff --git a/core/src/main/java/org/jruby/truffle/nodes/core/YieldingHashCoreMethodNode.java b/core/src/main/java/org/jruby/truffle/nodes/core/YieldingHashCoreMethodNode.java
@@ -36,10 +36,4 @@ public Object yield(VirtualFrame frame, RubyProc block, Object... arguments) {
         return dispatchNode.dispatch(frame, block, arguments);
     }
 
-    public boolean yieldBoolean(VirtualFrame frame, RubyProc block, Object... arguments) {
-        // TODO(CS): this should be a node!
-        RubyNode.notDesignedForCompilation();
-        return RubyTrueClass.toBoolean(yield(frame, block, arguments));
-    }
-
 }
diff --git a/core/src/main/java/org/jruby/truffle/nodes/dispatch/DispatchHeadNode.java b/core/src/main/java/org/jruby/truffle/nodes/dispatch/DispatchHeadNode.java
@@ -68,6 +68,15 @@ public Object call(
                 Dispatch.DispatchAction.CALL_METHOD);
     }
 
+    public boolean callIsTruthy(
+            VirtualFrame frame,
+            Object receiverObject,
+            Object methodName,
+            RubyProc blockObject,
+            Object... argumentsObjects) {
+        return context.getCoreLibrary().isTruthy(call(frame, receiverObject, methodName, blockObject, argumentsObjects));
+    }
+
     public double callFloat(
             VirtualFrame frame,
             Object receiverObject,
@@ -142,15 +151,15 @@ public boolean doesRespondTo(
             VirtualFrame frame,
             Object methodName,
             Object receiverObject) {
-        return (boolean) dispatch(
+        return context.getCoreLibrary().isTruthy(dispatch(
                 frame,
                 context.getCoreLibrary().getNilObject(),
                 null, // TODO(eregon): was RubyArguments.getSelf(frame.getArguments()),
                 receiverObject,
                 methodName,
                 null,
                 null,
-                Dispatch.DispatchAction.RESPOND_TO_METHOD);
+                Dispatch.DispatchAction.RESPOND_TO_METHOD));
     }
 
     public Object dispatch(

diff --git a/core/src/main/java/org/jruby/truffle/nodes/literal/HashLiteralNode.java b/core/src/main/java/org/jruby/truffle/nodes/literal/HashLiteralNode.java
@@ -99,7 +99,7 @@ public RubyHash executeRubyHash(VirtualFrame frame) {
                 final Object value = keyValues[n + 1].execute(frame);
 
                 for (int i = 0; i < n; i += 2) {
-                    if ((boolean) equalNode.call(frame, key, "==", null, storage[i])) {
+                    if (equalNode.callIsTruthy(frame, key, "==", null, storage[i])) {
                         storage[i + 1] = value;
                         continue initializers;
                     }

diff --git a/core/src/main/java/org/jruby/truffle/nodes/methods/ExceptionTranslatingNode.java b/core/src/main/java/org/jruby/truffle/nodes/methods/ExceptionTranslatingNode.java
@@ -63,15 +63,15 @@ public Object execute(VirtualFrame frame) {
     }
 
     private RubyBasicObject translate(ArithmeticException exception) {
-        if ((boolean) Options.TRUFFLE_EXCEPTIONS_PRINT_JAVA.load()) {
+        if (Options.TRUFFLE_EXCEPTIONS_PRINT_JAVA.load()) {
             exception.printStackTrace();
         }
 
         throw new RaiseException(getContext().getCoreLibrary().zeroDivisionError(this));
     }
 
     private RubyBasicObject translate(UnsupportedSpecializationException exception) {
-        if ((boolean) Options.TRUFFLE_EXCEPTIONS_PRINT_JAVA.load()) {
+        if (Options.TRUFFLE_EXCEPTIONS_PRINT_JAVA.load()) {
             exception.printStackTrace();
         }
 

diff --git a/core/src/main/java/org/jruby/truffle/runtime/core/CoreLibrary.java b/core/src/main/java/org/jruby/truffle/runtime/core/CoreLibrary.java
@@ -425,6 +425,17 @@ public RubyBasicObject box(Object object) {
         throw new UnsupportedOperationException("Don't know how to box " + object.getClass().getName());
     }
 
+    /**
+     * Convert a value to a boolean according to Ruby rules. Never fails.
+     */
+    public boolean isTruthy(Object value) {
+        if (value instanceof Boolean) {
+            return (boolean) value;
+        } else {
+            return value != nilObject && value != falseObject;
+        }
+    }
+
     public RubyException runtimeError(String message, Node currentNode) {
         CompilerAsserts.neverPartOfCompilation();
         return new RubyException(runtimeErrorClass, context.makeString(String.format("%s", message)), RubyCallStack.getBacktrace(currentNode));

diff --git a/core/src/main/java/org/jruby/truffle/runtime/core/RubyTrueClass.java b/core/src/main/java/org/jruby/truffle/runtime/core/RubyTrueClass.java
@@ -21,33 +21,6 @@ public RubyTrueClass(RubyClass objectClass) {
         super(objectClass);
     }
 
-    /**
-     * Convert a value to a boolean, without doing any lookup.
-     */
-    public static boolean toBoolean(Object value) {
-        RubyNode.notDesignedForCompilation();
-
-        assert value != null;
-
-        if (value instanceof RubyNilClass) {
-            return false;
-        }
-
-        if (value instanceof Boolean) {
-            return (boolean) value;
-        }
-
-        if (value instanceof RubyTrueClass) {
-            return true;
-        }
-
-        if (value instanceof RubyFalseClass) {
-            return false;
-        }
-
-        return true;
-    }
-
     public Object unbox() {
         return true;
     }