diff --git a/CHANGELOG.md b/CHANGELOG.md index 502a99849e..9a74d8b2fb 100644 --- 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 index 22cd35ace1..083951e8e5 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 @@ -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 index 97b462f5a2..282ea17996 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 @@ -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 index 2f090be7cd..8572414624 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 @@ -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) }"); }