From 58cfe754e7e698abe867244c727968eba8c158d9 Mon Sep 17 00:00:00 2001
From: Adam Welc <adam.welc@oracle.com>
Date: Mon, 27 Jan 2014 16:41:26 -0800
Subject: [PATCH] Implemented support for mutating "value" argument of the
replacement form for builtins.
---
.../truffle/r/nodes/RTruffleVisitor.java | 18 ++++-
.../r/nodes/access/WriteVariableNode.java | 73 +++++++++++++------
.../r/nodes/builtin/base/UpdateDimNames.java | 2 +-
.../truffle/r/parser/ast/AccessVariable.java | 4 +
.../parser/ast/SimpleAccessTempVariable.java | 52 +++++++++++++
.../oracle/truffle/r/parser/ast/Visitor.java | 2 +
.../truffle/r/parser/tools/BasicVisitor.java | 5 ++
.../truffle/r/test/ExpectedTestOutput.test | 17 +++++
.../r/test/simple/TestSimpleArithmetic.java | 5 +-
9 files changed, 148 insertions(+), 30 deletions(-)
create mode 100644 com.oracle.truffle.r.parser/src/com/oracle/truffle/r/parser/ast/SimpleAccessTempVariable.java
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/RTruffleVisitor.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/RTruffleVisitor.java
index ea2ad49044..186e816088 100644
--- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/RTruffleVisitor.java
+++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/RTruffleVisitor.java
@@ -262,9 +262,14 @@ public final class RTruffleVisitor extends BasicVisitor<RNode> {
SimpleAccessVariable vAST = (SimpleAccessVariable) args.first().getValue();
String vSymbol = vAST.getSymbol().toString();
- // store a
- final String a = "*tmp_a*";
- WriteVariableNode aAssign = WriteVariableNode.create(a, rhs, false, false);
+ //@formatter:off
+ // store a - need to use temporary, otherwise there is a failure in case multiple calls to
+ // the replacement form are chained:
+ // x<-c(1); y<-c(1); dim(x)<-1; dim(y)<-1; attr(x, "dimnames")<-(attr(y, "dimnames")<-list("b"))
+ //@formatter:on
+
+ final Object a = new Object();
+ WriteVariableNode aAssign = WriteVariableNode.create(a, rhs, false, false, true);
// read v, assign tmp
ReadVariableNode v = n.isSuper() ? ReadSuperVariableNode.create(vAST.getSource(), vSymbol, context, false, false) : ReadVariableNode.create(vAST.getSource(), vSymbol, context, false, false);
@@ -279,7 +284,7 @@ public final class RTruffleVisitor extends BasicVisitor<RNode> {
rfArgs.add(args.getNode(i));
}
}
- rfArgs.add(AccessVariable.create(null, a, false));
+ rfArgs.add(AccessVariable.create(null, a));
// replacement function call (use visitor for FunctionCall)
FunctionCall rfCall = new FunctionCall(null, f.getName(), rfArgs);
@@ -300,6 +305,11 @@ public final class RTruffleVisitor extends BasicVisitor<RNode> {
return ReadVariableNode.create(n.getSource(), n.getSymbol(), context, false, n.shouldCopyValue());
}
+ @Override
+ public RNode visit(SimpleAccessTempVariable n) {
+ return ReadVariableNode.create(n.getSource(), n.getSymbol(), context, false, false);
+ }
+
@Override
public RNode visit(If n) {
RNode condition = n.getCond().accept(this);
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/WriteVariableNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/WriteVariableNode.java
index a102d2f90a..34708faa34 100644
--- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/WriteVariableNode.java
+++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/WriteVariableNode.java
@@ -47,7 +47,11 @@ public abstract class WriteVariableNode extends RNode {
@com.oracle.truffle.api.CompilerDirectives.CompilationFinal boolean everSeenTemporary;
@com.oracle.truffle.api.CompilerDirectives.CompilationFinal boolean everSeenSequence;
- protected void writeObjectValue(@SuppressWarnings("unused") VirtualFrame virtualFrame, Frame frame, FrameSlot frameSlot, Object value) {
+ // the toBeCopied parameter is meant to prevent creation of the shared/non-temp vector; this
+ // needed for the implementation of the replacement forms of builtin functions as their last
+ // argument can be mutaded; for example, in "dimnames(x)<-list(1)", the assigned value list(1)
+ // must become list("1"), with the latter value returned as a result of the call
+ protected void writeObjectValue(@SuppressWarnings("unused") VirtualFrame virtualFrame, Frame frame, FrameSlot frameSlot, Object value, boolean toBeCopied) {
Object newValue = value;
if (!isArgWrite()) {
Object frameValue;
@@ -72,21 +76,33 @@ public abstract class WriteVariableNode extends RNode {
CompilerDirectives.transferToInterpreterAndInvalidate();
everSeenTemporary = true;
}
- rVector.markNonTemporary();
+ if (toBeCopied) {
+ RVector vectorCopy = rVector.copy();
+ newValue = vectorCopy;
+ } else {
+ rVector.markNonTemporary();
+ }
} else if (rVector.isShared()) {
if (!everSeenShared) {
CompilerDirectives.transferToInterpreterAndInvalidate();
everSeenShared = true;
}
RVector vectorCopy = rVector.copy();
- vectorCopy.markNonTemporary();
+ if (!toBeCopied) {
+ vectorCopy.markNonTemporary();
+ }
newValue = vectorCopy;
} else {
if (!everSeenNonShared) {
CompilerDirectives.transferToInterpreterAndInvalidate();
everSeenNonShared = true;
}
- rVector.makeShared();
+ if (toBeCopied) {
+ RVector vectorCopy = rVector.copy();
+ newValue = vectorCopy;
+ } else {
+ rVector.makeShared();
+ }
}
}
}
@@ -95,28 +111,34 @@ public abstract class WriteVariableNode extends RNode {
frame.setObject(frameSlot, newValue);
}
- public static WriteVariableNode create(Object symbol, RNode rhs, boolean isArgWrite, boolean isSuper) {
+ public static WriteVariableNode create(Object symbol, RNode rhs, boolean isArgWrite, boolean isSuper, boolean toBeCopied) {
if (!isSuper) {
- return UnresolvedWriteLocalVariableNodeFactory.create(rhs, isArgWrite, symbol.toString());
+ return UnresolvedWriteLocalVariableNodeFactory.create(rhs, isArgWrite, symbol.toString(), toBeCopied);
} else {
assert !isArgWrite;
- return new UnresolvedWriteSuperVariableNode(rhs, symbol.toString());
+ return new UnresolvedWriteSuperVariableNode(rhs, symbol.toString(), toBeCopied);
}
}
+ public static WriteVariableNode create(Object symbol, RNode rhs, boolean isArgWrite, boolean isSuper) {
+ return create(symbol, rhs, isArgWrite, isSuper, false);
+ }
+
public static WriteVariableNode create(SourceSection src, Object symbol, RNode rhs, boolean isArgWrite, boolean isSuper) {
- WriteVariableNode wvn = create(symbol, rhs, isArgWrite, isSuper);
+ WriteVariableNode wvn = create(symbol, rhs, isArgWrite, isSuper, false);
wvn.assignSourceSection(src);
return wvn;
}
public abstract void execute(VirtualFrame frame, Object value);
- @NodeField(name = "symbol", type = Object.class)
+ @NodeFields({@NodeField(name = "symbol", type = Object.class), @NodeField(name = "toBeCopied", type = boolean.class)})
public abstract static class UnresolvedWriteLocalVariableNode extends WriteVariableNode {
public abstract Object getSymbol();
+ public abstract boolean isToBeCopied();
+
@Specialization
public byte doLogical(VirtualFrame frame, byte value) {
resolveAndSet(frame, value, FrameSlotKind.Byte);
@@ -150,15 +172,17 @@ public abstract class WriteVariableNode extends RNode {
private void resolveAndSet(VirtualFrame frame, Object value, FrameSlotKind initialKind) {
CompilerAsserts.neverPartOfCompilation();
FrameSlot frameSlot = frame.getFrameDescriptor().findOrAddFrameSlot(getSymbol(), initialKind);
- replace(ResolvedWriteLocalVariableNode.create(getRhs(), this.isArgWrite(), frameSlot)).execute(frame, value);
+ replace(ResolvedWriteLocalVariableNode.create(getRhs(), this.isArgWrite(), frameSlot, isToBeCopied())).execute(frame, value);
}
}
- @NodeField(name = "frameSlot", type = FrameSlot.class)
+ @NodeFields({@NodeField(name = "frameSlot", type = FrameSlot.class), @NodeField(name = "toBeCopied", type = boolean.class)})
public abstract static class ResolvedWriteLocalVariableNode extends WriteVariableNode {
- public static ResolvedWriteLocalVariableNode create(RNode rhs, boolean isArgWrite, FrameSlot frameSlot) {
- return ResolvedWriteLocalVariableNodeFactory.create(rhs, isArgWrite, frameSlot);
+ public abstract boolean isToBeCopied();
+
+ public static ResolvedWriteLocalVariableNode create(RNode rhs, boolean isArgWrite, FrameSlot frameSlot, boolean toBeCopied) {
+ return ResolvedWriteLocalVariableNodeFactory.create(rhs, isArgWrite, frameSlot, toBeCopied);
}
@Specialization(guards = "isBooleanKind")
@@ -181,13 +205,13 @@ public abstract class WriteVariableNode extends RNode {
@Specialization(guards = "isObjectKind")
public Object doObject(VirtualFrame frame, FrameSlot frameSlot, RInvisible value) {
- super.writeObjectValue(frame, frame, frameSlot, value.get());
+ super.writeObjectValue(frame, frame, frameSlot, value.get(), isToBeCopied());
return value;
}
@Specialization(guards = "isObjectKind")
public Object doObject(VirtualFrame frame, FrameSlot frameSlot, Object value) {
- super.writeObjectValue(frame, frame, frameSlot, value);
+ super.writeObjectValue(frame, frame, frameSlot, value, isToBeCopied());
return value;
}
}
@@ -246,10 +270,12 @@ public abstract class WriteVariableNode extends RNode {
@Child private RNode rhs;
private final Object symbol;
+ private final boolean toBeCopied;
- public UnresolvedWriteSuperVariableNode(RNode rhs, Object symbol) {
+ public UnresolvedWriteSuperVariableNode(RNode rhs, Object symbol, boolean toBeCopied) {
this.rhs = adoptChild(rhs);
this.symbol = symbol;
+ this.toBeCopied = toBeCopied;
}
@Override
@@ -266,10 +292,10 @@ public abstract class WriteVariableNode extends RNode {
// if this is the first node in the chain, needs the rhs and enclosingFrame nodes
AccessEnclosingFrameNode enclosingFrameNode = RArguments.get(frame).getEnclosingFrame() == enclosingFrame ? AccessEnclosingFrameNodeFactory.create(1) : null;
writeNode = WriteSuperVariableNodeFactory.create(getRhs(), enclosingFrameNode, new FrameSlotNode.PresentFrameSlotNode(enclosingFrame.getFrameDescriptor().findOrAddFrameSlot(symbol)),
- this.isArgWrite());
+ this.isArgWrite(), toBeCopied);
} else {
- WriteSuperVariableNode actualWriteNode = WriteSuperVariableNodeFactory.create(null, null, new FrameSlotNode.UnresolvedFrameSlotNode(symbol), this.isArgWrite());
- writeNode = new WriteSuperVariableConditionalNode(actualWriteNode, new UnresolvedWriteSuperVariableNode(null, symbol), getRhs());
+ WriteSuperVariableNode actualWriteNode = WriteSuperVariableNodeFactory.create(null, null, new FrameSlotNode.UnresolvedFrameSlotNode(symbol), this.isArgWrite(), toBeCopied);
+ writeNode = new WriteSuperVariableConditionalNode(actualWriteNode, new UnresolvedWriteSuperVariableNode(null, symbol, toBeCopied), getRhs());
}
replace(writeNode).execute(frame, value, enclosingFrame);
}
@@ -282,7 +308,7 @@ public abstract class WriteVariableNode extends RNode {
execute(frame, value, enclosingFrame);
} else {
// we're in global scope, do a local write instead
- replace(UnresolvedWriteLocalVariableNodeFactory.create(getRhs(), this.isArgWrite(), symbol.toString())).execute(frame, value);
+ replace(UnresolvedWriteLocalVariableNodeFactory.create(getRhs(), this.isArgWrite(), symbol.toString(), toBeCopied)).execute(frame, value);
}
}
@@ -294,10 +320,13 @@ public abstract class WriteVariableNode extends RNode {
@SuppressWarnings("unused")
@NodeChildren({@NodeChild(value = "enclosingFrame", type = AccessEnclosingFrameNode.class), @NodeChild(value = "frameSlotNode", type = FrameSlotNode.class)})
+ @NodeField(name = "toBeCopied", type = boolean.class)
public abstract static class WriteSuperVariableNode extends AbstractWriteSuperVariableNode {
protected abstract FrameSlotNode getFrameSlotNode();
+ public abstract boolean isToBeCopied();
+
@Specialization(guards = "isBooleanKind")
public byte doBoolean(VirtualFrame frame, byte value, MaterializedFrame enclosingFrame, FrameSlot frameSlot) {
enclosingFrame.setByte(frameSlot, value);
@@ -318,13 +347,13 @@ public abstract class WriteVariableNode extends RNode {
@Specialization(guards = "isObjectKind")
public Object doObject(VirtualFrame frame, RInvisible value, MaterializedFrame enclosingFrame, FrameSlot frameSlot) {
- super.writeObjectValue(frame, enclosingFrame, frameSlot, value.get());
+ super.writeObjectValue(frame, enclosingFrame, frameSlot, value.get(), isToBeCopied());
return value;
}
@Specialization(guards = "isObjectKind")
public Object doObject(VirtualFrame frame, Object value, MaterializedFrame enclosingFrame, FrameSlot frameSlot) {
- super.writeObjectValue(frame, enclosingFrame, frameSlot, value);
+ super.writeObjectValue(frame, enclosingFrame, frameSlot, value, isToBeCopied());
return value;
}
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/builtin/base/UpdateDimNames.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/builtin/base/UpdateDimNames.java
index e58d4689d0..34a61f1d00 100644
--- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/builtin/base/UpdateDimNames.java
+++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/builtin/base/UpdateDimNames.java
@@ -78,7 +78,7 @@ public abstract class UpdateDimNames extends RBuiltinNode {
@Specialization(order = 3, guards = "!isZeroLength")
public RAbstractVector updateDimnames(VirtualFrame frame, RAbstractVector vector, RList list) {
RVector v = vector.materialize();
- v.setDimNames(list, this.getSourceSection());
+ v.setDimNames(convertToListOfStrings(frame, list), this.getSourceSection());
return v;
}
diff --git a/com.oracle.truffle.r.parser/src/com/oracle/truffle/r/parser/ast/AccessVariable.java b/com.oracle.truffle.r.parser/src/com/oracle/truffle/r/parser/ast/AccessVariable.java
index c3faf91f7c..d73a6107ff 100644
--- a/com.oracle.truffle.r.parser/src/com/oracle/truffle/r/parser/ast/AccessVariable.java
+++ b/com.oracle.truffle.r.parser/src/com/oracle/truffle/r/parser/ast/AccessVariable.java
@@ -17,4 +17,8 @@ public abstract class AccessVariable extends ASTNode {
public static ASTNode create(SourceSection src, String name, boolean shouldCopyValue) {
return new SimpleAccessVariable(src, Symbol.getSymbol(name), shouldCopyValue);
}
+
+ public static ASTNode create(SourceSection src, Object tempSymbol) {
+ return new SimpleAccessTempVariable(src, tempSymbol);
+ }
}
diff --git a/com.oracle.truffle.r.parser/src/com/oracle/truffle/r/parser/ast/SimpleAccessTempVariable.java b/com.oracle.truffle.r.parser/src/com/oracle/truffle/r/parser/ast/SimpleAccessTempVariable.java
new file mode 100644
index 0000000000..42cfa94bed
--- /dev/null
+++ b/com.oracle.truffle.r.parser/src/com/oracle/truffle/r/parser/ast/SimpleAccessTempVariable.java
@@ -0,0 +1,52 @@
+/*
+ * Copyright (c) 2014, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package com.oracle.truffle.r.parser.ast;
+
+import java.util.*;
+
+import com.oracle.truffle.api.*;
+
+@Precedence(Precedence.MAX)
+public class SimpleAccessTempVariable extends AccessVariable {
+
+ Object tempSymbol;
+
+ public SimpleAccessTempVariable(SourceSection source, Object tempSymbol) {
+ this.source = source;
+ this.tempSymbol = tempSymbol;
+ }
+
+ public Object getSymbol() {
+ return tempSymbol;
+ }
+
+ @Override
+ public <R> R accept(Visitor<R> v) {
+ return v.visit(this);
+ }
+
+ @Override
+ public <R> List<R> visitAll(Visitor<R> v) {
+ return Collections.emptyList();
+ }
+}
diff --git a/com.oracle.truffle.r.parser/src/com/oracle/truffle/r/parser/ast/Visitor.java b/com.oracle.truffle.r.parser/src/com/oracle/truffle/r/parser/ast/Visitor.java
index d370043d96..d719605561 100644
--- a/com.oracle.truffle.r.parser/src/com/oracle/truffle/r/parser/ast/Visitor.java
+++ b/com.oracle.truffle.r.parser/src/com/oracle/truffle/r/parser/ast/Visitor.java
@@ -76,6 +76,8 @@ public interface Visitor<R> {
R visit(SimpleAccessVariable readVariable);
+ R visit(SimpleAccessTempVariable readVariable);
+
R visit(FieldAccess fieldAccess);
R visit(SimpleAssignVariable assign);
diff --git a/com.oracle.truffle.r.parser/src/com/oracle/truffle/r/parser/tools/BasicVisitor.java b/com.oracle.truffle.r.parser/src/com/oracle/truffle/r/parser/tools/BasicVisitor.java
index 9638a45f56..36299ca596 100644
--- a/com.oracle.truffle.r.parser/src/com/oracle/truffle/r/parser/tools/BasicVisitor.java
+++ b/com.oracle.truffle.r.parser/src/com/oracle/truffle/r/parser/tools/BasicVisitor.java
@@ -199,6 +199,11 @@ public class BasicVisitor<R> implements Visitor<R> {
return visit((ASTNode) n);
}
+ @Override
+ public R visit(SimpleAccessTempVariable n) {
+ return visit((ASTNode) n);
+ }
+
@Override
public R visit(FieldAccess n) {
return visit((ASTNode) n);
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 20a60666f0..0b4ebbdd37 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
@@ -1047,6 +1047,23 @@ $dimnames[[2]]
+##com.oracle.truffle.r.test.simple.TestSimpleArithmetic.testUnaryNotPropagate
+#{ x<-1:4; dim(x)<-c(2, 2); names(x)<-101:104; attr(x, "dimnames")<-list(201:202, 203:204); attr(x, "foo")<-"foo"; y<-!x; attributes(y) }
+$names
+[1] "101" "102" "103" "104"
+
+$dim
+[1] 2 2
+
+$dimnames
+$dimnames[[1]]
+[1] "201" "202"
+
+$dimnames[[2]]
+[1] "203" "204"
+
+
+
##com.oracle.truffle.r.test.simple.TestSimpleArithmetic.testUnaryNotRaw
#{ a <- as.raw(12) ; !a }
[1] f3
diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/simple/TestSimpleArithmetic.java b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/simple/TestSimpleArithmetic.java
index 032629c989..7861b1c623 100644
--- a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/simple/TestSimpleArithmetic.java
+++ b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/simple/TestSimpleArithmetic.java
@@ -388,10 +388,9 @@ public class TestSimpleArithmetic extends TestBase {
}
@Test
- @Ignore
- public void testUnaryNotPropagateIgnore() {
+ public void testUnaryNotPropagate() {
// list of sequences should be converted to list of string vectors
- assertEvalError("{ x<-1:4; dim(x)<-c(2, 2); names(x)<-101:104; attr(x, \"dimnames\")<-list(201:202, 203:204); attr(x, \"foo\")<-\"foo\"; y<-!x; attributes(y) }");
+ assertEval("{ x<-1:4; dim(x)<-c(2, 2); names(x)<-101:104; attr(x, \"dimnames\")<-list(201:202, 203:204); attr(x, \"foo\")<-\"foo\"; y<-!x; attributes(y) }");
}
@Test
--
GitLab