From c6654fd09f86c598f4702f212eb3dfa6278d6f76 Mon Sep 17 00:00:00 2001
From: Mick Jordan <mick.jordan@oracle.com>
Date: Thu, 4 Jun 2015 13:54:37 -0700
Subject: [PATCH] revert default values of RPairList to RNull
---
.../truffle/r/nodes/builtin/base/Formals.java | 2 +-
.../com/oracle/truffle/r/nodes/RASTUtils.java | 2 +-
.../function/FunctionDefinitionNode.java | 2 +-
.../oracle/truffle/r/runtime/RSerialize.java | 53 +++++++++++++------
.../truffle/r/runtime/data/RDataFactory.java | 6 +--
.../truffle/r/runtime/data/RPairList.java | 25 +++++----
6 files changed, 59 insertions(+), 31 deletions(-)
diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Formals.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Formals.java
index 08ae0217b4..3d17b32136 100644
--- a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Formals.java
+++ b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Formals.java
@@ -44,7 +44,7 @@ public abstract class Formals extends RBuiltinNode {
return RNull.instance;
}
FormalArguments formalArgs = fdNode.getFormalArguments();
- Object succ = RUnboundValue.instance;
+ Object succ = RNull.instance;
for (int i = formalArgs.getSignature().getLength() - 1; i >= 0; i--) {
RNode def = formalArgs.getDefaultArgument(i);
Object defValue = def == null ? RMissing.instance : RDataFactory.createLanguage(def);
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/RASTUtils.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/RASTUtils.java
index 4817246a10..de05a6dfaf 100644
--- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/RASTUtils.java
+++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/RASTUtils.java
@@ -110,7 +110,7 @@ public class RASTUtils {
RPairList prev = null;
RPairList result = null;
for (int i = 0; i < closures.length; i++) {
- RPairList pl = RDataFactory.createPairList(createLanguageElement(unwrap(closures[i].getExpr())), RUnboundValue.instance, RUnboundValue.toRUnboundValue((signature.getName(i))));
+ RPairList pl = RDataFactory.createPairList(createLanguageElement(unwrap(closures[i].getExpr())), RNull.instance, RNull.toRNull((signature.getName(i))));
if (prev != null) {
prev.setCdr(pl);
} else {
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/FunctionDefinitionNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/FunctionDefinitionNode.java
index 1878a67a6c..4b7a53f5a8 100644
--- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/FunctionDefinitionNode.java
+++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/FunctionDefinitionNode.java
@@ -359,7 +359,7 @@ public final class FunctionDefinitionNode extends RRootNode implements RSyntaxNo
*/
public static void checkCloseBrace(RSerialize.State state, boolean hasBraces) {
if (hasBraces) {
- if (state.isNullCdr()) {
+ if (state.isNull()) {
// special case of empty body "{ }"
state.setNull();
} else {
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/RSerialize.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/RSerialize.java
index af240e6a48..91ec85c636 100644
--- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/RSerialize.java
+++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/RSerialize.java
@@ -1356,6 +1356,7 @@ public class RSerialize {
}
RPairList pl = (RPairList) RContext.getRRuntimeASTAccess().serialize(state, fun);
if (pl != null) {
+ state.convertUnboundValues(pl);
if (FastROptions.debugMatches("printWclosure")) {
Debug.printClosure(pl);
}
@@ -1570,10 +1571,10 @@ public class RSerialize {
public abstract void setNull();
/**
- * Checks for the special case where the active pairlist only has a {@link RNull}
- * {@code cdr}.
+ * Checks for the special case where the active pairlist has a {@link RNull} {@code car} and
+ * {@code cdr} and an unset {@code tag}.
*/
- public abstract boolean isNullCdr();
+ public abstract boolean isNull();
/**
* Closes the current pairlist, handling the case where a "simple" value is down-shifted
@@ -1609,6 +1610,11 @@ public class RSerialize {
*/
public abstract void switchCdrToCar();
+ /**
+ * Clean up any {@link RUnboundValue}s from shrink optimization.
+ */
+ public abstract void convertUnboundValues(RPairList pl);
+
// Implementation independent convenience methods
/**
@@ -1663,7 +1669,11 @@ public class RSerialize {
@Override
public State openPairList() {
- RPairList result = RDataFactory.createPairList();
+ /*
+ * In order to implement the "shrink" optimization in closePairList we set the tag and
+ * the cdr to RUnboundValue. N.B. It is a bug if this ever escapes to the outside world.
+ */
+ RPairList result = RDataFactory.createPairList(RNull.instance, RUnboundValue.instance, RUnboundValue.instance);
active.addFirst(result);
return this;
}
@@ -1731,16 +1741,11 @@ public class RSerialize {
if (top.cdr() == RUnboundValue.instance) {
if (top.getTag() == RUnboundValue.instance && top.getType() == null) {
// shrink back to non-pairlist (cf GnuR)
- assert top.car() != RUnboundValue.instance;
return top.car();
} else {
top.setCdr(RNull.instance);
return top;
}
- } else if (top.car() == RUnboundValue.instance) {
- assert false;
- assert top.getTag() == RUnboundValue.instance && top.getType() == null && top.cdr() != RUnboundValue.instance;
- return top.cdr();
} else {
return top;
}
@@ -1759,23 +1764,23 @@ public class RSerialize {
}
@Override
- public boolean isNullCdr() {
+ public boolean isNull() {
RPairList pl = active.peekFirst();
- return pl.getTag() == RUnboundValue.instance && pl.car() == RUnboundValue.instance && pl.cdr() == RNull.instance;
+ return pl.getTag() == RUnboundValue.instance && pl.car() == RNull.instance && pl.cdr() == RNull.instance;
}
@Override
public void switchCdrToCar() {
RPairList pl = active.removeFirst();
- assert pl.cdr() != RUnboundValue.instance && pl.car() == RUnboundValue.instance;
// setting the type prevents the usual value down-shift on close
- SEXPTYPE type;
+ RPairList spl;
if (pl.cdr() instanceof RPairList && ((RPairList) pl.cdr()).getType() == null) {
- type = null;
+ // preserve the "shrink" optimization
+ spl = RDataFactory.createPairList(pl.cdr(), RUnboundValue.instance, RUnboundValue.instance);
} else {
- type = SEXPTYPE.LISTSXP;
+ spl = RDataFactory.createPairList(pl.cdr(), RNull.instance, RNull.instance, SEXPTYPE.LISTSXP);
}
- active.addFirst(RDataFactory.createPairList(pl.cdr(), RUnboundValue.instance, RUnboundValue.instance, type));
+ active.addFirst(spl);
}
@Override
@@ -1811,6 +1816,22 @@ public class RSerialize {
return positionsLength[px];
}
+ @Override
+ public void convertUnboundValues(RPairList pl) {
+ Object obj = pl;
+ while (obj instanceof RPairList) {
+ RPairList plt = (RPairList) obj;
+ if (plt.getTag() == RUnboundValue.instance) {
+ plt.setTag(RNull.instance);
+ }
+ if (plt.car() instanceof RPairList) {
+ convertUnboundValues((RPairList) plt.car());
+ }
+ obj = plt.cdr();
+ assert !(obj instanceof RUnboundValue);
+ }
+ }
+
}
/**
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RDataFactory.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RDataFactory.java
index 3bb117de2a..a3d612a632 100644
--- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RDataFactory.java
+++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RDataFactory.java
@@ -373,15 +373,15 @@ public final class RDataFactory {
}
public static RPairList createPairList() {
- return traceDataCreated(new RPairList(RUnboundValue.instance, RUnboundValue.instance, RUnboundValue.instance, null));
+ return traceDataCreated(new RPairList());
}
public static RPairList createPairList(Object car) {
- return traceDataCreated(new RPairList(car, RUnboundValue.instance, RUnboundValue.instance, null));
+ return traceDataCreated(new RPairList(car, RNull.instance, RNull.instance, null));
}
public static RPairList createPairList(Object car, Object cdr) {
- return traceDataCreated(new RPairList(car, cdr, RUnboundValue.instance, null));
+ return traceDataCreated(new RPairList(car, cdr, RNull.instance, null));
}
public static RPairList createPairList(Object car, Object cdr, Object tag) {
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RPairList.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RPairList.java
index fad9d6182e..3dc7b47229 100644
--- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RPairList.java
+++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RPairList.java
@@ -33,15 +33,25 @@ import com.oracle.truffle.r.runtime.gnur.*;
* {@code null} is never allowed as a value for the tag, car or cdr, only the type.
*/
public class RPairList extends RAttributeStorage implements RAttributable, RAbstractContainer {
- private Object car = RUnboundValue.instance;
- private Object cdr = RUnboundValue.instance;
- private Object tag = RUnboundValue.instance;
+ private Object car = RNull.instance;
+ private Object cdr = RNull.instance;
+ /**
+ * Externally, i.e., when serialized this is either a SYMSXP ({@link RSymbol}) or {@link RNull}.
+ * Internally it may take on other, non-null, values.
+ */
+ private Object tag = RNull.instance;
/**
- * Denotes the (GnuR) typeof entity that the pairlist represents.
+ * Denotes the (GnuR) type of entity that the pairlist represents. (Internal use only).
*/
private SEXPTYPE type;
+ /**
+ * Uninitialized pairlist.
+ */
+ RPairList() {
+ }
+
/**
* Variant used in unserialization to record the GnuR type the pairlist denotes.
*/
@@ -78,13 +88,11 @@ public class RPairList extends RAttributeStorage implements RAttributable, RAbst
}
public void setCar(Object newCar) {
- assert car == RUnboundValue.instance;
assert newCar != null;
car = newCar;
}
public void setCdr(Object newCdr) {
- assert cdr == RUnboundValue.instance;
assert newCdr != null;
cdr = newCdr;
}
@@ -109,7 +117,6 @@ public class RPairList extends RAttributeStorage implements RAttributable, RAbst
}
public void setTag(Object newTag) {
- assert tag == RUnboundValue.instance;
assert newTag != null;
this.tag = newTag;
}
@@ -120,7 +127,7 @@ public class RPairList extends RAttributeStorage implements RAttributable, RAbst
}
public boolean isNullTag() {
- return tag == RUnboundValue.instance || tag == RNull.instance;
+ return tag == RNull.instance;
}
public SEXPTYPE getType() {
@@ -189,7 +196,7 @@ public class RPairList extends RAttributeStorage implements RAttributable, RAbst
}
public static boolean isNull(Object obj) {
- return obj == RNull.instance || obj == RUnboundValue.instance;
+ return obj == RNull.instance;
}
@Override
--
GitLab