[GR-5611] Changes to FastR as required by data.table (mostly bug fixes of FastR found from intergrating data.table).

Palez · Palez · commit ee11fce71b24 · 2018-11-26T01:33:06.000-08:00
PullRequest: fastr/1800
diff --git a/com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/common/JavaUpCallsRFFIImpl.java b/com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/common/JavaUpCallsRFFIImpl.java
@@ -96,6 +96,8 @@
 import com.oracle.truffle.r.runtime.data.RTypedValue;
 import com.oracle.truffle.r.runtime.data.RUnboundValue;
 import com.oracle.truffle.r.runtime.data.RVector;
+import com.oracle.truffle.r.runtime.data.model.RAbstractAtomicVector;
+import com.oracle.truffle.r.runtime.data.model.RAbstractListBaseVector;
 import com.oracle.truffle.r.runtime.data.model.RAbstractListVector;
 import com.oracle.truffle.r.runtime.data.model.RAbstractStringVector;
 import com.oracle.truffle.r.runtime.data.model.RAbstractVector;
@@ -739,7 +741,7 @@ public Object PRINTNAME(Object x) {
             return x;
         }
         guaranteeInstanceOf(x, RSymbol.class);
-        return CharSXPWrapper.create(((RSymbol) x).getName());
+        return ((RSymbol) x).getWrappedName();
     }
 
     @Override
@@ -2410,8 +2412,12 @@ public Object LOGICAL(Object x) {
     }
 
     @Override
+    @TruffleBoundary
     public Object REAL(Object x) {
-        return VectorRFFIWrapper.get(guaranteeVectorOrNull(x, RDoubleVector.class));
+        if ((x instanceof RAbstractStringVector) || (x instanceof RAbstractListBaseVector)) {
+            RFFIUtils.unimplemented("REAL is being called for type " + Utils.getTypeName(x));
+        }
+        return VectorRFFIWrapper.get(guaranteeVectorOrNull(x, RAbstractAtomicVector.class));
     }
 
     @Override
diff --git a/com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/upcalls/StdUpCallsRFFI.java b/com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/upcalls/StdUpCallsRFFI.java
@@ -203,7 +203,7 @@ public interface StdUpCallsRFFI {
     /**
      * WARNING: argument order reversed from Rf_findVarInFrame!
      */
-    Object Rf_findVar(Object symbolArg, Object envArg);
+    Object Rf_findVar(Object symbolArg, @RFFIResultOwner Object envArg);
 
     Object Rf_findVarInFrame(@RFFIResultOwner Object envArg, Object symbolArg);
 
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/RASTBuilder.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/RASTBuilder.java
@@ -132,7 +132,15 @@ private RSyntaxNode createCall(SourceSection source, RSyntaxNode lhs, List<Argum
                         // switch the args if needed
                         RSyntaxNode lhsArg = args.get(switchArgs ? 1 : 0).value;
                         RSyntaxNode rhsArg = args.get(switchArgs ? 0 : 1).value;
-                        return new ReplacementDispatchNode(source, lhsLookup, lhsArg, rhsArg, isSuper, context.getReplacementVarsStartIndex());
+                        String lhsName = args.get(0).name;
+                        String rhsName = args.get(1).name;
+                        ArgumentsSignature names;
+                        if (lhsName == null && rhsName == null) {
+                            names = ArgumentsSignature.empty(2);
+                        } else {
+                            names = ArgumentsSignature.get(lhsName, rhsName);
+                        }
+                        return new ReplacementDispatchNode(source, lhsLookup, lhsArg, rhsArg, isSuper, context.getReplacementVarsStartIndex(), names);
                 }
             } else if (args.size() == 3) {
                 switch (symbol) {
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/control/ReplacementDispatchNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/control/ReplacementDispatchNode.java
@@ -61,14 +61,16 @@ public final class ReplacementDispatchNode extends OperatorNode {
 
     private final boolean isSuper;
     private final int tempNamesStartIndex;
+    private final ArgumentsSignature names;
 
-    public ReplacementDispatchNode(SourceSection src, RSyntaxLookup operator, RSyntaxNode lhs, RSyntaxNode rhs, boolean isSuper, int tempNamesStartIndex) {
+    public ReplacementDispatchNode(SourceSection src, RSyntaxLookup operator, RSyntaxNode lhs, RSyntaxNode rhs, boolean isSuper, int tempNamesStartIndex, ArgumentsSignature names) {
         super(src, operator);
         assert lhs != null && rhs != null;
         this.lhs = lhs.asRNode();
         this.rhs = rhs.asRNode();
         this.isSuper = isSuper;
         this.tempNamesStartIndex = tempNamesStartIndex;
+        this.names = names;
     }
 
     @Override
@@ -123,7 +125,7 @@ private RNode createWriteVariableNode(RSyntaxNode lhsSyntax) {
 
     @Override
     public ArgumentsSignature getSyntaxSignature() {
-        return ArgumentsSignature.empty(2);
+        return names;
     }
 
     @Override
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/control/ReplacementNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/control/ReplacementNode.java
@@ -154,7 +154,7 @@ private static RNode createFunctionUpdate(SourceSection source, RSyntaxNode newL
             newArgs[1] = builder.lookup(oldArgs[1].getLazySourceSection(), ((RSyntaxLookup) oldArgs[1]).getIdentifier() + "<-", true);
             newSyntaxLHS = RCallSpecialNode.createCall(callLHS.getLazySourceSection(), builder.process(callLHS.getSyntaxLHS(), codeBuilderContext).asRNode(), callLHS.getSyntaxSignature(), newArgs);
         }
-        return RCallSpecialNode.createCallInReplace(source, newSyntaxLHS.asRNode(), ArgumentsSignature.get(names), argNodes, 0, argNodes.length - 1).asRNode();
+        return RCallSpecialNode.createCallInReplace(source, newSyntaxLHS.asRNode(), ArgumentsSignature.get(names), argNodes, argNodes.length - 1).asRNode();
     }
 
     static RPairList getLanguage(WriteVariableNode wvn) {
@@ -169,11 +169,11 @@ private static RSyntaxNode createLookup(SourceSection source, String idenifier)
         return RContext.getASTBuilder().lookup(source, idenifier, false);
     }
 
-    private static RNode createReplacementTarget(RSyntaxLookup variable, boolean isSuper, boolean localPeek) {
+    private static RNode createReplacementTarget(RSyntaxLookup variable, boolean isSuper, boolean isSpecial) {
         if (isSuper) {
             return ReadVariableNode.wrap(variable.getLazySourceSection(), ReadVariableNode.createSuperLookup(variable.getIdentifier())).asRNode();
         } else {
-            return localPeek ? new PeekLocalVariableNode(variable.getIdentifier())
+            return isSpecial ? new PeekLocalVariableNode(variable.getIdentifier())
                             : ReadVariableNode.wrap(variable.getLazySourceSection(), ReadVariableNode.create(variable.getIdentifier(), true)).asRNode();
         }
     }
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/helpers/MaterializeNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/helpers/MaterializeNode.java
@@ -98,21 +98,12 @@ protected Object doGeneric(Object o) {
     }
 
     private RList materializeContents(RList list) {
-        boolean changed = false;
-        RList materializedContents = null;
         for (int i = 0; i < list.getLength(); i++) {
             Object element = list.getDataAt(i);
             Object materializedElem = doGenericSlowPath(element);
             if (materializedElem != element) {
-                materializedContents = (RList) list.copy();
-                changed = true;
+                list.setDataAt(i, materializedElem);
             }
-            if (changed && materializedElem != element) {
-                materializedContents.setDataAt(i, materializedElem);
-            }
-        }
-        if (changed) {
-            return materializedContents;
         }
         return list;
     }
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RSymbol.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RSymbol.java
@@ -47,11 +47,10 @@ public final class RSymbol extends RAttributeStorage {
 
     public static final RSymbol MISSING = RDataFactory.createSymbol("");
 
-    private final String name;
     private CharSXPWrapper nameWrapper;
 
     private RSymbol(String name) {
-        this.name = name;
+        this.nameWrapper = CharSXPWrapper.createInterned(name);
     }
 
     @TruffleBoundary
@@ -70,13 +69,10 @@ public RType getRType() {
     }
 
     public String getName() {
-        return name;
+        return nameWrapper.getContents();
     }
 
     public CharSXPWrapper getWrappedName() {
-        if (nameWrapper == null) {
-            nameWrapper = CharSXPWrapper.createInterned(name);
-        }
         return nameWrapper;
     }
 
@@ -99,7 +95,7 @@ public boolean equals(Object obj) {
         if (obj == this) {
             return true;
         } else if (obj instanceof RSymbol) {
-            return ((RSymbol) obj).getName() == this.name;
+            return ((RSymbol) obj).getName() == this.nameWrapper.getContents();
         }
         return false;
     }
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/nodes/RSyntaxLookup.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/nodes/RSyntaxLookup.java
@@ -82,6 +82,7 @@ default boolean isPromiseLookup() {
      * section.
      */
     static RSyntaxLookup createDummyLookup(SourceSection source, String identifier, boolean isFunctionLookup) {
+        assert Utils.isInterned(identifier);
         return new RSyntaxLookup() {
             @Override
             public SourceSection getLazySourceSection() {
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
@@ -77254,6 +77254,10 @@ Error in substitute(1, NA) : invalid environment specified
 #substitute(1, list(list(1)))
 [1] 1
 
+##com.oracle.truffle.r.test.builtins.TestBuiltin_substitute.testSubstitute#
+#substitute(`:=`(a=3,b=3))
+`:=`(a = 3, b = 3)
+
 ##com.oracle.truffle.r.test.builtins.TestBuiltin_substitute.testSubstitute#
 #substitute(expr=1, env=1)
 Error in substitute(expr = 1, env = 1) : invalid environment specified
diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_substitute.java b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_substitute.java
@@ -150,5 +150,6 @@ public void testSubstitute() {
         assertEval("{ foo <- function(...) bar(3+2); bar <- function(...) { list(...); substitute(list(...)); }; foo(1+2); }");
 
         assertEval("typeof(substitute())");
+        assertEval("substitute(`:=`(a=3,b=3))");
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -154,7 +154,7 @@ private static RNode createFunctionUpdate(SourceSection source, RSyntaxNode newL`
`154`	`154`	`newArgs[1] = builder.lookup(oldArgs[1].getLazySourceSection(), ((RSyntaxLookup) oldArgs[1]).getIdentifier() + "<-", true);`
`155`	`155`	`newSyntaxLHS = RCallSpecialNode.createCall(callLHS.getLazySourceSection(), builder.process(callLHS.getSyntaxLHS(), codeBuilderContext).asRNode(), callLHS.getSyntaxSignature(), newArgs);`
`156`	`156`	`}`
`157`		`- return RCallSpecialNode.createCallInReplace(source, newSyntaxLHS.asRNode(), ArgumentsSignature.get(names), argNodes, 0, argNodes.length - 1).asRNode();`
	`157`	`+ return RCallSpecialNode.createCallInReplace(source, newSyntaxLHS.asRNode(), ArgumentsSignature.get(names), argNodes, argNodes.length - 1).asRNode();`
`158`	`158`	`}`
`159`	`159`
`160`	`160`	`static RPairList getLanguage(WriteVariableNode wvn) {`
`@@ -169,11 +169,11 @@ private static RSyntaxNode createLookup(SourceSection source, String idenifier)`
`169`	`169`	`return RContext.getASTBuilder().lookup(source, idenifier, false);`
`170`	`170`	`}`
`171`	`171`
`172`		`- private static RNode createReplacementTarget(RSyntaxLookup variable, boolean isSuper, boolean localPeek) {`
	`172`	`+ private static RNode createReplacementTarget(RSyntaxLookup variable, boolean isSuper, boolean isSpecial) {`
`173`	`173`	`if (isSuper) {`
`174`	`174`	`return ReadVariableNode.wrap(variable.getLazySourceSection(), ReadVariableNode.createSuperLookup(variable.getIdentifier())).asRNode();`
`175`	`175`	`} else {`
`176`		`- return localPeek ? new PeekLocalVariableNode(variable.getIdentifier())`
	`176`	`+ return isSpecial ? new PeekLocalVariableNode(variable.getIdentifier())`
`177`	`177`	`: ReadVariableNode.wrap(variable.getLazySourceSection(), ReadVariableNode.create(variable.getIdentifier(), true)).asRNode();`
`178`	`178`	`}`
`179`	`179`	`}`
Original file line number	Diff line number	Diff line change
`@@ -98,21 +98,12 @@ protected Object doGeneric(Object o) {`
`98`	`98`	`}`
`99`	`99`
`100`	`100`	`private RList materializeContents(RList list) {`
`101`		`- boolean changed = false;`
`102`		`- RList materializedContents = null;`
`103`	`101`	`for (int i = 0; i < list.getLength(); i++) {`
`104`	`102`	`Object element = list.getDataAt(i);`
`105`	`103`	`Object materializedElem = doGenericSlowPath(element);`
`106`	`104`	`if (materializedElem != element) {`
`107`		`- materializedContents = (RList) list.copy();`
`108`		`- changed = true;`
	`105`	`+ list.setDataAt(i, materializedElem);`
`109`	`106`	`}`
`110`		`- if (changed && materializedElem != element) {`
`111`		`- materializedContents.setDataAt(i, materializedElem);`
`112`		`- }`
`113`		`- }`
`114`		`- if (changed) {`
`115`		`- return materializedContents;`
`116`	`107`	`}`
`117`	`108`	`return list;`
`118`	`109`	`}`
Original file line number	Diff line number	Diff line change
`@@ -47,11 +47,10 @@ public final class RSymbol extends RAttributeStorage {`
`47`	`47`
`48`	`48`	`public static final RSymbol MISSING = RDataFactory.createSymbol("");`
`49`	`49`
`50`		`- private final String name;`
`51`	`50`	`private CharSXPWrapper nameWrapper;`
`52`	`51`
`53`	`52`	`private RSymbol(String name) {`
`54`		`- this.name = name;`
	`53`	`+ this.nameWrapper = CharSXPWrapper.createInterned(name);`
`55`	`54`	`}`
`56`	`55`
`57`	`56`	`@TruffleBoundary`
`@@ -70,13 +69,10 @@ public RType getRType() {`
`70`	`69`	`}`
`71`	`70`
`72`	`71`	`public String getName() {`
`73`		`- return name;`
	`72`	`+ return nameWrapper.getContents();`
`74`	`73`	`}`
`75`	`74`
`76`	`75`	`public CharSXPWrapper getWrappedName() {`
`77`		`- if (nameWrapper == null) {`
`78`		`- nameWrapper = CharSXPWrapper.createInterned(name);`
`79`		`- }`
`80`	`76`	`return nameWrapper;`
`81`	`77`	`}`
`82`	`78`
`@@ -99,7 +95,7 @@ public boolean equals(Object obj) {`
`99`	`95`	`if (obj == this) {`
`100`	`96`	`return true;`
`101`	`97`	`} else if (obj instanceof RSymbol) {`
`102`		`- return ((RSymbol) obj).getName() == this.name;`
	`98`	`+ return ((RSymbol) obj).getName() == this.nameWrapper.getContents();`
`103`	`99`	`}`
`104`	`100`	`return false;`
`105`	`101`	`}`
Original file line number	Diff line number	Diff line change
`@@ -150,5 +150,6 @@ public void testSubstitute() {`
`150`	`150`	`assertEval("{ foo <- function(...) bar(3+2); bar <- function(...) { list(...); substitute(list(...)); }; foo(1+2); }");`
`151`	`151`
`152`	`152`	`assertEval("typeof(substitute())");`
	`153`	+ assertEval("substitute(`:=`(a=3,b=3))");
`153`	`154`	`}`
`154`	`155`	`}`