add error checks to assign, coerce inherits, remove currentframe code

com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Assign.java

@@ -24,51 +24,34 @@ package com.oracle.truffle.r.nodes.builtin.base;
 
 import static com.oracle.truffle.r.runtime.RBuiltinKind.*;
 
-import com.oracle.truffle.api.*;
 import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
 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.access.*;
+import com.oracle.truffle.r.nodes.*;
 import com.oracle.truffle.r.nodes.builtin.*;
+import com.oracle.truffle.r.nodes.unary.*;
 import com.oracle.truffle.r.runtime.*;
 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.*;
 
 /**
- * The {@code assign} builtin. There are two special cases worth optimnizing:
+ * The {@code assign} builtin. There are two special cases worth optimizing:
  * <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>
+ * somewhat by the signature of the {@code .Internal}, which adds an frame. Not worth optimizing
+ * anyway as using {@code assign} for the current frame is highly unlikely.</li>
  * </ul>
  *
  */
 @RBuiltin(name = "assign", kind = INTERNAL, parameterNames = {"x", "value", "envir", "inherits"})
 public abstract class Assign extends RInvisibleBuiltinNode {
 
-    @Child private WriteVariableNode writeVariableNode;
-
-    @CompilationFinal private String lastName;
-
     @CompilationFinal private final BranchProfile[] slotFoundOnIteration = {BranchProfile.create(), BranchProfile.create(), BranchProfile.create()};
     private final BranchProfile errorProfile = BranchProfile.create();
     private final BranchProfile warningProfile = BranchProfile.create();
-    private final BranchProfile invalidateProfile = BranchProfile.create();
-
-    private void ensureWrite(String x) {
-        if (writeVariableNode == null || !x.equals(lastName)) {
-            CompilerDirectives.transferToInterpreterAndInvalidate();
-            lastName = x;
-            // must write to super (effect must be visible in caller frame of the assign builtin)
-            WriteVariableNode wvn = WriteVariableNode.create(lastName, null, false, false);
-            writeVariableNode = writeVariableNode == null ? insert(wvn) : writeVariableNode.replace(wvn);
-        }
-    }
 
     private String checkVariable(RAbstractStringVector xVec) {
         int len = xVec.getLength();
@@ -84,10 +67,16 @@ public abstract class Assign extends RInvisibleBuiltinNode {
         }
     }
 
+    @CreateCast("arguments")
+    public RNode[] castInherits(RNode[] arguments) {
+        arguments[3] = CastLogicalNodeGen.create(arguments[3], false, false, false);
+        return arguments;
+    }
+
     /**
      * The general case that requires searching the environment hierarchy.
      */
-    @Specialization(guards = {"inheritsIsTrue", "!currentFrame"})
+    @Specialization(guards = {"inheritsIsTrue"})
     protected Object assignInherit(RAbstractStringVector xVec, Object value, REnvironment envir, @SuppressWarnings("unused") byte inherits) {
         controlVisibility();
         String x = checkVariable(xVec);
@@ -111,39 +100,7 @@ public abstract class Assign extends RInvisibleBuiltinNode {
         return value;
     }
 
-    @SuppressWarnings("unused")
-    @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(x);
-        int iterationsAmount = CompilerAsserts.compilationConstant(slotFoundOnIteration.length);
-        for (int i = 0; i < iterationsAmount; i++) {
-            if (isAppropriateFrameSlot(slot, materializedFrame)) {
-                addValueToFrame(x, value, materializedFrame, slot);
-                return value;
-            }
-            slotFoundOnIteration[i].enter();
-            materializedFrame = RArguments.getEnclosingFrame(materializedFrame);
-            slot = materializedFrame.getFrameDescriptor().findFrameSlot(x);
-        }
-        assignInheritGenericCase(materializedFrame, x, value);
-        return value;
-    }
-
-    @Specialization(guards = {"!inheritsIsTrue", "currentFrame"})
-    @SuppressWarnings("unused")
-    protected Object assignNoInherit(VirtualFrame frame, RAbstractStringVector xVec, Object value, REnvironment envir, byte inherits) {
-        String x = checkVariable(xVec);
-        controlVisibility();
-        ensureWrite(x);
-        writeVariableNode.execute(frame, value);
-        return value;
-    }
-
-    @Specialization(guards = {"!inheritsIsTrue", "!currentFrame"})
+    @Specialization(guards = {"!inheritsIsTrue"})
     @SuppressWarnings("unused")
     protected Object assignNoInherit(RAbstractStringVector xVec, Object value, REnvironment envir, byte inherits) {
         String x = checkVariable(xVec);
@@ -161,27 +118,17 @@ public abstract class Assign extends RInvisibleBuiltinNode {
         return value;
     }
 
-    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;
-    }
-
-    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);
+    @SuppressWarnings("unused")
+    @Fallback
+    protected Object assignFallback(Object xVec, Object value, Object envir, Object inherits) {
+        errorProfile.enter();
+        if (RRuntime.asString(xVec) == null) {
+            throw RError.error(getEncapsulatingSourceSection(), RError.Message.INVALID_FIRST_ARGUMENT);
+        } else if (!(envir instanceof REnvironment)) {
+            throw RError.error(getEncapsulatingSourceSection(), RError.Message.INVALID_ARGUMENT, "pos");
+        } else {
+            throw RError.error(getEncapsulatingSourceSection(), RError.Message.INVALID_ARGUMENT, "inherits");
         }
-        FrameSlotChangeMonitor.setObjectAndInvalidate(frame, fs, value, true, invalidateProfile);
-    }
-
-    private static boolean isAppropriateFrameSlot(FrameSlot frameSlot, MaterializedFrame materializedFrame) {
-        return frameSlot != null || REnvironment.isGlobalEnvFrame(materializedFrame);
     }
 
     @SuppressWarnings("unused")
@@ -189,13 +136,4 @@ public abstract class Assign extends RInvisibleBuiltinNode {
         return inherits == RRuntime.LOGICAL_TRUE;
     }
 
-    @SuppressWarnings("unused")
-    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;
-    }
 }