Fix: use the SetAttributeNode in the RFFI function Rf_setAttrib

This means that the correct coercion and validation will take place.

Author: steve-s

com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/common/JavaUpCallsRFFIImpl.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2019, 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
@@ -320,35 +320,8 @@ public Object Rf_getAttrib(Object obj, Object name) {
     }
 
     @Override
-    @TruffleBoundary
     public void Rf_setAttrib(Object obj, Object name, Object val) {
-        if (obj == RNull.instance) {
-            return;
-        }
-        if (obj instanceof RAttributable) {
-            RAttributable attrObj = (RAttributable) obj;
-            String nameAsString;
-            if (name instanceof RSymbol) {
-                nameAsString = ((RSymbol) name).getName();
-            } else {
-                nameAsString = RRuntime.asString(name);
-                assert nameAsString != null;
-            }
-            nameAsString = Utils.intern(nameAsString);
-            if (val == RNull.instance) {
-                if ("class" == nameAsString) {
-                    removeClassAttr(attrObj);
-                } else {
-                    removeAttr(attrObj, nameAsString);
-                }
-            } else if ("class" == nameAsString) {
-                attrObj.initAttributes().define(nameAsString, val);
-            } else {
-                attrObj.setAttr(nameAsString, val);
-            }
-        } else {
-            throw RInternalError.shouldNotReachHere();
-        }
+        throw implementedAsNode();
     }
 
     @TruffleBoundary

com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/nodes/AttributesAccessNodes.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2019, 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
@@ -34,11 +34,13 @@
 import com.oracle.truffle.api.profiles.ConditionProfile;
 import com.oracle.truffle.r.ffi.impl.nodes.AttributesAccessNodesFactory.ATTRIBNodeGen;
 import com.oracle.truffle.r.ffi.impl.nodes.AttributesAccessNodesFactory.CopyMostAttribNodeGen;
+import com.oracle.truffle.r.ffi.impl.nodes.AttributesAccessNodesFactory.RfSetAttribNodeGen;
 import com.oracle.truffle.r.ffi.impl.nodes.AttributesAccessNodesFactory.SetAttribNodeGen;
 import com.oracle.truffle.r.ffi.impl.nodes.AttributesAccessNodesFactory.TAGNodeGen;
 import com.oracle.truffle.r.nodes.attributes.CopyOfRegAttributesNode;
 import com.oracle.truffle.r.nodes.attributes.GetAttributeNode;
 import com.oracle.truffle.r.nodes.attributes.GetAttributesNode;
+import com.oracle.truffle.r.nodes.attributes.RemoveAttributeNode;
 import com.oracle.truffle.r.nodes.attributes.SetAttributeNode;
 import com.oracle.truffle.r.nodes.attributes.SpecialAttributesFunctions.GetRowNamesAttributeNode;
 import com.oracle.truffle.r.nodes.function.opt.UpdateShareableChildValueNode;
@@ -91,6 +93,53 @@ private String castToString(Object name) {
         }
     }
 
+    /**
+     * Sets a single attribute.
+     */
+    public abstract static class RfSetAttribNode extends FFIUpCallNode.Arg3 {
+        @Child private CastNode castNameNode;
+
+        public static RfSetAttribNode create() {
+            return RfSetAttribNodeGen.create();
+        }
+
+        @Specialization(guards = "!isNull(value)")
+        protected Object setValue(RAttributable target, Object name, Object value,
+                        @Cached("create()") SetAttributeNode setAttribNode) {
+            setAttribNode.execute(target, (String) getCastNameNode().doCast(name), value);
+            return RNull.instance;
+        }
+
+        @Specialization
+        protected Object setValue(@SuppressWarnings("unused") RNull target, @SuppressWarnings("unused") Object name, @SuppressWarnings("unused") Object value) {
+            return RNull.instance;
+        }
+
+        @Specialization
+        protected Object unsetValue(RAttributable target, Object name, @SuppressWarnings("unused") RNull nullVal,
+                        @Cached("create()") RemoveAttributeNode removeAttributeNode) {
+            removeAttributeNode.execute(target, (String) getCastNameNode().doCast(name));
+            return RNull.instance;
+        }
+
+        @Fallback
+        protected Object others(Object target, Object name, Object value) {
+            throw unsupportedTypes("Rf_setAttrib", target, name, value);
+        }
+
+        protected static boolean isNull(Object o) {
+            return o == RNull.instance;
+        }
+
+        private CastNode getCastNameNode() {
+            if (castNameNode == null) {
+                CompilerDirectives.transferToInterpreterAndInvalidate();
+                castNameNode = insert(newCastBuilder().asStringVector().findFirst().buildCastNode());
+            }
+            return castNameNode;
+        }
+    }
+
     public abstract static class ATTRIB extends FFIUpCallNode.Arg1 {
 
         @Specialization
@@ -208,6 +257,10 @@ public static CopyMostAttrib create() {
         }
     }
 
+    /**
+     * Overrides the attributes pairlist of given object with a new pairlist. In FastR, we have to
+     * convert the pairlist to our representation.
+     */
     public abstract static class SetAttribNode extends FFIUpCallNode.Arg2 {
 
         public static SetAttribNode create() {

com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/upcalls/StdUpCallsRFFI.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2019, 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
@@ -29,6 +29,7 @@
 import com.oracle.truffle.r.ffi.impl.nodes.AttributesAccessNodes.ATTRIB;
 import com.oracle.truffle.r.ffi.impl.nodes.AttributesAccessNodes.CopyMostAttrib;
 import com.oracle.truffle.r.ffi.impl.nodes.AttributesAccessNodes.GetAttrib;
+import com.oracle.truffle.r.ffi.impl.nodes.AttributesAccessNodes.RfSetAttribNode;
 import com.oracle.truffle.r.ffi.impl.nodes.AttributesAccessNodes.SetAttribNode;
 import com.oracle.truffle.r.ffi.impl.nodes.AttributesAccessNodes.TAG;
 import com.oracle.truffle.r.ffi.impl.nodes.CoerceNodes.AsCharacterFactor;
@@ -216,6 +217,7 @@ public interface StdUpCallsRFFI {
     @RFFIUpCallNode(GetAttrib.class)
     Object Rf_getAttrib(@RFFIResultOwner Object obj, Object name);
 
+    @RFFIUpCallNode(RfSetAttribNode.class)
     void Rf_setAttrib(Object obj, Object name, Object val);
 
     int Rf_inherits(Object x, @RFFICstring String clazz);