From f7f245051893e7dd4ec71ef5b4ea24ed9646a548 Mon Sep 17 00:00:00 2001 From: stepan <stepan.sindelar@oracle.com> Date: Mon, 23 Apr 2018 15:53:37 +0200 Subject: [PATCH] Fixes in `rep` with NULL and empty vectors --- .../truffle/r/nodes/builtin/base/Repeat.java | 10 ++++-- .../r/runtime/data/RComplexVector.java | 14 ++++---- .../truffle/r/runtime/data/RDoubleVector.java | 6 ++-- .../truffle/r/runtime/data/RIntVector.java | 6 ++-- .../truffle/r/runtime/data/RListBase.java | 1 + .../r/runtime/data/RLogicalVector.java | 12 ++++--- .../truffle/r/runtime/data/RRawVector.java | 6 ++-- .../truffle/r/runtime/data/RStringVector.java | 12 ++++--- .../truffle/r/test/ExpectedTestOutput.test | 33 +++++++++++++++++++ .../r/test/builtins/TestBuiltin_rep.java | 10 ++++++ 10 files changed, 86 insertions(+), 24 deletions(-) diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Repeat.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Repeat.java index 5cee144e27..bd8d2c4312 100644 --- a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Repeat.java +++ b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Repeat.java @@ -58,6 +58,7 @@ import com.oracle.truffle.r.runtime.data.RArgsValuesAndNames; import com.oracle.truffle.r.runtime.data.RAttributesLayout; import com.oracle.truffle.r.runtime.data.RDataFactory; import com.oracle.truffle.r.runtime.data.RMissing; +import com.oracle.truffle.r.runtime.data.RNull; import com.oracle.truffle.r.runtime.data.RStringVector; import com.oracle.truffle.r.runtime.data.RTypes; import com.oracle.truffle.r.runtime.data.RVector; @@ -99,7 +100,7 @@ public abstract class Repeat extends RBuiltinNode.Arg2 { static { Casts casts = new Casts(Repeat.class); - casts.arg("x").mustBe(abstractVectorValue(), RError.Message.ATTEMPT_TO_REPLICATE, typeName()); + casts.arg("x").allowNull().mustBe(abstractVectorValue(), RError.Message.ATTEMPT_TO_REPLICATE, typeName()); // prepare cast pipeline nodes for vararg matching PB_TIMES = new PipelineBuilder("times"); @@ -137,6 +138,11 @@ public abstract class Repeat extends RBuiltinNode.Arg2 { return new Object[]{RMissing.instance, RArgsValuesAndNames.EMPTY}; } + @Specialization + protected RNull repeatNull(@SuppressWarnings("unused") RNull x, @SuppressWarnings("unused") RArgsValuesAndNames args) { + return RNull.instance; + } + @Specialization protected Object repeat(VirtualFrame frame, RAbstractVector x, RArgsValuesAndNames args) { RArgsValuesAndNames margs = prepareArgs.execute(args, null); @@ -238,7 +244,7 @@ public abstract class Repeat extends RBuiltinNode.Arg2 { * Extend or truncate the vector to a specified length. */ private static RVector<?> handleLengthOut(RAbstractVector x, int lengthOut) { - return x.copyResized(lengthOut, false); + return x.copyResized(lengthOut, x.getLength() == 0); } /** diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RComplexVector.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RComplexVector.java index a5498304f6..7e9bbe7ebd 100644 --- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RComplexVector.java +++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RComplexVector.java @@ -153,16 +153,18 @@ public final class RComplexVector extends RVector<double[]> implements RAbstract private double[] copyResizedData(int size, boolean fillNA) { int csize = size << 1; - double[] newData = Arrays.copyOf(getReadonlyData(), csize); - if (csize > this.getLength()) { + double[] localData = getReadonlyData(); + double[] newData = Arrays.copyOf(localData, csize); + if (csize > localData.length) { if (fillNA) { - for (int i = data.length; i < size; i++) { + for (int i = localData.length; i < csize; i++) { newData[i] = RRuntime.DOUBLE_NA; } } else { - for (int i = data.length, j = 0; i <= csize - 2; i += 2, j = Utils.incMod(j + 1, data.length)) { - newData[i] = data[j]; - newData[i + 1] = data[j + 1]; + assert localData.length > 0 : "cannot call resize on empty vector if fillNA == false"; + for (int i = localData.length, j = 0; i <= csize - 2; i += 2, j = Utils.incMod(j + 1, localData.length)) { + newData[i] = localData[j]; + newData[i + 1] = localData[j + 1]; } } } diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RDoubleVector.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RDoubleVector.java index c591f5f6b4..893b7512a9 100644 --- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RDoubleVector.java +++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RDoubleVector.java @@ -178,6 +178,7 @@ public final class RDoubleVector extends RVector<double[]> implements RAbstractD newData[i] = RRuntime.DOUBLE_NA; } } else { + assert oldData.length > 0 : "cannot call resize on empty vector if fillNA == false"; for (int i = oldDataLength, j = 0; i < newData.length; ++i, j = Utils.incMod(j, oldDataLength)) { newData[i] = oldData[j]; } @@ -187,8 +188,9 @@ public final class RDoubleVector extends RVector<double[]> implements RAbstractD } private double[] copyResizedData(int size, boolean fillNA) { - double[] newData = Arrays.copyOf(getReadonlyData(), size); - return resizeData(newData, this.data, this.getLength(), fillNA); + double[] localData = getReadonlyData(); + double[] newData = Arrays.copyOf(localData, size); + return resizeData(newData, localData, localData.length, fillNA); } @Override diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RIntVector.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RIntVector.java index d3552c1fdb..20438aaa0d 100644 --- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RIntVector.java +++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RIntVector.java @@ -181,6 +181,7 @@ public final class RIntVector extends RVector<int[]> implements RAbstractIntVect newData[i] = RRuntime.INT_NA; } } else { + assert oldDataLength > 0 : "cannot call resize on empty vector if fillNA == false"; for (int i = oldDataLength, j = 0; i < newData.length; ++i, j = Utils.incMod(j, oldDataLength)) { newData[i] = oldData[j]; } @@ -190,8 +191,9 @@ public final class RIntVector extends RVector<int[]> implements RAbstractIntVect } private int[] copyResizedData(int size, boolean fillNA) { - int[] newData = Arrays.copyOf(getReadonlyData(), size); - return resizeData(newData, this.data, this.getLength(), fillNA); + int[] localData = getReadonlyData(); + int[] newData = Arrays.copyOf(localData, size); + return resizeData(newData, localData, localData.length, fillNA); } @Override diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RListBase.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RListBase.java index 3c59ac2adc..ad9adb0365 100644 --- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RListBase.java +++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RListBase.java @@ -169,6 +169,7 @@ public abstract class RListBase extends RVector<Object[]> implements RAbstractLi newData[i] = RNull.instance; } } else { + assert oldDataLength > 0 : "cannot call resize on empty vector if fillNA == false"; for (int i = oldData.length, j = 0; i < newData.length; ++i, j = Utils.incMod(j, oldData.length)) { newData[i] = oldData[j]; } diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RLogicalVector.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RLogicalVector.java index c5f4390abf..483fdec598 100644 --- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RLogicalVector.java +++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RLogicalVector.java @@ -154,15 +154,17 @@ public final class RLogicalVector extends RVector<byte[]> implements RAbstractLo } private byte[] copyResizedData(int size, boolean fillNA) { - byte[] newData = Arrays.copyOf(getReadonlyData(), size); - if (size > this.getLength()) { + byte[] localData = getReadonlyData(); + byte[] newData = Arrays.copyOf(localData, size); + if (size > localData.length) { if (fillNA) { - for (int i = data.length; i < size; i++) { + for (int i = localData.length; i < size; i++) { newData[i] = RRuntime.LOGICAL_NA; } } else { - for (int i = data.length, j = 0; i < size; ++i, j = Utils.incMod(j, data.length)) { - newData[i] = data[j]; + assert localData.length > 0 : "cannot call resize on empty vector if fillNA == false"; + for (int i = localData.length, j = 0; i < size; ++i, j = Utils.incMod(j, localData.length)) { + newData[i] = localData[j]; } } } diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RRawVector.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RRawVector.java index 49ef57251f..2edd0d69e9 100644 --- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RRawVector.java +++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RRawVector.java @@ -155,10 +155,12 @@ public final class RRawVector extends RVector<byte[]> implements RAbstractRawVec } private byte[] copyResizedData(int size, boolean fillNA) { - byte[] newData = Arrays.copyOf(getReadonlyData(), size); + byte[] localData = getReadonlyData(); + byte[] newData = Arrays.copyOf(localData, size); if (!fillNA) { + assert localData.length > 0 : "cannot call resize on empty vector if fillNA == false"; // NA is 00 for raw - for (int i = data.length, j = 0; i < size; ++i, j = Utils.incMod(j, data.length)) { + for (int i = localData.length, j = 0; i < size; ++i, j = Utils.incMod(j, localData.length)) { newData[i] = data[j]; } } diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RStringVector.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RStringVector.java index eeb2117671..c73b9f689e 100644 --- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RStringVector.java +++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RStringVector.java @@ -146,15 +146,17 @@ public final class RStringVector extends RVector<String[]> implements RAbstractS } private String[] copyResizedData(int size, String fill) { - String[] newData = Arrays.copyOf(data, size); - if (size > this.getLength()) { + String[] localData = getReadonlyData(); + String[] newData = Arrays.copyOf(localData, size); + if (size > localData.length) { if (fill != null) { - for (int i = data.length; i < size; i++) { + for (int i = localData.length; i < size; i++) { newData[i] = fill; } } else { - for (int i = data.length, j = 0; i < size; ++i, j = Utils.incMod(j, data.length)) { - newData[i] = data[j]; + assert localData.length > 0 : "cannot call resize on empty vector if fillNA == false"; + for (int i = localData.length, j = 0; i < size; ++i, j = Utils.incMod(j, localData.length)) { + newData[i] = localData[j]; } } } 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 8027948599..3b787d3e95 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 @@ -53121,6 +53121,39 @@ attr(,"useBytes") #rep(3, 4,) [1] 3 3 3 3 +##com.oracle.truffle.r.test.builtins.TestBuiltin_rep.testRep# +#rep(NULL) +NULL + +##com.oracle.truffle.r.test.builtins.TestBuiltin_rep.testRep# +#rep(character(), length.out=2) +[1] NA NA + +##com.oracle.truffle.r.test.builtins.TestBuiltin_rep.testRep# +#rep(complex(), length.out=2) +[1] NA NA + +##com.oracle.truffle.r.test.builtins.TestBuiltin_rep.testRep# +#rep(list(), length.out=2) +[[1]] +NULL + +[[2]] +NULL + + +##com.oracle.truffle.r.test.builtins.TestBuiltin_rep.testRep# +#rep(numeric(), length.out=2) +[1] NA NA + +##com.oracle.truffle.r.test.builtins.TestBuiltin_rep.testRep# +#rep(numeric(), times=3) +numeric(0) + +##com.oracle.truffle.r.test.builtins.TestBuiltin_rep.testRep# +#rep(raw(), length.out=2) +[1] 00 00 + ##com.oracle.truffle.r.test.builtins.TestBuiltin_rep.testRep# #rep(x<-42) [1] 42 diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_rep.java b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_rep.java index a6aaf4e375..028788e9f3 100644 --- a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_rep.java +++ b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_rep.java @@ -244,5 +244,15 @@ public class TestBuiltin_rep extends TestBase { // FIXME: should not print the warnings if empty args occur in '...' assertEval(Output.IgnoreWarningMessage, "rep(3, 4,)"); + + assertEval("rep(numeric(), length.out=2)"); + assertEval("rep(character(), length.out=2)"); + assertEval("rep(raw(), length.out=2)"); + assertEval("rep(complex(), length.out=2)"); + assertEval("rep(list(), length.out=2)"); + + assertEval("rep(numeric(), times=3)"); + + assertEval("rep(NULL)"); } } -- GitLab