From 97e9692619e58e8eee407ed0fea5ef31753d0369 Mon Sep 17 00:00:00 2001
From: Miloslav Metelka <miloslav.metelka@oracle.com>
Date: Tue, 8 Aug 2017 00:04:08 +0200
Subject: [PATCH] Updated fix to handle NAs with na.rm=TRUE properly (returns
Object from doIntVector() in reducer.
---
.../unary/UnaryArithmeticReduceNode.java | 41 ++++++-------------
.../truffle/r/test/ExpectedTestOutput.test | 12 +++---
.../r/test/builtins/TestBuiltin_max.java | 6 +--
.../r/test/builtins/TestBuiltin_min.java | 14 ++-----
4 files changed, 24 insertions(+), 49 deletions(-)
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/unary/UnaryArithmeticReduceNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/unary/UnaryArithmeticReduceNode.java
index 0a6de6adf1..369ea71c71 100644
--- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/unary/UnaryArithmeticReduceNode.java
+++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/unary/UnaryArithmeticReduceNode.java
@@ -72,7 +72,6 @@ public abstract class UnaryArithmeticReduceNode extends RBaseNode {
protected final ReduceSemantics semantics;
protected final boolean supportString;
protected final boolean supportComplex;
- protected final boolean useDoubleStartForEmptyVector;
private final NACheck na = NACheck.create();
private final ConditionProfile naRmProfile = ConditionProfile.createBinaryProfile();
@@ -83,7 +82,6 @@ public abstract class UnaryArithmeticReduceNode extends RBaseNode {
this.arithmetic = factory.createOperation();
this.supportString = semantics.supportString;
this.supportComplex = semantics.supportComplex;
- this.useDoubleStartForEmptyVector = semantics.useDoubleStartForEmptyVector;
}
private String handleString(RStringVector operand, boolean naRm, boolean finite, int offset) {
@@ -202,16 +200,8 @@ public abstract class UnaryArithmeticReduceNode extends RBaseNode {
}
}
- @Specialization(guards = {"useDoubleStartForEmptyVector", "operand.getLength() == 0"})
- protected double doIntVectorEmpty(@SuppressWarnings("unused") RIntVector operand,
- @SuppressWarnings("unused") boolean naRm,
- @SuppressWarnings("unused") boolean finite) {
- emptyWarning();
- return semantics.getDoubleStart();
- }
-
@Specialization
- protected int doIntVector(RIntVector operand, boolean naRm, boolean finite) {
+ protected Object doIntVector(RIntVector operand, boolean naRm, boolean finite) {
RBaseNode.reportWork(this, operand.getLength());
boolean profiledNaRm = naRmProfile.profile(naRm || finite);
int result = semantics.getIntStart();
@@ -237,6 +227,9 @@ public abstract class UnaryArithmeticReduceNode extends RBaseNode {
}
if (opCount == 0) {
emptyWarning();
+ if (semantics.isUseDoubleStartForEmptyVector()) {
+ return semantics.getDoubleStart();
+ }
}
return result;
}
@@ -277,16 +270,8 @@ public abstract class UnaryArithmeticReduceNode extends RBaseNode {
return result;
}
- @Specialization(guards = {"useDoubleStartForEmptyVector", "operand.getLength() == 0"})
- protected double doLogicalVectorEmpty(@SuppressWarnings("unused") RLogicalVector operand,
- @SuppressWarnings("unused") boolean naRm,
- @SuppressWarnings("unused") boolean finite) {
- emptyWarning();
- return semantics.getDoubleStart();
- }
-
@Specialization
- protected int doLogicalVector(RLogicalVector operand, boolean naRm, @SuppressWarnings("unused") boolean finite) {
+ protected Object doLogicalVector(RLogicalVector operand, boolean naRm, @SuppressWarnings("unused") boolean finite) {
RBaseNode.reportWork(this, operand.getLength());
boolean profiledNaRm = naRmProfile.profile(naRm);
int result = semantics.getIntStart();
@@ -312,20 +297,15 @@ public abstract class UnaryArithmeticReduceNode extends RBaseNode {
}
if (opCount == 0) {
emptyWarning();
+ if (semantics.isUseDoubleStartForEmptyVector()) {
+ return semantics.getDoubleStart();
+ }
}
return result;
}
- @Specialization(guards = {"useDoubleStartForEmptyVector", "operand.getLength() == 0"})
- protected double doIntSequenceEmpty(@SuppressWarnings("unused") RIntSequence operand,
- @SuppressWarnings("unused") boolean naRm,
- @SuppressWarnings("unused") boolean finite) {
- emptyWarning();
- return semantics.getDoubleStart();
- }
-
@Specialization
- protected int doIntSequence(RIntSequence operand, @SuppressWarnings("unused") boolean naRm, @SuppressWarnings("unused") boolean finite) {
+ protected Object doIntSequence(RIntSequence operand, @SuppressWarnings("unused") boolean naRm, @SuppressWarnings("unused") boolean finite) {
RBaseNode.reportWork(this, operand.getLength());
int result = semantics.getIntStart();
int current = operand.getStart();
@@ -339,6 +319,9 @@ public abstract class UnaryArithmeticReduceNode extends RBaseNode {
}
if (operand.getLength() == 0) {
emptyWarning();
+ if (semantics.isUseDoubleStartForEmptyVector()) {
+ return semantics.getDoubleStart();
+ }
}
return result;
}
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 d3334b70c5..fa3bc6d5fe 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
@@ -36131,7 +36131,7 @@ In max(as.double(NA), na.rm = TRUE) :
#{ max(as.integer(NA), as.integer(NA), na.rm=FALSE) }
[1] NA
-##com.oracle.truffle.r.test.builtins.TestBuiltin_max.testMaximum#Ignored.ImplementationError#
+##com.oracle.truffle.r.test.builtins.TestBuiltin_max.testMaximum#
#{ max(as.integer(NA), as.integer(NA), na.rm=TRUE) }
[1] -Inf
Warning message:
@@ -36142,7 +36142,7 @@ In max(as.integer(NA), as.integer(NA), na.rm = TRUE) :
#{ max(as.integer(NA), na.rm=FALSE) }
[1] NA
-##com.oracle.truffle.r.test.builtins.TestBuiltin_max.testMaximum#Ignored.ImplementationError#
+##com.oracle.truffle.r.test.builtins.TestBuiltin_max.testMaximum#
#{ max(as.integer(NA), na.rm=TRUE) }
[1] -Inf
Warning message:
@@ -36849,7 +36849,7 @@ In min(as.double(NA), na.rm = TRUE) :
#{ min(as.integer(NA), as.integer(NA), na.rm=FALSE) }
[1] NA
-##com.oracle.truffle.r.test.builtins.TestBuiltin_min.testMinimum#Ignored.ImplementationError#Output.IgnoreWarningContext#
+##com.oracle.truffle.r.test.builtins.TestBuiltin_min.testMinimum#
#{ min(as.integer(NA), as.integer(NA), na.rm=TRUE) }
[1] Inf
Warning message:
@@ -36860,7 +36860,7 @@ In min(as.integer(NA), as.integer(NA), na.rm = TRUE) :
#{ min(as.integer(NA), na.rm=FALSE) }
[1] NA
-##com.oracle.truffle.r.test.builtins.TestBuiltin_min.testMinimum#Ignored.ImplementationError#Output.IgnoreWarningContext#
+##com.oracle.truffle.r.test.builtins.TestBuiltin_min.testMinimum#
#{ min(as.integer(NA), na.rm=TRUE) }
[1] Inf
Warning message:
@@ -36907,13 +36907,13 @@ In min(double()) : no non-missing arguments to min; returning Inf
Warning message:
In min(double(0)) : no non-missing arguments to min; returning Inf
-##com.oracle.truffle.r.test.builtins.TestBuiltin_min.testMinimum#Ignored.ImplementationError#Output.IgnoreWarningContext#
+##com.oracle.truffle.r.test.builtins.TestBuiltin_min.testMinimum#
#{ min(integer()) }
[1] Inf
Warning message:
In min(integer()) : no non-missing arguments to min; returning Inf
-##com.oracle.truffle.r.test.builtins.TestBuiltin_min.testMinimum#Ignored.ImplementationError#Output.IgnoreWarningContext#
+##com.oracle.truffle.r.test.builtins.TestBuiltin_min.testMinimum#
#{ min(integer(0)) }
[1] Inf
Warning message:
diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_max.java b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_max.java
index a06988258f..7278f8e037 100644
--- a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_max.java
+++ b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_max.java
@@ -217,10 +217,8 @@ public class TestBuiltin_max extends TestBase {
assertEval("{ max(integer(0)) }");
assertEval("{ max(integer()) }");
assertEval("{ max(as.double(NA), na.rm=TRUE) }");
- // FIXME -2147483647 returned instead of -Inf
- assertEval(Ignored.ImplementationError, "{ max(as.integer(NA), na.rm=TRUE) }");
- // FIXME -2147483647 returned instead of -Inf
- assertEval(Ignored.ImplementationError, "{ max(as.integer(NA), as.integer(NA), na.rm=TRUE) }");
+ assertEval("{ max(as.integer(NA), na.rm=TRUE) }");
+ assertEval("{ max(as.integer(NA), as.integer(NA), na.rm=TRUE) }");
assertEval("{ max(logical(0)) }");
assertEval("{ max(seq_len(0)) }");
diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_min.java b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_min.java
index 442c0c3a6c..5db4be652d 100644
--- a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_min.java
+++ b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_min.java
@@ -173,17 +173,11 @@ public class TestBuiltin_min extends TestBase {
assertEval("{ min(\"42\", as.character(NA), \"7\", na.rm=TRUE) }");
assertEval("{ min(\"42\", as.character(NA), \"7\", na.rm=FALSE) }");
- // FIXME
- // Expected output: [1] Inf
- // FastR output: [1] 2147483647
- assertEval(Ignored.ImplementationError, Output.IgnoreWarningContext, "{ min(integer(0)) }");
- assertEval(Ignored.ImplementationError, Output.IgnoreWarningContext, "{ min(integer()) }");
+ assertEval("{ min(integer(0)) }");
+ assertEval("{ min(integer()) }");
assertEval("{ min(as.double(NA), na.rm=TRUE) }");
- // FIXME
- // Expected output: [1] Inf
- // FastR output: [1] 2147483647
- assertEval(Ignored.ImplementationError, Output.IgnoreWarningContext, "{ min(as.integer(NA), na.rm=TRUE) }");
- assertEval(Ignored.ImplementationError, Output.IgnoreWarningContext, "{ min(as.integer(NA), as.integer(NA), na.rm=TRUE) }");
+ assertEval("{ min(as.integer(NA), na.rm=TRUE) }");
+ assertEval("{ min(as.integer(NA), as.integer(NA), na.rm=TRUE) }");
assertEval("min(c(1,NA,2), na.rm=NA)");
}
--
GitLab