UpdateStorageMode: support NULL and show proper error messages

Author: steve-s

com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/UpdateStorageMode.java

@@ -28,6 +28,7 @@
 import com.oracle.truffle.api.dsl.Cached;
 import com.oracle.truffle.api.dsl.Specialization;
 import com.oracle.truffle.api.object.DynamicObject;
+import com.oracle.truffle.api.profiles.ValueProfile;
 import com.oracle.truffle.r.nodes.attributes.ArrayAttributeNode;
 import com.oracle.truffle.r.nodes.attributes.SetAttributeNode;
 import com.oracle.truffle.r.nodes.attributes.SpecialAttributesFunctions.SetClassAttributeNode;
@@ -62,13 +63,23 @@ public abstract class UpdateStorageMode extends RBuiltinNode.Arg2 {
     }
 
     @Specialization
+    protected Object updateNull(@SuppressWarnings("unused") RNull x, String value,
+                    @Cached("createEqualityProfile()") ValueProfile valueProfile) {
+        RType mode = typeFromMode.execute(valueProfile.profile(value));
+        checkMode(mode);
+        return mode.create(0, false);
+    }
+
+    protected static boolean notNull(Object x) {
+        return RNull.instance != x;
+    }
+
+    @Specialization(guards = "notNull(x)")
     protected Object update(Object x, String value,
                     @Cached("create()") ArrayAttributeNode attrAttrAccess,
                     @Cached("create()") SetAttributeNode setAttrNode) {
         RType mode = typeFromMode.execute(value);
-        if (mode == RType.DefunctReal || mode == RType.DefunctSingle) {
-            throw error(RError.Message.USE_DEFUNCT, mode.getName(), mode == RType.DefunctSingle ? "mode<-" : "double");
-        }
+        checkMode(mode);
         initTypeOfNode();
         RType typeX = typeof.execute(x);
         if (typeX == mode) {
@@ -115,6 +126,12 @@ protected Object update(Object x, String value,
         throw error(RError.Message.INVALID_UNNAMED_VALUE);
     }
 
+    private void checkMode(RType mode) {
+        if (mode == RType.DefunctReal || mode == RType.DefunctSingle) {
+            throw error(RError.Message.USE_DEFUNCT, mode.getName(), mode == RType.DefunctSingle ? "mode<-" : "double");
+        }
+    }
+
     private void initCastTypeNode() {
         if (castTypeNode == null) {
             CompilerDirectives.transferToInterpreterAndInvalidate();

com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/RError.java

@@ -574,6 +574,7 @@ public enum Message {
         ARGUMENT_NOT_VECTOR("argument %d is not a vector"),
         CANNOT_COERCE("cannot coerce type '%s' to vector of type '%s'"),
         CANNOT_COERCE_RFFI("(%s) object cannot be coerced to type '%s'"),
+        CANNOT_COERCE_QUOTED("'%s' object cannot be coerced to type '%s'"),
         ARGUMENT_ONLY_FIRST("argument '%s' has length > 1 and only the first element will be used"),
         ARGUMENT_ONLY_FIRST_1("only the first element of '%s' argument used"),
         ARGUMENT_WRONG_LENGTH("wrong length for argument"),

com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test

@@ -74529,6 +74529,24 @@ can write   "no"
 #argv <- structure(list(x = structure(c(1, 0.666666666666667,     0.333333333333333, 0, -0.333333333333333, -0.666666666666667,     -1, -1.33333333333333, -1.66666666666667, 1.5, 1, 0.5, 0,     -0.5, -1, -1.5, -2, -2.5, 3, 2, 1, 0, -1, -2, -3, -4, -5,     -Inf, -Inf, -Inf, NaN, Inf, Inf, Inf, Inf, Inf, -3, -2, -1,     0, 1, 2, 3, 4, 5, -1.5, -1, -0.5, 0, 0.5, 1, 1.5, 2, 2.5,     -1, -0.666666666666667, -0.333333333333333, 0, 0.333333333333333,     0.666666666666667, 1, 1.33333333333333, 1.66666666666667,     -0.75, -0.5, -0.25, 0, 0.25, 0.5, 0.75, 1, 1.25, -0.6, -0.4,     -0.2, 0, 0.2, 0.4, 0.6, 0.8, 1), .Dim = c(9L, 9L))), .Names = 'x');do.call('storage.mode', argv)
 [1] "double"
 
+##com.oracle.truffle.r.test.builtins.TestBuiltin_storagemodeassign.testErrors#
+#{ x <- 1; storage.mode(x) <- 42.5; }
+Error in storage.mode(x) <- 42.5 :
+  'value' must be non-null character string
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_storagemodeassign.testErrors#Output.IgnoreErrorMessage#
+#{ x <- as.pairlist(1); storage.mode(x) <- 'integer'; }
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_storagemodeassign.testErrors#Output.IgnoreErrorMessage#
+#{ x <- new.env(); storage.mode(x) <- 'integer'; }
+Error in storage.mode(x) <- "integer" :
+  environments cannot be coerced to other types
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_storagemodeassign.testErrors#Output.IgnoreErrorMessage#
+#{ x <- quote(a+b); storage.mode(x) <- 'integer'; }
+Error in storage.mode(x) <- "integer" :
+  'pairlist' object cannot be coerced to type 'integer'
+
 ##com.oracle.truffle.r.test.builtins.TestBuiltin_storagemodeassign.testUpdateStorageMode#
 #{ x <- c(1L, 2L); dim(x)<-c(1,2); storage.mode(x) <- "double"; x}
      [,1] [,2]
@@ -74541,6 +74559,30 @@ can write   "no"
 #{ x <- c(1L, 2L); storage.mode(x) <- "not.double"}
 Error in storage.mode(x) <- "not.double" : invalid value
 
+##com.oracle.truffle.r.test.builtins.TestBuiltin_storagemodeassign.testUpdateStorageModeWithNull#
+#{ x <- NULL; storage.mode(x) <- 'character'; x }
+character(0)
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_storagemodeassign.testUpdateStorageModeWithNull#
+#{ x <- NULL; storage.mode(x) <- 'double'; x }
+numeric(0)
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_storagemodeassign.testUpdateStorageModeWithNull#
+#{ x <- NULL; storage.mode(x) <- 'integer'; x }
+integer(0)
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_storagemodeassign.testUpdateStorageModeWithNull#
+#{ x <- NULL; storage.mode(x) <- 'logical'; x }
+logical(0)
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_storagemodeassign.testUpdateStorageModeWithNull#
+#{ x <- NULL; storage.mode(x) <- 'numeric'; x }
+numeric(0)
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_storagemodeassign.testUpdateStorageModeWithNull#
+#{ x <- NULL; storage.mode(x) <- 'raw'; x }
+raw(0)
+
 ##com.oracle.truffle.r.test.builtins.TestBuiltin_storagemodeassign.teststoragemodeassign1#
 #argv <- list(structure(c(4L, 5L, 10L, 9L, 13L, 13L, 12L, 15L, 18L, 19L, 22L, 27L, 28L, 24L, 27L, 28L, 30L, 31L, 32L, 36L, 28L, 32L, 35L, 33L, 38L, 41L, 38L, 38L, 32L, 34L, 44L, 44L, 44L, 46L, 47L, 49L, 50L, 53L, 52L, 55L, 54L, 60L, 63L, 86L, 85L, 85L, 78L, 74L, 97L, 98L, 98L, 99L, 99L, 101L, 108L, 110L, 108L, 111L, 115L, 117L, 70L, 77L, 83L, 61L, 69L, 78L, 66L, 58L, 64L, 69L, 66L, 61L, 76L, 72L, 64L, 53L, 63L, 59L, 77L, 49L, 69L, 88L, 75L, 61L, 65L, 74L, 72L, 76L, 58L, 55L, 60L, 52L, 60L, 61L, 72L, 147L, 149L, 153L, 154L, 151L, 150L, 145L, 143L, 143L, 141L, 156L, 149L, 143L, 142L, 149L, 152L, 142L, 144L, 152L, 155L, 124L, 136L, 139L, 132L, 115L, 96L, 94L, 96L, 122L, 116L, 124L, 119L, 128L, 115L, 111L, 111L, 116L, 126L, 117L, 115L, 4L, 12L, 21L, 15L, 15L, 16L, 18L, 13L, 20L, 21L, 23L, 25L, 27L, 31L, 30L), .Dim = c(75L, 2L), .Dimnames = list(c('1', '2', '3', '4', '5', '6', '7', '8', '9', '10', '11', '12', '13', '14', '15', '16', '17', '18', '19', '20', '21', '22', '23', '24', '25', '26', '27', '28', '29', '30', '31', '32', '33', '34', '35', '36', '37', '38', '39', '40', '41', '42', '43', '44', '45', '46', '47', '48', '49', '50', '51', '52', '53', '54', '55', '56', '57', '58', '59', '60', '61', '62', '63', '64', '65', '66', '67', '68', '69', '70', '71', '72', '73', '74', '75'), c('x', 'y'))), value = 'double');`storage.mode<-`(argv[[1]],argv[[2]]);
      x   y

com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_storagemodeassign.java

@@ -58,4 +58,16 @@ public void testUpdateStorageMode() {
         assertEval("{ x <- c(1L, 2L); storage.mode(x) <- \"not.double\"}");
         assertEval("{ x <- c(1L, 2L); dim(x)<-c(1,2); storage.mode(x) <- \"double\"; x}");
     }
+
+    @Test
+    public void testUpdateStorageModeWithNull() {
+        assertEval(template("{ x <- NULL; storage.mode(x) <- '%0'; x }", new String[]{"integer", "character", "logical", "double", "numeric", "raw"}));
+    }
+
+    @Test
+    public void testErrors() {
+        // TODO: need to fix cast pipeline and specializations in UpdateStorageMode
+        assertEval(Ignored.ImplementationError, template("{ x <- %0; storage.mode(x) <- 'integer'; }", new String[]{"new.env()", "quote(a+b)", "as.pairlist(1)"}));
+        assertEval("{ x <- 1; storage.mode(x) <- 42.5; }");
+    }
 }