From 2d2d25510e3f76412c6c940fefa252d32ae60775 Mon Sep 17 00:00:00 2001
From: stepan <stepan.sindelar@oracle.com>
Date: Tue, 20 Jun 2017 17:50:24 +0200
Subject: [PATCH] Fix: do not raise "missing value" error when reading variable
in inlined args
---
.../access/variables/ReadVariableNode.java | 13 +++++++---
.../r/nodes/function/ArgumentMatcher.java | 26 +++++++++++++++++--
.../truffle/r/test/ExpectedTestOutput.test | 4 +++
.../r/test/builtins/TestBuiltin_seq.java | 7 +++++
4 files changed, 45 insertions(+), 5 deletions(-)
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/variables/ReadVariableNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/variables/ReadVariableNode.java
index dc171181ab..24665dff03 100644
--- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/variables/ReadVariableNode.java
+++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/variables/ReadVariableNode.java
@@ -98,7 +98,10 @@ public final class ReadVariableNode extends RSourceSectionNode implements RSynta
// start the lookup in the enclosing frame
Super,
// whether a promise should be forced to check its type or not during lookup
- ForcedTypeCheck;
+ ForcedTypeCheck,
+ // whether reads of RMissing should not throw error and just proceed, this is the case for
+ // inlined varargs, which should not show missing value error
+ SilentMissing
}
public static ReadVariableNode create(String name) {
@@ -113,6 +116,10 @@ public final class ReadVariableNode extends RSourceSectionNode implements RSynta
return new ReadVariableNode(RSyntaxNode.SOURCE_UNAVAILABLE, name, mode, ReadKind.Silent);
}
+ public static ReadVariableNode createSilentMissing(String name, RType mode) {
+ return new ReadVariableNode(RSyntaxNode.SOURCE_UNAVAILABLE, name, mode, ReadKind.SilentMissing);
+ }
+
public static ReadVariableNode createSuperLookup(SourceSection src, String name) {
return new ReadVariableNode(src, name, RType.Any, ReadKind.Super);
}
@@ -853,7 +860,7 @@ public final class ReadVariableNode extends RSourceSectionNode implements RSynta
if ((isNullProfile == null && obj == null) || (isNullProfile != null && isNullProfile.profile(obj == null))) {
return false;
}
- if (obj == RMissing.instance) {
+ if (kind != ReadKind.SilentMissing && obj == RMissing.instance) {
unexpectedMissingProfile.enter();
throw RError.error(RError.SHOW_CALLER, RError.Message.ARGUMENT_MISSING, getIdentifier());
}
@@ -893,7 +900,7 @@ public final class ReadVariableNode extends RSourceSectionNode implements RSynta
if (obj == null) {
return false;
}
- if (obj == RMissing.instance) {
+ if (kind != ReadKind.SilentMissing && obj == RMissing.instance) {
throw RError.error(RError.SHOW_CALLER, RError.Message.ARGUMENT_MISSING, getIdentifier());
}
if (mode == RType.Any) {
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/ArgumentMatcher.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/ArgumentMatcher.java
index db25097a00..778adee698 100644
--- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/ArgumentMatcher.java
+++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/ArgumentMatcher.java
@@ -38,6 +38,7 @@ import com.oracle.truffle.api.nodes.Node;
import com.oracle.truffle.r.nodes.RASTUtils;
import com.oracle.truffle.r.nodes.RRootNode;
import com.oracle.truffle.r.nodes.access.ConstantNode;
+import com.oracle.truffle.r.nodes.access.variables.ReadVariableNode;
import com.oracle.truffle.r.nodes.function.PromiseNode.VarArgNode;
import com.oracle.truffle.r.runtime.Arguments;
import com.oracle.truffle.r.runtime.ArgumentsSignature;
@@ -359,6 +360,7 @@ public class ArgumentMatcher {
// Has varargs? Unfold!
if (suppliedIndex == MatchPermutation.VARARGS) {
int varArgsLen = match.varargsPermutation.length;
+ boolean shouldInline = shouldInlineArgument(builtin, formalIndex, fastPath);
String[] newNames = new String[varArgsLen];
RNode[] newVarArgs = new RNode[varArgsLen];
int index = 0;
@@ -375,7 +377,7 @@ public class ArgumentMatcher {
}
}
newNames[index] = match.varargsSignature.getName(i);
- newVarArgs[index] = varArg;
+ newVarArgs[index] = shouldInline ? updateInlinedArg(varArg) : varArg;
index++;
}
@@ -399,7 +401,7 @@ public class ArgumentMatcher {
}
ArgumentsSignature signature = ArgumentsSignature.get(newNames);
- if (shouldInlineArgument(builtin, formalIndex, fastPath)) {
+ if (shouldInline) {
resArgs[formalIndex] = PromiseNode.createVarArgsInlined(newVarArgs, signature);
} else {
boolean forcedEager = fastPath != null && fastPath.forcedEagerPromise(formalIndex);
@@ -447,6 +449,26 @@ public class ArgumentMatcher {
return builtin != null && builtin.evaluatesArg(formalIndex);
}
+ /**
+ * Reads of the {@link RMissing} values must not be reported as error in inlined varargs. This
+ * method updates any wrapped ReadVariableNode to just return missing values without raising an
+ * error.
+ *
+ * @see com.oracle.truffle.r.nodes.function.PromiseNode.InlineVarArgsNode
+ */
+ private static RNode updateInlinedArg(RNode node) {
+ if (!(node instanceof WrapArgumentNode)) {
+ return node;
+ }
+ WrapArgumentNode wrapper = (WrapArgumentNode) node;
+ if (!(wrapper.getOperand() instanceof ReadVariableNode)) {
+ return node;
+ }
+ ReadVariableNode rvn = (ReadVariableNode) wrapper.getOperand();
+ ReadVariableNode newRvn = ReadVariableNode.createSilentMissing(rvn.getIdentifier(), rvn.getMode());
+ return WrapArgumentNode.create(newRvn, wrapper.getIndex());
+ }
+
private static RNode wrapUnmatched(FormalArguments formals, RBuiltinDescriptor builtin, int formalIndex, boolean noOpt) {
if (builtin != null && !builtin.evaluatesArg(formalIndex) && formals.getDefaultArgument(formalIndex) != null) {
/*
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 4cfca94bd6..45e05ed3d3 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
@@ -61867,6 +61867,10 @@ integer(0)
#seq(integer())
integer(0)
+##com.oracle.truffle.r.test.builtins.TestBuiltin_seq.testSeqArgMatching#
+#{ foo <- function(beg, end, by, len) seq(beg, end, by, length.out = len); foo(beg=1, by=1, len=10) }
+ [1] 1 2 3 4 5 6 7 8 9 10
+
##com.oracle.truffle.r.test.builtins.TestBuiltin_seq.testSeqDispatch#
#{ d <- as.Date(1, origin = "1970-01-01"); seq(d, by=1, length.out=4) }
[1] "1970-01-02" "1970-01-03" "1970-01-04" "1970-01-05"
diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_seq.java b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_seq.java
index 493b127679..1475666adc 100644
--- a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_seq.java
+++ b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_seq.java
@@ -144,6 +144,13 @@ public class TestBuiltin_seq extends TestBase {
assertEval(Output.MayIgnoreErrorContext, template("%0(1, 20, 3, length.out=10)", SEQFUNS));
}
+ // argument matching corner-case appearing in seq (because it has a fast-path and varargs)
+ // taken and adapted from the scales package
+ @Test
+ public void testSeqArgMatching() {
+ assertEval("{ foo <- function(beg, end, by, len) seq(beg, end, by, length.out = len); foo(beg=1, by=1, len=10) }");
+ }
+
// Old tests, undoubtedly partially overlapping
@Test
--
GitLab