From ae140d1606a09b1e38b09cdf071cf9b21cba1dfc Mon Sep 17 00:00:00 2001
From: Christian Humer <christian.humer@oracle.com>
Date: Mon, 15 Feb 2016 14:12:51 -0800
Subject: [PATCH] Fix names<- must never assign itself as names vector; Cleanup
 names<-.

---
 .../r/nodes/builtin/base/UpdateNames.java     | 76 ++++++++-----------
 .../truffle/r/runtime/data/RVector.java       |  4 +
 .../truffle/r/test/ExpectedTestOutput.test    |  4 +
 .../r/test/builtins/TestBuiltin_names.java    |  1 +
 4 files changed, 39 insertions(+), 46 deletions(-)

diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/UpdateNames.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/UpdateNames.java
index cfeb5ab385..1ff67547bc 100644
--- a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/UpdateNames.java
+++ b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/UpdateNames.java
@@ -22,18 +22,20 @@
  */
 package com.oracle.truffle.r.nodes.builtin.base;
 
-import static com.oracle.truffle.r.runtime.RBuiltinKind.*;
+import static com.oracle.truffle.r.runtime.RBuiltinKind.PRIMITIVE;
 
-import java.util.*;
-
-import com.oracle.truffle.api.*;
+import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
-import com.oracle.truffle.api.dsl.*;
-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.*;
-import com.oracle.truffle.r.runtime.data.model.*;
+import com.oracle.truffle.api.dsl.Specialization;
+import com.oracle.truffle.r.nodes.builtin.RInvisibleBuiltinNode;
+import com.oracle.truffle.r.nodes.unary.CastStringNode;
+import com.oracle.truffle.r.nodes.unary.CastStringNodeGen;
+import com.oracle.truffle.r.runtime.RBuiltin;
+import com.oracle.truffle.r.runtime.data.RDataFactory;
+import com.oracle.truffle.r.runtime.data.RNull;
+import com.oracle.truffle.r.runtime.data.RStringVector;
+import com.oracle.truffle.r.runtime.data.model.RAbstractContainer;
+import com.oracle.truffle.r.runtime.data.model.RAbstractVector;
 
 @RBuiltin(name = "names<-", kind = PRIMITIVE, parameterNames = {"x", "value"})
 public abstract class UpdateNames extends RInvisibleBuiltinNode {
@@ -52,46 +54,28 @@ public abstract class UpdateNames extends RInvisibleBuiltinNode {
 
     @Specialization
     @TruffleBoundary
-    protected RAbstractContainer updateNames(RAbstractContainer container, @SuppressWarnings("unused") RNull names) {
-        controlVisibility();
-        RAbstractContainer result = container.materializeNonShared();
-        result.setNames(null);
-        return result;
-    }
-
-    @Specialization
-    @TruffleBoundary
-    protected RAbstractContainer updateNames(RAbstractContainer container, RStringVector names) {
+    protected RAbstractContainer updateNames(RAbstractContainer container, Object names) {
         controlVisibility();
-        RAbstractContainer result = container.materializeNonShared();
-        RStringVector namesVector = names;
-        if (names.getLength() < result.getLength()) {
-            namesVector = names.copyResized(result.getLength(), true);
+        Object newNames = castString(names);
+        if (newNames == RNull.instance) {
+            RAbstractContainer result = container.materializeNonShared();
+            result.setNames(null);
+            return result;
         }
-        result.setNames(namesVector);
-        return result;
-    }
 
-    @Specialization
-    @TruffleBoundary
-    protected RAbstractContainer updateNames(RAbstractContainer container, String name) {
-        controlVisibility();
-        RAbstractContainer result = container.materializeNonShared();
-        String[] names = new String[result.getLength()];
-        Arrays.fill(names, RRuntime.STRING_NA);
-        names[0] = name;
-        RStringVector namesVector = RDataFactory.createStringVector(names, names.length <= 1 && names[0] != RRuntime.STRING_NA);
-        result.setNames(namesVector);
-        return result;
-    }
-
-    @Specialization
-    protected RAbstractContainer updateNames(RAbstractContainer container, Object names) {
-        controlVisibility();
-        if (names instanceof RAbstractVector) {
-            return updateNames(container, (RStringVector) castString(names));
+        RStringVector stringVector;
+        if (newNames instanceof String) {
+            stringVector = RDataFactory.createStringVector((String) newNames);
         } else {
-            return updateNames(container, (String) castString(names));
+            stringVector = (RStringVector) ((RAbstractVector) newNames).materialize();
         }
+        RAbstractContainer result = container.materializeNonShared();
+        if (stringVector.getLength() < result.getLength()) {
+            stringVector = stringVector.copyResized(result.getLength(), true);
+        } else if (stringVector == container) {
+            stringVector = (RStringVector) stringVector.copy();
+        }
+        result.setNames(stringVector);
+        return result;
     }
 }
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RVector.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RVector.java
index f6f430d0ef..4ecd2e663c 100644
--- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RVector.java
+++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RVector.java
@@ -64,6 +64,7 @@ public abstract class RVector extends RSharingAttributeStorage implements RShare
     protected RVector(boolean complete, int length, int[] dimensions, RStringVector names) {
         this.complete = complete;
         this.dimensions = dimensions;
+        assert names != this;
         this.names = names;
         this.rowNames = RNull.instance;
         if (names != null) {
@@ -102,6 +103,7 @@ public abstract class RVector extends RSharingAttributeStorage implements RShare
     }
 
     public final void setInternalNames(RStringVector newNames) {
+        assert newNames != this;
         names = newNames;
     }
 
@@ -286,6 +288,7 @@ public abstract class RVector extends RSharingAttributeStorage implements RShare
         } else {
             putAttribute(RRuntime.NAMES_ATTR_KEY, newNames);
         }
+        assert newNames != this;
         this.names = newNames;
     }
 
@@ -314,6 +317,7 @@ public abstract class RVector extends RSharingAttributeStorage implements RShare
                 this.dimNames = newDimNames;
             } else {
                 putAttribute(RRuntime.NAMES_ATTR_KEY, newNames);
+                assert newNames != this;
                 this.names = newNames;
             }
         }
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
index 3a09e444bf..90fed61947 100644
--- 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
@@ -29261,6 +29261,10 @@ Error in k() : argument "y" is missing, with no default
 #{ x <- quote(plot(x = age, y = weight)); names(x) }
 [1] ""  "x" "y"
 
+##com.oracle.truffle.r.test.builtins.TestBuiltin_names.testNames
+#{ symNames <- c("foobar", "bar"); names(symNames) = symNames; names(names(symNames)); }
+NULL
+
 ##com.oracle.truffle.r.test.builtins.TestBuiltin_names.testNames
 #{ x<-c(1,2,3); dim(x)<-3; dimnames(x)<-list(c(11,12,13)); names(x) }
 [1] "11" "12" "13"
diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_names.java b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_names.java
index 75c63f04ab..25807e7b3c 100644
--- a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_names.java
+++ b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_names.java
@@ -201,6 +201,7 @@ public class TestBuiltin_names extends TestBase {
     @Test
     public void testNames() {
         assertEval("{ x<-c(1,2,3); dim(x)<-3; dimnames(x)<-list(c(11,12,13)); names(x) }");
+        assertEval("{ symNames <- c(\"foobar\", \"bar\"); names(symNames) = symNames; names(names(symNames)); }");
     }
 
     @Test
-- 
GitLab