From e35160b5b93e40bc351c7a6c80ffbb792e95a80d Mon Sep 17 00:00:00 2001
From: Mick Jordan <mick.jordan@oracle.com>
Date: Tue, 27 Jan 2015 16:45:02 -0800
Subject: [PATCH] assign as .Internal

---
 .../truffle/r/nodes/builtin/base/Assign.java  | 197 ++++++++----------
 .../truffle/r/nodes/builtin/base/R/assign.R   |  30 +++
 2 files changed, 122 insertions(+), 105 deletions(-)
 create mode 100644 com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/R/assign.R

diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Assign.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Assign.java
index 2810a7c458..36f2bc351b 100644
--- a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Assign.java
+++ b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Assign.java
@@ -30,36 +30,34 @@ import com.oracle.truffle.api.dsl.*;
 import com.oracle.truffle.api.frame.*;
 import com.oracle.truffle.api.nodes.ExplodeLoop;
 import com.oracle.truffle.api.utilities.BranchProfile;
-import com.oracle.truffle.r.nodes.*;
 import com.oracle.truffle.r.nodes.access.*;
 import com.oracle.truffle.r.nodes.builtin.*;
 import com.oracle.truffle.r.runtime.*;
-import com.oracle.truffle.r.runtime.data.*;
+import com.oracle.truffle.r.runtime.data.model.*;
 import com.oracle.truffle.r.runtime.env.*;
 import com.oracle.truffle.r.runtime.env.REnvironment.*;
 import com.oracle.truffle.r.runtime.env.frame.*;
 
-@RBuiltin(name = "assign", kind = SUBSTITUTE, parameterNames = {"x", "value", "pos", "envir", "inherits", "immediate"})
-// TODO INTERNAL
+/**
+ * The {@code assign} builtin. There are two special cases worth optimnizing:
+ * <ul>
+ * <li>{@code inherits == FALSE}. No need to search environment hierarchy.</li>
+ * <li>{@code envir} corresponds to the currently active frame. Unfortunately this is masked
+ * somewhat by the signature of the {@code .Internal}, which only provides an environment. So this
+ * is currently disabled,</li>
+ * </ul>
+ *
+ */
+@RBuiltin(name = "assign", kind = INTERNAL, parameterNames = {"x", "value", "envir", "inherits"})
 public abstract class Assign extends RInvisibleBuiltinNode {
 
-    // TODO convert to .Internal using assign.R to simplify the environment specializations
-
     @Child private WriteVariableNode writeVariableNode;
 
     @CompilationFinal private String lastName;
 
-    // FIXME deal with omitted parameters: pos, imemdiate
-
     @CompilationFinal private final BranchProfile[] slotFoundOnIteration = {BranchProfile.create(), BranchProfile.create(), BranchProfile.create()};
     private final BranchProfile invalidateProfile = BranchProfile.create();
 
-    @Override
-    public RNode[] getParameterValues() {
-        return new RNode[]{ConstantNode.create(RMissing.instance), ConstantNode.create(RMissing.instance), ConstantNode.create(-1), ConstantNode.create(RMissing.instance),
-                        ConstantNode.create(RRuntime.LOGICAL_FALSE), ConstantNode.create(RRuntime.LOGICAL_TRUE)};
-    }
-
     private void ensureWrite(String x) {
         if (writeVariableNode == null || !x.equals(lastName)) {
             CompilerDirectives.transferToInterpreterAndInvalidate();
@@ -70,138 +68,127 @@ public abstract class Assign extends RInvisibleBuiltinNode {
         }
     }
 
-    @Specialization(guards = {"noEnv", "!doesInheritS"})
-    @SuppressWarnings("unused")
-    protected Object assignNoInherit(VirtualFrame frame, String x, Object value, Object pos, RMissing envir, byte inherits, byte immediate) {
+    private String checkVariable(RAbstractStringVector xVec) throws RError {
+        int len = xVec.getLength();
+        if (len == 1) {
+            return xVec.getDataAt(0);
+        } else if (len == 0) {
+            throw RError.error(getEncapsulatingSourceSection(), RError.Message.INVALID_FIRST_ARGUMENT);
+        } else {
+            RContext.getInstance().setEvalWarning("only the first element is used as variable name");
+            return xVec.getDataAt(0);
+        }
+    }
+
+    /**
+     * The general case that requires searching the environment hierarchy.
+     */
+    @Specialization(guards = {"inheritsIsTrue", "!currentFrame"})
+    protected Object assignInherit(RAbstractStringVector xVec, Object value, REnvironment envir, @SuppressWarnings("unused") byte inherits) {
         controlVisibility();
-        ensureWrite(x);
-        writeVariableNode.execute(frame, value);
+        String x = checkVariable(xVec);
+        REnvironment env = envir;
+        while (env != null) {
+            if (env.get(x) != null) {
+                break;
+            }
+            env = env.getParent();
+        }
+        try {
+            if (env != null) {
+                env.put(x, value);
+            } else {
+                REnvironment.globalEnv().put(x, value);
+            }
+        } catch (PutException ex) {
+            throw RError.error(getEncapsulatingSourceSection(), ex);
+        }
         return value;
     }
 
-    @ExplodeLoop
-    @Specialization(guards = {"noEnv", "doesInheritS"})
     @SuppressWarnings("unused")
-    protected Object assignInherit(VirtualFrame virtualFrame, String variableName, Object variableValue, Object pos, RMissing environment, byte inherits, byte immediate) {
+    @ExplodeLoop
+    @Specialization(guards = {"inheritsIsTrue", "currentFrame"})
+    protected Object assignInherit(VirtualFrame virtualFrame, RAbstractStringVector xVec, Object value, REnvironment envir, byte inherits) {
+        String x = checkVariable(xVec);
         controlVisibility();
         MaterializedFrame materializedFrame = virtualFrame.materialize();
-        FrameSlot slot = materializedFrame.getFrameDescriptor().findFrameSlot(variableName);
+        FrameSlot slot = materializedFrame.getFrameDescriptor().findFrameSlot(x);
         int iterationsAmount = CompilerAsserts.compilationConstant(slotFoundOnIteration.length);
         for (int i = 0; i < iterationsAmount; i++) {
             if (isAppropriateFrameSlot(slot, materializedFrame)) {
-                addValueToFrame(variableName, variableValue, materializedFrame, slot);
-                return variableValue;
+                addValueToFrame(x, value, materializedFrame, slot);
+                return value;
             }
             slotFoundOnIteration[i].enter();
             materializedFrame = RArguments.getEnclosingFrame(materializedFrame);
-            slot = materializedFrame.getFrameDescriptor().findFrameSlot(variableName);
-        }
-        assignInheritGenericCase(materializedFrame, variableName, variableValue);
-        return variableValue;
-    }
-
-    private Object assignInheritGenericCase(MaterializedFrame startFrame, String variableName, Object variableValue) {
-        MaterializedFrame materializedFrame = startFrame;
-        FrameSlot frameSlot = materializedFrame.getFrameDescriptor().findFrameSlot(variableName);
-        while (!isAppropriateFrameSlot(frameSlot, materializedFrame)) {
-            materializedFrame = RArguments.getEnclosingFrame(materializedFrame);
-            frameSlot = materializedFrame.getFrameDescriptor().findFrameSlot(variableName);
+            slot = materializedFrame.getFrameDescriptor().findFrameSlot(x);
         }
-        addValueToFrame(variableName, variableValue, materializedFrame, frameSlot);
-        return variableValue;
-    }
-
-    private void addValueToFrame(String variableName, Object variableValue, Frame frame, FrameSlot frameSlot) {
-        FrameSlot fs = frameSlot;
-        if (fs == null) {
-            fs = FrameSlotChangeMonitor.addFrameSlot(frame.getFrameDescriptor(), variableName, FrameSlotKind.Illegal);
-        }
-        FrameSlotChangeMonitor.setObjectAndInvalidate(frame, fs, variableValue, invalidateProfile);
-    }
-
-    private static boolean isAppropriateFrameSlot(FrameSlot frameSlot, MaterializedFrame materializedFrame) {
-        return frameSlot != null || REnvironment.isGlobalEnvFrame(materializedFrame);
+        assignInheritGenericCase(materializedFrame, x, value);
+        return value;
     }
 
-    @Specialization(guards = "!doesInherit")
+    @Specialization(guards = {"!inheritsIsTrue", "currentFrame"})
     @SuppressWarnings("unused")
-    protected Object assignNoInherit(String x, Object value, REnvironment pos, RMissing envir, byte inherits, byte immediate) {
+    protected Object assignNoInherit(VirtualFrame frame, RAbstractStringVector xVec, Object value, REnvironment envir, byte inherits) {
+        String x = checkVariable(xVec);
         controlVisibility();
-        if (pos == REnvironment.emptyEnv()) {
-            throw RError.error(getEncapsulatingSourceSection(), RError.Message.CANNOT_ASSIGN_IN_EMPTY_ENV);
-        }
-        try {
-            pos.put(x, value);
-        } catch (PutException ex) {
-            throw RError.error(getEncapsulatingSourceSection(), ex);
-        }
+        ensureWrite(x);
+        writeVariableNode.execute(frame, value);
         return value;
     }
 
-    @Specialization(guards = "!doesInheritX")
-    @SuppressWarnings("unused")
-    protected Object assignNoInherit(String x, Object value, int pos, REnvironment envir, byte inherits, byte immediate) {
-        return assignNoInherit(x, value, envir, RMissing.instance, inherits, immediate);
-    }
-
-    @Specialization(guards = "doesInherit")
+    @Specialization(guards = {"!inheritsIsTrue", "!currentFrame"})
     @SuppressWarnings("unused")
-    protected Object assignInherit(String x, Object value, REnvironment pos, RMissing envir, byte inherits, byte immediate) {
+    protected Object assignNoInherit(RAbstractStringVector xVec, Object value, REnvironment envir, byte inherits) {
+        String x = checkVariable(xVec);
         controlVisibility();
-        REnvironment env = pos;
-        while (env != null) {
-            if (env.get(x) != null) {
-                break;
-            }
-            env = env.getParent();
+        if (envir == REnvironment.emptyEnv()) {
+            throw RError.error(getEncapsulatingSourceSection(), RError.Message.CANNOT_ASSIGN_IN_EMPTY_ENV);
         }
         try {
-            if (env != null) {
-                env.put(x, value);
-            } else {
-                REnvironment.globalEnv().put(x, value);
-            }
+            envir.put(x, value);
         } catch (PutException ex) {
             throw RError.error(getEncapsulatingSourceSection(), ex);
         }
         return value;
     }
 
-    @Specialization(guards = "!doesInherit")
-    protected Object assignNoInherit(RStringVector x, Object value, REnvironment pos, RMissing envir, byte inherits, byte immediate) {
-        controlVisibility();
-        return assignNoInherit(x.getDataAt(0), value, pos, envir, inherits, immediate);
-    }
-
-    @Specialization(guards = "doesInherit")
-    protected Object assignInherit(RStringVector x, Object value, REnvironment pos, RMissing envir, byte inherits, byte immediate) {
-        controlVisibility();
-        return assignInherit(x.getDataAt(0), value, pos, envir, inherits, immediate);
-    }
-
-    @Specialization(guards = "doesInheritX")
-    protected Object assignInherit(RStringVector x, Object value, @SuppressWarnings("unused") int pos, REnvironment envir, byte inherits, byte immediate) {
-        controlVisibility();
-        return assignInherit(x.getDataAt(0), value, envir, RMissing.instance, inherits, immediate);
+    private Object assignInheritGenericCase(MaterializedFrame startFrame, String x, Object value) {
+        MaterializedFrame materializedFrame = startFrame;
+        FrameSlot frameSlot = materializedFrame.getFrameDescriptor().findFrameSlot(x);
+        while (!isAppropriateFrameSlot(frameSlot, materializedFrame)) {
+            materializedFrame = RArguments.getEnclosingFrame(materializedFrame);
+            frameSlot = materializedFrame.getFrameDescriptor().findFrameSlot(x);
+        }
+        addValueToFrame(x, value, materializedFrame, frameSlot);
+        return value;
     }
 
-    @SuppressWarnings("unused")
-    protected static boolean doesInherit(Object x, Object value, REnvironment pos, RMissing envir, byte inherits, byte immediate) {
-        return inherits == RRuntime.LOGICAL_TRUE;
+    private void addValueToFrame(String x, Object value, Frame frame, FrameSlot frameSlot) {
+        FrameSlot fs = frameSlot;
+        if (fs == null) {
+            fs = FrameSlotChangeMonitor.addFrameSlot(frame.getFrameDescriptor(), x, FrameSlotKind.Illegal);
+        }
+        FrameSlotChangeMonitor.setObjectAndInvalidate(frame, fs, value, invalidateProfile);
     }
 
-    @SuppressWarnings("unused")
-    protected static boolean doesInheritX(Object x, Object value, int pos, REnvironment envir, byte inherits, byte immediate) {
-        return inherits == RRuntime.LOGICAL_TRUE;
+    private static boolean isAppropriateFrameSlot(FrameSlot frameSlot, MaterializedFrame materializedFrame) {
+        return frameSlot != null || REnvironment.isGlobalEnvFrame(materializedFrame);
     }
 
     @SuppressWarnings("unused")
-    protected static boolean doesInheritS(String x, Object value, Object pos, RMissing envir, byte inherits, byte immediate) {
+    protected static boolean inheritsIsTrue(VirtualFrame frame, RAbstractStringVector x, Object value, REnvironment envir, byte inherits) {
         return inherits == RRuntime.LOGICAL_TRUE;
     }
 
     @SuppressWarnings("unused")
-    protected static boolean noEnv(String x, Object value, Object pos, RMissing envir, byte inherits, byte immediate) {
-        return !(pos instanceof REnvironment);
+    protected static boolean currentFrame(VirtualFrame frame, RAbstractStringVector x, Object value, REnvironment envir, byte inherits) {
+        /*
+         * TODO how to determine this efficiently, remembering that "frame" is for the assign
+         * closure, not the function that called it, which is where we want to do the assign iff
+         * that matches "envir"
+         */
+        return false;
     }
 }
diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/R/assign.R b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/R/assign.R
new file mode 100644
index 0000000000..17c24420e9
--- /dev/null
+++ b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/R/assign.R
@@ -0,0 +1,30 @@
+#  File src/library/base/R/assign.R
+#  Part of the R package, http://www.R-project.org
+#
+#  Copyright (C) 1995-2012 The R Core Team
+#
+#  This program is free software; you can redistribute it and/or modify
+#  it under the terms of the GNU General Public License as published by
+#  the Free Software Foundation; either version 2 of the License, or
+#  (at your option) any later version.
+#
+#  This program 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 for more details.
+#
+#  A copy of the GNU General Public License is available at
+#  http://www.r-project.org/Licenses/
+
+assign <-
+		function (x, value, pos = -1, envir = as.environment(pos),
+				inherits = FALSE, immediate = TRUE)
+	.Internal(assign(x, value, envir, inherits))
+
+## do_list2env in ../../../main/envir.c
+list2env <- function(x, envir = NULL, parent = parent.frame(),
+		hash = (length(x) > 100), size = max(29L, length(x)))
+{
+	if (is.null(envir)) envir <- new.env(hash=hash, parent=parent, size=size)
+	.Internal(list2env(x, envir))
+}
-- 
GitLab