[GR-10977] [GR-10799] Unify possible results of R2Foreign.

PullRequest: fastr/1610

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/EngineRootNode.java

@@ -58,7 +58,7 @@ class EngineRootNode extends RootNode {
     private final ContextReference<RContext> contextReference;
 
     @Child private EngineBodyNode bodyNode;
-    @Child private R2Foreign r2Foreign = R2ForeignNodeGen.create();
+    @Child private R2Foreign r2Foreign = R2Foreign.create();
 
     EngineRootNode(EngineBodyNode bodyNode, RContext context, SourceSection sourceSection, MaterializedFrame executionFrame) {
         super(context.getLanguage());

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

@@ -395,7 +395,7 @@ protected ListKeyInfoImplNode createKeyInfoNode() {
         private void initR2ForeignNode() {
             if (r2Foreign == null) {
                 CompilerDirectives.transferToInterpreterAndInvalidate();
-                r2Foreign = insert(R2ForeignNodeGen.create());
+                r2Foreign = insert(R2Foreign.create());
             }
         }
 

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/RArgsValuesAndNamesMR.java

@@ -120,7 +120,7 @@ protected static boolean test(TruffleObject receiver) {
     }
 
     abstract static class RArgsValuesAndNamesReadImplNode extends Node {
-        @Child private R2Foreign r2Foreign = R2ForeignNodeGen.create();
+        @Child private R2Foreign r2Foreign = R2Foreign.create();
 
         private final ConditionProfile unknownIdentifier = ConditionProfile.createBinaryProfile();
 

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

@@ -164,7 +164,7 @@ protected REnvironmentKeyInfoImplNode createKeyInfoNode() {
         private void initR2ForeignNode() {
             if (r2Foreign == null) {
                 CompilerDirectives.transferToInterpreterAndInvalidate();
-                r2Foreign = insert(R2ForeignNodeGen.create());
+                r2Foreign = insert(R2Foreign.create());
             }
         }
 

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/RFunctionMR.java

@@ -59,7 +59,7 @@ protected Object access(@SuppressWarnings("unused") RFunction receiver) {
     @Resolve(message = "EXECUTE")
     public abstract static class RFunctionExecuteNode extends Node {
         @Child private Foreign2R foreign2R = Foreign2RNodeGen.create();
-        @Child private R2Foreign r2Foreign = R2ForeignNodeGen.create();
+        @Child private R2Foreign r2Foreign = R2Foreign.create();
         @Child private RExplicitCallNode call = RExplicitCallNode.create();
 
         protected Object access(RFunction receiver, Object[] arguments) {

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);
+    }
 }