[GR-28147] Fix AssertionError in Sprintf.

PullRequest: fastr/2544
oracle · Dec 14, 2020 · 26ff8cf · 26ff8cf
2 parents 10266f0 + 4cb2ffe
commit 26ff8cf
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 15 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,9 @@
+# 21.0.0
+
+Bug fixes:
+
+* Fix `AssertionError` in `sprintf` (#169)
+
 # 20.3.0
 
 Bug fixes:

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
@@ -202,16 +202,11 @@ private static int maxLengthAndConvertToScalar(Object[] values) {
         for (int i = 0; i < values.length; i++) {
             if (values[i] instanceof RAbstractVector) {
                 int vecLength = ((RAbstractVector) values[i]).getLength();
-                if (vecLength == 0) {
-                    // result will be empty character vector in this case, as in:
-                    // sprintf("%d %d", as.integer(c(7,42)), integer())
-                    return 0;
-                } else {
-                    if (vecLength == 1) {
-                        values[i] = ((RAbstractVector) values[i]).getDataAtAsObject(0);
-                    }
-                    length = Math.max(vecLength, length);
+                assert vecLength != 0;
+                if (vecLength == 1) {
+                    values[i] = ((RAbstractVector) values[i]).getDataAtAsObject(0);
                 }
+                length = Math.max(vecLength, length);
             } else {
                 length = Math.max(1, length);
             }
@@ -231,12 +226,12 @@ private static Object[] createSprintfArgs(Object[] values, int index, int maxLen
         return sprintfArgs;
     }
 
-    @Specialization(guards = {"!oneElement(args)", "hasNull(args)"})
+    @Specialization(guards = {"!oneElement(args)", "hasNullOrEmptyVec(args)"})
     protected RStringVector sprintf(@SuppressWarnings("unused") Object fmt, @SuppressWarnings("unused") RArgsValuesAndNames args) {
         return RDataFactory.createEmptyStringVector();
     }
 
-    @Specialization(guards = {"!oneElement(args)", "!hasNull(args)"})
+    @Specialization(guards = {"!oneElement(args)", "!hasNullOrEmptyVec(args)"})
     @TruffleBoundary
     protected RStringVector sprintf(String fmt, RArgsValuesAndNames args) {
         Object[] values = args.getArguments();
@@ -267,7 +262,7 @@ protected Object sprintfOneElement(String fmt, RArgsValuesAndNames args) {
         return sprintfRecursive.executeObject(fmt, args.getArgument(0));
     }
 
-    @Specialization(guards = {"!oneElement(args)", "!hasNull(args)"})
+    @Specialization(guards = {"!oneElement(args)", "!hasNullOrEmptyVec(args)"})
     @TruffleBoundary
     protected RStringVector sprintf(RStringVector fmt, RArgsValuesAndNames args) {
         if (fmt.getLength() == 0) {
@@ -276,8 +271,15 @@ protected RStringVector sprintf(RStringVector fmt, RArgsValuesAndNames args) {
             String[] data = new String[fmt.getLength()];
             for (int i = 0; i < data.length; i++) {
                 RStringVector formatted = sprintf(fmt.getDataAt(i), args);
-                assert formatted.getLength() > 0;
-                data[i] = formatted.getDataAt(args.getLength() == 0 ? 0 : i % Math.min(args.getLength(), formatted.getLength()));
+                if (args.getLength() == 0) {
+                    if (formatted.getLength() == 0) {
+                        data[i] = null;
+                    } else {
+                        data[i] = formatted.getDataAt(0);
+                    }
+                } else {
+                    data[i] = formatted.getDataAt(i % Math.min(args.getLength(), formatted.getLength()));
+                }
             }
             return RDataFactory.createStringVector(data, RDataFactory.COMPLETE_VECTOR);
         }
@@ -739,10 +741,15 @@ protected boolean oneElement(RArgsValuesAndNames args) {
         return args.getLength() == 1;
     }
 
-    protected boolean hasNull(RArgsValuesAndNames args) {
+    protected boolean hasNullOrEmptyVec(RArgsValuesAndNames args) {
         for (int i = 0; i < args.getLength(); i++) {
             if (args.getArgument(i) == RNull.instance) {
                 return true;
+            } else if (args.getArgument(i) instanceof RAbstractVector) {
+                RAbstractVector vector = (RAbstractVector) args.getArgument(i);
+                if (vector.getLength() == 0) {
+                    return true;
+                }
             }
         }
 

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
@@ -82782,10 +82782,26 @@ character(0)
 #{ sprintf('%.3i', 4.0) }
 [1] "004"
 
+##com.oracle.truffle.r.test.builtins.TestBuiltin_sprintf.testSprintf#
+#{ sprintf('%d%s', NULL, 'Hello') }
+character(0)
+
 ##com.oracle.truffle.r.test.builtins.TestBuiltin_sprintf.testSprintf#
 #{ sprintf('%g', 4.3345423) }
 [1] "4.33454"
 
+##com.oracle.truffle.r.test.builtins.TestBuiltin_sprintf.testSprintf#
+#{ sprintf('%s%d', 'Hello', NULL) }
+character(0)
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_sprintf.testSprintf#
+#{ sprintf('%s%d', 'Hello', c()) }
+character(0)
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_sprintf.testSprintf#
+#{ sprintf('%s%d', 'Hello', seq_along(c())) }
+character(0)
+
 ##com.oracle.truffle.r.test.builtins.TestBuiltin_sprintf.testSprintf#
 #{ sprintf('%s', list('hello world')) }
 [1] "hello world"

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
@@ -180,6 +180,12 @@ public void testSprintf() {
         assertEval("{ sprintf('% g', 4.33) }");
         assertEval("{ sprintf('%g', 4.3345423) }");
 
+        // If one of args is NULL or an empty vector, sprintf should produce character(0).
+        assertEval("{ sprintf('%s%d', 'Hello', c()) }");
+        assertEval("{ sprintf('%s%d', 'Hello', NULL) }");
+        assertEval("{ sprintf('%d%s', NULL, 'Hello') }");
+        assertEval("{ sprintf('%s%d', 'Hello', seq_along(c())) }");
+
         assertEval(Ignored.ImplementationError, "{ sprintf('%#g', 4.0) }");
     }