diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/EnvFunctions.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/EnvFunctions.java index ff40d89ab95f1d38c380aeb010f134ccc24f91fe..376077d5c0f8d86d15f92394598e8aef36238620 100644 --- a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/EnvFunctions.java +++ b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/EnvFunctions.java @@ -193,7 +193,7 @@ public class EnvFunctions { RFunction func = (RFunction) funcArg; Frame enclosing = func.getEnclosingFrame(); REnvironment env = RArguments.getEnvironment(enclosing); - return env == null ? REnvironment.lexicalChain(enclosing.materialize()) : env; + return env == null ? REnvironment.createEnclosingEnvironments(enclosing.materialize()) : env; } else { // Not an error according to GnuR return RNull.instance; diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Internal.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Internal.java index 92b4b5b3547a2fcbc29364517b6a0ab81ea07dad..0a3d1566b86951a448475043413eb37790177e40 100644 --- a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Internal.java +++ b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Internal.java @@ -74,9 +74,6 @@ public abstract class Internal extends RBuiltinNode { // .Internal function is validated CompilerDirectives.transferToInterpreterAndInvalidate(); // Replace the original call; we can't just use callNode as that will cause recursion! - if (symbol.getName().equals("paste")) { - System.console(); - } RCallNode internalCallNode = RCallNode.createInternalCall(frame, this.getParent().getSourceSection(), callNode, function, symbol); this.getParent().replace(internalCallNode); // evaluate the actual builtin this time, next time we won't get here! diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Substitute.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Substitute.java index 3fa9343058223961b787e05f27cdccbd44070c46..4eb5e2dad050ba6c4e466af827bc2f7f1b3c11df 100644 --- a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Substitute.java +++ b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Substitute.java @@ -57,8 +57,7 @@ public abstract class Substitute extends RBuiltinNode { protected Object doSubstitute(VirtualFrame frame, RPromise expr, @SuppressWarnings("unused") RMissing envMissing) { controlVisibility(); // In the global environment, substitute behaves like quote - REnvironment env = REnvironment.checkNonFunctionFrame(frame); - if (env == REnvironment.globalEnv()) { + if (REnvironment.isGlobalEnvFrame(frame)) { return checkQuote().execute(frame, expr); } // We have to examine all the names in the expression: @@ -80,9 +79,7 @@ public abstract class Substitute extends RBuiltinNode { // N.B. In many cases it is ok to just return the value, which is what we do to // allow progress on package loading } - if (env == null) { - env = REnvironment.frameToEnvironment(frame.materialize()); - } + REnvironment env = REnvironment.frameToEnvironment(frame.materialize()); Object val = env.get(name); if (val == null) { // not bound in env diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/REnvironment.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/REnvironment.java index 0c009488ffdd147d7e34dfab63136c0da366d4d8..3a51d9ef2c1d5672c21b3cfdd15be3d6cb5b0404 100644 --- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/REnvironment.java +++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/REnvironment.java @@ -72,6 +72,10 @@ import com.oracle.truffle.r.runtime.envframe.*; * "imports" environment. The parent of "package:base" is the empty environment, but the parent of * "namespace:base" is the global environment. * + * Whereas R types generally use value semantics environments do not; they have reference semantics. + * In particular in FastR, there is exactly one environment created for any package or function + * Truffle frame, allowing equality to be tested using {@code ==}. + * * TODO retire the {@code Package}, {@code Namespace} and {@code Imports} classes as they are only * used by the builtin packages, and will be completely redundant when they are loaded from * serialized package meta-data as will happen in due course. @@ -144,19 +148,14 @@ public abstract class REnvironment implements RAttributable { * Returns {@code true} iff {@code frame} is that associated with {@code env}. */ public static boolean isFrameForEnv(Frame frame, REnvironment env) { - Object id = env.frameAccess.id(); - if (id == null) { - return false; - } - FrameSlot idSlot = frame.getFrameDescriptor().findFrameSlot(id); - if (idSlot == null) { - return false; + REnvironment frameEnv = RArguments.getEnvironment(frame); + if (frameEnv == env) { + return true; } - try { - return frame.getObject(idSlot) == id; - } catch (FrameSlotTypeException fste) { - return false; + if (frameEnv == null) { + frameEnv = createEnclosingEnvironments(frame.materialize()); } + return frameEnv == env; } /** @@ -182,7 +181,7 @@ public abstract class REnvironment implements RAttributable { /** * Check whether the given frame is indeed the frame stored in the global environment. */ - public static boolean isGlobalEnvFrame(MaterializedFrame frame) { + public static boolean isGlobalEnvFrame(Frame frame) { return isFrameForEnv(frame, globalEnv); } @@ -426,32 +425,15 @@ public abstract class REnvironment implements RAttributable { return result; } - /** - * Check if a frame corresponds to a function. If there is no function associated with the - * frame, then is it one of the package environments. Fortunately, we do not have to do a search - * as, in this case, the {@link REnvironment} value is also stored in the frame. - * - * @return ({code null) if this is a function frame, else the associated environment - */ - public static REnvironment checkNonFunctionFrame(Frame frame) { - RFunction callerFunc = RArguments.getFunction(frame); - if (callerFunc == null) { - REnvironment env = RArguments.getEnvironment(frame); - assert env != null; - return env; - } else { - return null; - } - } - /** * Converts a {@link Frame} to an {@link REnvironment}, which necessarily requires the frame to * be materialized. */ public static REnvironment frameToEnvironment(MaterializedFrame frame) { - REnvironment env = checkNonFunctionFrame(frame); + REnvironment env = RArguments.getEnvironment(frame); if (env == null) { - env = lexicalChain(frame); + assert RArguments.getFunction(frame) != null; + env = createEnclosingEnvironments(frame); } return env; } @@ -466,11 +448,11 @@ public abstract class REnvironment implements RAttributable { * {@link MaterializedFrame}. */ @SlowPath - public static REnvironment lexicalChain(MaterializedFrame frame) { - REnvironment env = checkNonFunctionFrame(frame); + public static REnvironment createEnclosingEnvironments(MaterializedFrame frame) { + REnvironment env = RArguments.getEnvironment(frame); if (env == null) { // parent is the env of the enclosing frame - env = REnvironment.Function.create(lexicalChain(RArguments.getEnclosingFrame(frame)), frame); + env = REnvironment.Function.create(createEnclosingEnvironments(RArguments.getEnclosingFrame(frame)), frame); } return env; } @@ -844,10 +826,15 @@ public abstract class REnvironment implements RAttributable { private Function(REnvironment parent, MaterializedFrame frame) { // function environments are not named super(parent, UNNAMED, frame); + // Associate frame with the environment + RArguments.setEnvironment(frame, this); } private static Function create(REnvironment parent, MaterializedFrame frame) { - Function result = new Function(parent, frame); + Function result = (Function) RArguments.getEnvironment(frame); + if (result == null) { + result = new Function(parent, frame); + } return result; } diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/envframe/REnvFrameAccess.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/envframe/REnvFrameAccess.java index 9d9024e185235a30d434679621ccb37f910ea439..f58119f88c210cb0a4798618a18c46f551f8e177 100644 --- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/envframe/REnvFrameAccess.java +++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/envframe/REnvFrameAccess.java @@ -35,14 +35,6 @@ import com.oracle.truffle.r.runtime.data.*; * environment which never has an associated frame. */ public class REnvFrameAccess { - /** - * Return the unique id that identifies the associated environment in the frame, or {@code null} - * if none. - */ - public Object id() { - throw notImplemented("id"); - } - /** * Return the value of object named {@code name} or {@code null} if not found. */ @@ -70,7 +62,7 @@ public class REnvFrameAccess { /** * Return the names in the environment that match {@code pattern}. - * + * * @param allNames if {@code false} ignore names beginning with ".". * @param pattern if not {@code null} only include names matching {@code pattern}. */ diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/envframe/REnvTruffleFrameAccess.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/envframe/REnvTruffleFrameAccess.java index c5042bc78a60634355f2896d62d85771cf408340..83950ef8c7d8420884386ea701d6a513867d91f8 100644 --- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/envframe/REnvTruffleFrameAccess.java +++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/envframe/REnvTruffleFrameAccess.java @@ -38,13 +38,8 @@ import com.oracle.truffle.r.runtime.data.*; public class REnvTruffleFrameAccess extends REnvFrameAccessBindingsAdapter { private MaterializedFrame frame; - private Object id; public REnvTruffleFrameAccess(VirtualFrame frame) { - this.id = new Object(); - FrameDescriptor fd = frame.getFrameDescriptor(); - FrameSlot idSlot = fd.addFrameSlot(id); - frame.setObject(idSlot, id); this.frame = frame.materialize(); } @@ -132,11 +127,6 @@ public class REnvTruffleFrameAccess extends REnvFrameAccessBindingsAdapter { return null; } - @Override - public Object id() { - return id; - } - private static String[] getStringIdentifiers(FrameDescriptor fd) { return fd.getIdentifiers().stream().filter(e -> (e instanceof String)).collect(Collectors.toSet()).toArray(RRuntime.STRING_ARRAY_SENTINEL); }