From 82ad65e242fcf51245aac912192104720ee76493 Mon Sep 17 00:00:00 2001
From: stepan <stepan.sindelar@oracle.com>
Date: Wed, 18 Oct 2017 13:42:34 +0200
Subject: [PATCH] Fixes in Sprintf
* support lists and nested NULLs
* corner cases with empty width and/or zero padded format
---
.../truffle/r/nodes/builtin/base/Sprintf.java | 30 +++++++++++++---
.../truffle/r/test/ExpectedTestOutput.test | 35 +++++++++++++++++++
.../r/test/builtins/TestBuiltin_sprintf.java | 23 ++++++++++++
3 files changed, 84 insertions(+), 4 deletions(-)
diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Sprintf.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Sprintf.java
index fcadf58d32..0aa1c986c3 100644
--- a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Sprintf.java
+++ b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Sprintf.java
@@ -22,6 +22,7 @@
*/
package com.oracle.truffle.r.nodes.builtin.base;
+import static com.oracle.truffle.r.nodes.builtin.CastBuilder.Predef.stringValue;
import static com.oracle.truffle.r.runtime.builtins.RBehavior.PURE;
import static com.oracle.truffle.r.runtime.builtins.RBuiltinKind.INTERNAL;
@@ -31,11 +32,14 @@ import com.oracle.truffle.api.CompilerDirectives;
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
import com.oracle.truffle.api.dsl.Specialization;
import com.oracle.truffle.r.nodes.builtin.RBuiltinNode;
+import com.oracle.truffle.r.runtime.ArgumentsSignature;
import com.oracle.truffle.r.runtime.RError;
+import com.oracle.truffle.r.runtime.RError.Message;
import com.oracle.truffle.r.runtime.RRuntime;
import com.oracle.truffle.r.runtime.builtins.RBuiltin;
import com.oracle.truffle.r.runtime.data.RArgsValuesAndNames;
import com.oracle.truffle.r.runtime.data.RDataFactory;
+import com.oracle.truffle.r.runtime.data.RList;
import com.oracle.truffle.r.runtime.data.RMissing;
import com.oracle.truffle.r.runtime.data.RNull;
import com.oracle.truffle.r.runtime.data.RStringVector;
@@ -49,13 +53,19 @@ import com.oracle.truffle.r.runtime.data.model.RAbstractVector;
public abstract class Sprintf extends RBuiltinNode.Arg2 {
static {
- Casts.noCasts(Sprintf.class);
+ Casts casts = new Casts(Sprintf.class);
+ casts.arg("fmt").mustBe(stringValue()).asStringVector();
}
public abstract Object executeObject(String fmt, Object args);
@Child private Sprintf sprintfRecursive;
+ @Specialization
+ protected RStringVector sprintf(RAbstractStringVector fmt, RList values) {
+ return sprintf(fmt, new RArgsValuesAndNames(values.getReadonlyData(), ArgumentsSignature.empty(values.getLength())));
+ }
+
@Specialization
protected RStringVector sprintf(@SuppressWarnings("unused") RAbstractStringVector fmt, @SuppressWarnings("unused") RNull x) {
return RDataFactory.createEmptyStringVector();
@@ -272,7 +282,10 @@ public abstract class Sprintf extends RBuiltinNode.Arg2 {
data[i] = (String) formattedObj;
} else {
RStringVector formatted = (RStringVector) formattedObj;
- assert formatted.getLength() > 0;
+ if (formatted.getLength() == 0) {
+ // Any NULL inside args causes the whole result to be empty vector
+ return formatted;
+ }
data[i] = formatted.getDataAt(i % formatted.getLength());
}
}
@@ -494,7 +507,11 @@ public abstract class Sprintf extends RBuiltinNode.Arg2 {
widthAndPrecision(cs, j, fi);
j = fi.nextChar;
} else if (c == '.') {
- // apparently precision can be specified without width as well
+ // apparently precision can be specified without width as well, but in
+ // such case the '0' prefix if any should be interpreted as width...
+ if (fi.padZero) {
+ fi.padZero = false;
+ }
oneWidth(cs, j + 1, fi, false);
j = fi.nextChar;
} else {
@@ -532,7 +549,7 @@ public abstract class Sprintf extends RBuiltinNode.Arg2 {
if (isNumeric(cs[j])) {
n = number(cs, j, fi);
j = fi.nextChar;
- } else {
+ } else if (cs[j] == '*') {
assert cs[j] == '*';
if (width) {
fi.widthIsArg = true;
@@ -548,6 +565,11 @@ public abstract class Sprintf extends RBuiltinNode.Arg2 {
} else {
n = fi.argc++;
}
+ } else {
+ if (!isConversion(cs[j])) {
+ throw RError.error(RError.NO_CALLER, Message.UNRECOGNIZED_FORMAT, new String(cs));
+ }
+ n = 0;
}
if (width) {
fi.width = n;
diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test
index 4231476011..b85c0d981e 100644
--- a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test
+++ b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test
@@ -67588,6 +67588,33 @@ $`2`
[1] NA
+##com.oracle.truffle.r.test.builtins.TestBuiltin_sprintf.testCornerCases#Ignored.ImplementationError#
+#{ sprintf(c('hello %d %s', 'world %d %s'), list(2, 'x')) }
+Error in sprintf(c("hello %d %s", "world %d %s"), list(2, "x")) :
+ unsupported type
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_sprintf.testGenericDispatch#Ignored.Unimplemented#
+#{ as.character.myclass65231 <- function(x) '42'; y <- 2; class(y) <- 'myclass65231'; sprintf('%s', y); }
+[1] "42"
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_sprintf.testGenericDispatch#Ignored.Unimplemented#
+#{ as.double.myclass65321 <- function(x) 3.14; y <- 'str'; class(y) <- 'myclass65321'; sprintf('%g', y); }
+Error in sprintf("%g", y) :
+ invalid format '%g'; use format %s for character objects
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_sprintf.testGenericDispatch#Ignored.Unimplemented#
+#{ as.double.myclass65321 <- function(x) 3.14; y <- 3L; class(y) <- 'myclass65321'; sprintf('%g', y); }
+[1] "3.14"
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_sprintf.testSprintf#
+#{ asdfgerta <- sprintf('%.0f', 1); }
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_sprintf.testSprintf#
+#{ asdfgerta <- sprintf('%0.0f', 1); }
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_sprintf.testSprintf#
+#{ asdfgerta <- sprintf('%0.f', 1); }
+
##com.oracle.truffle.r.test.builtins.TestBuiltin_sprintf.testSprintf#
#{ sprintf("%.3g", 1.234) }
[1] "1.23"
@@ -67672,6 +67699,10 @@ character(0)
#{ sprintf("foo") }
[1] "foo"
+##com.oracle.truffle.r.test.builtins.TestBuiltin_sprintf.testSprintf#
+#{ sprintf('%s', list('hello world')) }
+[1] "hello world"
+
##com.oracle.truffle.r.test.builtins.TestBuiltin_sprintf.testSprintf#
#{ sprintf('plot_%02g', 3L) }
[1] "plot_03"
@@ -67692,6 +67723,10 @@ character(0)
#{ sprintf(c("foo", "bar")) }
[1] "foo" "bar"
+##com.oracle.truffle.r.test.builtins.TestBuiltin_sprintf.testSprintf#
+#{ sprintf(c('hello', 'world'), NULL) }
+character(0)
+
##com.oracle.truffle.r.test.builtins.TestBuiltin_sprintf.testsprintf1#
#argv <- list('%s is not TRUE', 'identical(fxy, c(1, 2, 3))'); .Internal(sprintf(argv[[1]], argv[[2]]))
[1] "identical(fxy, c(1, 2, 3)) is not TRUE"
diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_sprintf.java b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_sprintf.java
index 4fee95e62e..de38c81297 100644
--- a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_sprintf.java
+++ b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_sprintf.java
@@ -151,5 +151,28 @@ public class TestBuiltin_sprintf extends TestBase {
assertEval("{ sprintf(c(\"foo %f %d\", \"bar %f %d\"), c(7,1), c(42L, 2L)) }");
assertEval("{ sprintf(\"%.3g\", 1.234) }");
assertEval("{ sprintf('plot_%02g', 3L) }");
+ assertEval("{ sprintf(c('hello', 'world'), NULL) }");
+ assertEval("{ sprintf('%s', list('hello world')) }");
+ // Note: we save the result to variable to avoid diff because of different formatting,
+ // however, at least we test that the format expression is not causing any problems to
+ // FastR.
+ assertEval("{ asdfgerta <- sprintf('%0.f', 1); }");
+ assertEval("{ asdfgerta <- sprintf('%0.0f', 1); }");
+ assertEval("{ asdfgerta <- sprintf('%.0f', 1); }");
+ }
+
+ @Test
+ public void testGenericDispatch() {
+ // from the doc: sprintf uses as.character for %s arguments, and as.double for %f, %E, ...
+ assertEval(Ignored.Unimplemented, "{ as.character.myclass65231 <- function(x) '42'; y <- 2; class(y) <- 'myclass65231'; sprintf('%s', y); }");
+ assertEval(Ignored.Unimplemented, "{ as.double.myclass65321 <- function(x) 3.14; y <- 3L; class(y) <- 'myclass65321'; sprintf('%g', y); }");
+ // catch: probably before calling as.double there is numeric validation? Strings are not OK
+ // even, when they have as.double function
+ assertEval(Ignored.Unimplemented, "{ as.double.myclass65321 <- function(x) 3.14; y <- 'str'; class(y) <- 'myclass65321'; sprintf('%g', y); }");
+ }
+
+ @Test
+ public void testCornerCases() {
+ assertEval(Ignored.ImplementationError, "{ sprintf(c('hello %d %s', 'world %d %s'), list(2, 'x')) }");
}
}
--
GitLab