Normalize the values that R may return via interop

Author: steve-s

CHANGELOG.md

@@ -1,4 +1,17 @@
 
+# 1.0 RC 5
+
+Updates in interop:
+
+* R code evaluated via interop never returns a Java primitive type, but always a vector
+* Vectors of size 1 that do not contain NA can be unboxed
+* Sending the READ message to an atomic R vector (array subscript in most languages) gives
+  * Java primitive type as long as the value is not `NA`
+  * a special value that responds to `IS_NULL` with `true`. If this value is passed back to R it behaves as `NA` again
+* Note that sending the READ message to a list, environment, or other heterogenous data structure never gives atomic Java type but a primitive R vector
+
+# 1.0 RC 4
+
 # 1.0 RC 3
 
 Added missing R builtins and C API

com.oracle.truffle.r.engine/src/com/oracle/truffle/r/engine/interop/RAbstractVectorAccessFactory.java

@@ -51,13 +51,14 @@
 import com.oracle.truffle.r.nodes.access.vector.ExtractVectorNode;
 import com.oracle.truffle.r.nodes.access.vector.ReplaceVectorNode;
 import com.oracle.truffle.r.nodes.access.vector.ReplaceVectorNodeGen;
+import com.oracle.truffle.r.nodes.builtin.base.IsNA;
+import com.oracle.truffle.r.nodes.builtin.base.IsNANodeGen;
 import com.oracle.truffle.r.nodes.control.RLengthNode;
 import com.oracle.truffle.r.runtime.RRuntime;
 import com.oracle.truffle.r.runtime.data.NativeDataAccess;
 import com.oracle.truffle.r.runtime.data.RLogical;
 import com.oracle.truffle.r.runtime.data.RRaw;
 import com.oracle.truffle.r.runtime.data.RScalar;
-import com.oracle.truffle.r.runtime.data.RString;
 import com.oracle.truffle.r.runtime.data.model.RAbstractAtomicVector;
 import com.oracle.truffle.r.runtime.data.model.RAbstractLogicalVector;
 import com.oracle.truffle.r.runtime.data.model.RAbstractRawVector;
@@ -79,6 +80,9 @@ public final SourceSection getSourceSection() {
     }
 }
 
+/**
+ * Implements interop messages for all {@link RAbstractAtomicVector} subclasses.
+ */
 public final class RAbstractVectorAccessFactory implements StandardFactory {
 
     abstract static class VectorReadImplNode extends InteropRootNode {
@@ -127,7 +131,7 @@ private Object read(Object receiver, Object[] positions) {
             Object value = extract.apply(receiver, positions, RLogical.TRUE, RLogical.TRUE);
             if (r2Foreign == null) {
                 CompilerDirectives.transferToInterpreterAndInvalidate();
-                r2Foreign = insert(R2ForeignNodeGen.create());
+                r2Foreign = insert(R2Foreign.createNoBox());
             }
             return r2Foreign.execute(value);
         }
@@ -324,26 +328,23 @@ public Object execute(VirtualFrame frame) {
     @Override
     public CallTarget accessIsBoxed() {
         return Truffle.getRuntime().createCallTarget(new InteropRootNode() {
+            @Child IsNA isNANode = IsNANodeGen.create();
+            private final ConditionProfile isEmpty = ConditionProfile.createBinaryProfile();
+
             @Override
             public Object execute(VirtualFrame frame) {
                 RAbstractVector arg = (RAbstractVector) ForeignAccess.getReceiver(frame);
-                return arg.getLength() == 1 && isUnBoxable(arg);
+                if (isEmpty.profile(arg.getLength() == 0)) {
+                    return false;
+                }
+                Object o = arg.getDataAtAsObject(0);
+                return arg.getLength() == 1 && !isNA(isNANode, o);
             }
         });
     }
 
-    private static boolean isUnBoxable(RAbstractVector vector) {
-        Object o = vector.getDataAtAsObject(0);
-        return isPrimitive(o);
-    }
-
-    private static boolean isPrimitive(Object element) {
-        if (element == null) {
-            return false;
-        }
-        final Class<?> elementType = element.getClass();
-        return elementType == String.class || elementType == Character.class || elementType == Boolean.class || elementType == Byte.class || elementType == Short.class ||
-                        elementType == Integer.class || elementType == Long.class || elementType == Float.class || elementType == Double.class;
+    private static boolean isNA(IsNA isNANode, Object value) {
+        return RRuntime.fromLogical((Byte) isNANode.execute(value));
     }
 
     @Override
@@ -382,14 +383,21 @@ public Object execute(VirtualFrame frame) {
     @Override
     public CallTarget accessUnbox() {
         return Truffle.getRuntime().createCallTarget(new InteropRootNode() {
+            @Child IsNA isNANode = IsNANodeGen.create();
+            private final ConditionProfile isLogical = ConditionProfile.createBinaryProfile();
+
             @Override
             public Object execute(VirtualFrame frame) {
                 RAbstractVector arg = (RAbstractVector) ForeignAccess.getReceiver(frame);
                 if (arg.getLength() == 1) {
-                    return arg.getDataAtAsObject(0);
-                } else {
-                    throw UnsupportedMessageException.raise(Message.UNBOX);
+                    Object value = arg.getDataAtAsObject(0);
+                    if (isLogical.profile(arg instanceof RAbstractLogicalVector)) {
+                        return RLogicalMR.unboxLogical((Byte) value);
+                    } else if (!isNA(isNANode, value)) {
+                        return value;
+                    }
                 }
+                throw UnsupportedMessageException.raise(Message.UNBOX);
             }
         });
     }

com.oracle.truffle.r.engine/src/com/oracle/truffle/r/engine/interop/RForeignAccessFactoryImpl.java

@@ -38,6 +38,7 @@
 import com.oracle.truffle.r.runtime.data.RFunction;
 import com.oracle.truffle.r.runtime.data.RInteger;
 import com.oracle.truffle.r.runtime.data.RInteropScalar;
+import com.oracle.truffle.r.runtime.data.RInteropScalar.RInteropNA;
 import com.oracle.truffle.r.runtime.data.RPairList;
 import com.oracle.truffle.r.runtime.data.RList;
 import com.oracle.truffle.r.runtime.data.RLogical;
@@ -118,6 +119,8 @@ public ForeignAccess getForeignAccess(RTruffleObject obj) {
             return RMissingMRForeign.ACCESS;
         } else if (obj instanceof REmpty) {
             return REmptyMRForeign.ACCESS;
+        } else if (obj instanceof RInteropNA) {
+            return RInteropNAMRForeign.ACCESS;
         } else if (obj instanceof RAbstractAtomicVector) {
             return ForeignAccess.create(new RAbstractVectorAccessFactory(), new RAbstractVectorAccessFactory.Check());
         } else {

com.oracle.truffle.r.engine/src/com/oracle/truffle/r/engine/interop/RInteropNAMR.java

@@ -0,0 +1,47 @@
+/*
+ * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 3 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 3 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package com.oracle.truffle.r.engine.interop;
+
+import com.oracle.truffle.api.interop.CanResolve;
+import com.oracle.truffle.api.interop.MessageResolution;
+import com.oracle.truffle.api.interop.Resolve;
+import com.oracle.truffle.api.interop.TruffleObject;
+import com.oracle.truffle.api.nodes.Node;
+import com.oracle.truffle.r.runtime.data.RInteropScalar.RInteropNA;
+
+@MessageResolution(receiverType = RInteropNA.class)
+public class RInteropNAMR {
+    @Resolve(message = "IS_NULL")
+    public abstract static class RInteropNAIsNullNode extends Node {
+        protected Object access(@SuppressWarnings("unused") RInteropNA receiver) {
+            return true;
+        }
+    }
+
+    @CanResolve
+    public abstract static class RInteropNACheck extends Node {
+        protected static boolean test(TruffleObject receiver) {
+            return receiver instanceof RInteropNA;
+        }
+    }
+}

com.oracle.truffle.r.engine/src/com/oracle/truffle/r/engine/interop/RLogicalMR.java

@@ -23,18 +23,22 @@
 package com.oracle.truffle.r.engine.interop;
 
 import com.oracle.truffle.api.interop.CanResolve;
+import com.oracle.truffle.api.interop.Message;
 import com.oracle.truffle.api.interop.MessageResolution;
 import com.oracle.truffle.api.interop.Resolve;
 import com.oracle.truffle.api.interop.TruffleObject;
+import com.oracle.truffle.api.interop.UnsupportedMessageException;
 import com.oracle.truffle.api.nodes.Node;
+import com.oracle.truffle.r.runtime.RRuntime;
 import com.oracle.truffle.r.runtime.data.RLogical;
+import com.oracle.truffle.r.runtime.data.model.RAbstractLogicalVector;
 
 @MessageResolution(receiverType = RLogical.class)
 public class RLogicalMR {
     @Resolve(message = "IS_BOXED")
     public abstract static class RLogicalIsBoxedNode extends Node {
         protected Object access(@SuppressWarnings("unused") RLogical receiver) {
-            return true;
+            return !RRuntime.isNA(receiver.getValue());
         }
     }
 
@@ -47,8 +51,8 @@ protected Object access(@SuppressWarnings("unused") RLogical receiver, @Suppress
 
     @Resolve(message = "UNBOX")
     public abstract static class RLogicalUnboxNode extends Node {
-        protected byte access(RLogical receiver) {
-            return receiver.getValue();
+        protected Object access(RLogical receiver) {
+            return unboxLogical(receiver.getValue());
         }
     }
 
@@ -58,4 +62,15 @@ protected static boolean test(TruffleObject receiver) {
             return receiver instanceof RLogical;
         }
     }
+
+    public static boolean isUnboxable(byte value) {
+        return !RRuntime.isNA(value);
+    }
+
+    public static Object unboxLogical(byte value) {
+        if (!isUnboxable(value)) {
+            throw UnsupportedMessageException.raise(Message.UNBOX);
+        }
+        return RRuntime.fromLogical(value);
+    }
 }

com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/fastr/FastRInterop.java

@@ -104,7 +104,6 @@
 import com.oracle.truffle.r.runtime.interop.Foreign2R;
 import com.oracle.truffle.r.runtime.interop.ForeignArray2R;
 import com.oracle.truffle.r.runtime.interop.R2Foreign;
-import com.oracle.truffle.r.runtime.interop.R2ForeignNodeGen;
 
 public class FastRInterop {
 
@@ -818,7 +817,7 @@ public Object toArray(RInteropScalar ri, @SuppressWarnings("unused") RMissing cl
         }
 
         protected R2Foreign createR2Foreign() {
-            return R2Foreign.create();
+            return R2Foreign.createNoBox();
         }
 
         private static int[] getDim(boolean flat, RAbstractVector vec) {

com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RInteropScalar.java

@@ -24,6 +24,7 @@
 
 import com.oracle.truffle.api.CompilerAsserts;
 import com.oracle.truffle.api.CompilerDirectives.ValueType;
+import com.oracle.truffle.r.runtime.RRuntime;
 import com.oracle.truffle.r.runtime.RType;
 
 @ValueType
@@ -227,4 +228,26 @@ public Class<?> getJavaType() {
             return Short.TYPE;
         }
     }
+
+    /**
+     * Represents an {@code NA} value passed to the interop. This value should never appear in the
+     * FastR execution, it is only passed to interop and converted back to primitive value if passed
+     * back to FastR.
+     */
+    public static final class RInteropNA implements RTruffleObject {
+        public static final RInteropNA INT = new RInteropNA(RRuntime.INT_NA);
+        public static final RInteropNA DOUBLE = new RInteropNA(RRuntime.DOUBLE_NA);
+        public static final RInteropNA STRING = new RInteropNA(RRuntime.STRING_NA);
+        public static final RInteropNA LOGICAL = new RInteropNA(RRuntime.LOGICAL_NA);
+
+        private final Object value;
+
+        public RInteropNA(Object value) {
+            this.value = value;
+        }
+
+        public Object getValue() {
+            return value;
+        }
+    }
 }

com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/interop/Foreign2R.java

@@ -33,6 +33,7 @@
 import com.oracle.truffle.r.runtime.RInternalError;
 import com.oracle.truffle.api.nodes.Node;
 import com.oracle.truffle.r.runtime.RRuntime;
+import com.oracle.truffle.r.runtime.data.RInteropScalar.RInteropNA;
 import com.oracle.truffle.r.runtime.data.RNull;
 import com.oracle.truffle.r.runtime.nodes.RBaseNode;
 
@@ -80,6 +81,11 @@ public String doChar(char obj) {
         return ((Character) obj).toString();
     }
 
+    @Specialization
+    public Object doInteropNA(RInteropNA interopNA) {
+        return interopNA.getValue();
+    }
+
     @Specialization(guards = "isNull(obj)")
     public RNull doNull(@SuppressWarnings("unused") Object obj) {
         return RNull.instance;