From 49bbf78df462611016f4512e30730bc0d4a2c73d Mon Sep 17 00:00:00 2001
From: Mick Jordan <mick.jordan@oracle.com>
Date: Fri, 5 Feb 2016 17:30:36 -0800
Subject: [PATCH] remove structure SUBSTITIUTE in favor of R version; fix
 associated bugs

---
 .../r/library/fastr/FastRTreeStats.java       |   2 +-
 .../r/nodes/builtin/base/BasePackage.java     |   1 -
 .../r/nodes/builtin/base/Structure.java       | 157 ------------------
 .../nodes/builtin/base/UpdateAttributes.java  |   7 +-
 .../r/nodes/builtin/base/UpdateNames.java     |   4 +-
 .../truffle/r/runtime/data/RFactor.java       |  10 +-
 6 files changed, 14 insertions(+), 167 deletions(-)
 delete mode 100644 com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Structure.java

diff --git a/com.oracle.truffle.r.library/src/com/oracle/truffle/r/library/fastr/FastRTreeStats.java b/com.oracle.truffle.r.library/src/com/oracle/truffle/r/library/fastr/FastRTreeStats.java
index df195c3eb3..d7140ace2e 100644
--- a/com.oracle.truffle.r.library/src/com/oracle/truffle/r/library/fastr/FastRTreeStats.java
+++ b/com.oracle.truffle.r.library/src/com/oracle/truffle/r/library/fastr/FastRTreeStats.java
@@ -100,7 +100,7 @@ public abstract class FastRTreeStats extends RExternalBuiltinNode.Arg2 {
         String[] names = new String[functionCounts.size()];
         for (int i = 0; i < listData.length; i++) {
             SyntaxNodeCount snc = functionCounts.get(i);
-            listData[i] = RDataFactory.createIntVector(new int[]{snc.total(), snc.nonSyntaxNodeCount, snc.syntaxNodeCount}, RDataFactory.COMPLETE_VECTOR, COLNAMES);
+            listData[i] = RDataFactory.createIntVector(new int[]{snc.total(), snc.syntaxNodeCount, snc.nonSyntaxNodeCount}, RDataFactory.COMPLETE_VECTOR, COLNAMES);
             names[i] = snc.name();
         }
         return RDataFactory.createList(listData, RDataFactory.createStringVector(names, RDataFactory.COMPLETE_VECTOR));
diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/BasePackage.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/BasePackage.java
index 7e8fba0c33..b649ddd74a 100644
--- a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/BasePackage.java
+++ b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/BasePackage.java
@@ -461,7 +461,6 @@ public class BasePackage extends RBuiltinPackage {
         add(StandardGeneric.class, StandardGenericNodeGen::create);
         add(Stop.class, StopNodeGen::create);
         add(Strtoi.class, StrtoiNodeGen::create);
-        add(Structure.class, StructureNodeGen::create);
         add(Substitute.class, SubstituteNodeGen::create);
         add(Substr.class, SubstrNodeGen::create);
         add(Sum.class, SumNodeGen::create);
diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Structure.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Structure.java
deleted file mode 100644
index abd2a5d01a..0000000000
--- a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Structure.java
+++ /dev/null
@@ -1,157 +0,0 @@
-/*
- * Copyright (c) 2014, 2015, Oracle and/or its affiliates. All rights reserved.
- * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
- *
- * This code is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 only, as
- * published by the Free Software Foundation.
- *
- * This code 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
- * version 2 for more details (a copy is included in the LICENSE file that
- * accompanied this code).
- *
- * You should have received a copy of the GNU General Public License version
- * 2 along with this work; if not, write to the Free Software Foundation,
- * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
- *
- * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
- * or visit www.oracle.com if you need additional information or have any
- * questions.
- */
-package com.oracle.truffle.r.nodes.builtin.base;
-
-import static com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
-import static com.oracle.truffle.r.runtime.RBuiltinKind.*;
-
-import com.oracle.truffle.api.CompilerDirectives;
-import com.oracle.truffle.api.dsl.*;
-import com.oracle.truffle.api.profiles.ConditionProfile;
-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.REnvironment;
-
-/**
- * A temporary substitution to work around bug with {@code list(...)} used in R version.
- */
-@RBuiltin(name = "structure", kind = SUBSTITUTE, parameterNames = {".Data", "..."})
-public abstract class Structure extends RBuiltinNode {
-    private final ConditionProfile instanceOfStringProfile = ConditionProfile.createBinaryProfile();
-    private final RAttributeProfiles attrProfiles = RAttributeProfiles.create();
-
-    @SuppressWarnings("unused")
-    @Specialization
-    protected Object structure(RMissing obj, RMissing args) {
-        CompilerDirectives.transferToInterpreter();
-        throw RError.error(this, RError.Message.ARGUMENT_MISSING, ".Data");
-    }
-
-    @Specialization
-    protected Object structure(RAbstractContainer obj, @SuppressWarnings("unused") RMissing args) {
-        return obj;
-    }
-
-    private static String fixupAttrName(String s) {
-        // as per documentation of the "structure" function
-        if (s.equals(".Dim")) {
-            return "dim";
-        } else if (s.equals(".Dimnames")) {
-            return "dimnames";
-        } else if (s.equals(".Names")) {
-            return "names";
-        } else if (s.equals(".Tsp")) {
-            return "tsp";
-        } else if (s.equals(".Label")) {
-            return "levels";
-        } else {
-            return s.intern();
-        }
-    }
-
-    @Specialization
-    protected Object structure(RFunction obj, RArgsValuesAndNames args) {
-        return doStructure(obj, args);
-    }
-
-    @Specialization
-    protected Object structure(REnvironment obj, RArgsValuesAndNames args) {
-        return doStructure(obj, args);
-    }
-
-    @Specialization
-    protected Object structure(RSymbol obj, RArgsValuesAndNames args) {
-        return doStructure(obj, args);
-    }
-
-    @Specialization
-    protected Object structure(RExternalPtr obj, RArgsValuesAndNames args) {
-        return doStructure(obj, args);
-    }
-
-    @Specialization
-    protected Object structure(RAbstractContainer obj, RArgsValuesAndNames args) {
-        RAttributable res = obj;
-        // TODO: should we consider storing attributes with sequences?
-        if (res instanceof RSequence) {
-            res = ((RSequence) res).createVector();
-        }
-        return doStructure(res, args);
-    }
-
-    // Ideally we would only need this as a specialization, but then we fail due to
-    // lack of auto-translation of, e.g. Double to RAbstractContainer.
-    @TruffleBoundary
-    protected Object doStructure(RAttributable obj, RArgsValuesAndNames args) {
-        Object[] values = args.getArguments();
-        ArgumentsSignature signature = getSuppliedSignature();
-        validateArgNames(signature);
-        RAttributable res = obj;
-        for (int i = 0; i < values.length; i++) {
-            Object value = fixupValue(values[i]);
-            String attrName = fixupAttrName(signature.getName(i + 1));
-            if (attrName.equals(RRuntime.CLASS_ATTR_KEY)) {
-                if (value == RNull.instance) {
-                    res = res.setClassAttr(null, true);
-                } else {
-                    res = res.setClassAttr((RStringVector) value, true);
-                }
-            } else {
-                if (value == RNull.instance) {
-                    res.removeAttr(attrProfiles, attrName);
-                } else {
-                    res.setAttr(attrName, value);
-                }
-            }
-        }
-        return res;
-    }
-
-    private Object fixupValue(Object value) {
-        if (instanceOfStringProfile.profile(value instanceof String)) {
-            return RDataFactory.createStringVectorFromScalar((String) value);
-        } else {
-            return value;
-        }
-    }
-
-    @TruffleBoundary
-    private void validateArgNames(ArgumentsSignature signature) {
-        int containerIndex = 0;
-        if (findNullIn(signature, containerIndex + 1)) {
-            throw RError.error(this, RError.Message.ATTRIBUTES_NAMED);
-        }
-    }
-
-    @TruffleBoundary
-    private static boolean findNullIn(ArgumentsSignature signature, int startIndex) {
-        for (int i = startIndex; i < signature.getLength(); i++) {
-            if (signature.getName(i) == null) {
-                return true;
-            }
-        }
-        return false;
-    }
-}
diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/UpdateAttributes.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/UpdateAttributes.java
index 9acfb7ef6f..9fd199f43e 100644
--- a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/UpdateAttributes.java
+++ b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/UpdateAttributes.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 2016, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -41,7 +41,6 @@ import com.oracle.truffle.r.runtime.RError;
 import com.oracle.truffle.r.runtime.RRuntime;
 import com.oracle.truffle.r.runtime.data.RAttributable;
 import com.oracle.truffle.r.runtime.data.RAttributeProfiles;
-import com.oracle.truffle.r.runtime.data.RDataFactory;
 import com.oracle.truffle.r.runtime.data.RList;
 import com.oracle.truffle.r.runtime.data.RNull;
 import com.oracle.truffle.r.runtime.data.RShareable;
@@ -233,11 +232,11 @@ public abstract class UpdateAttributes extends RInvisibleBuiltinNode {
                         throw RError.error(this, RError.Message.ATTRIBUTES_NAMED);
                     }
                     if (attrName.equals(RRuntime.CLASS_ATTR_KEY)) {
-                        Object attrValue = RRuntime.asString(list.getDataAt(i));
+                        Object attrValue = list.getDataAt(i);
                         if (attrValue == null) {
                             throw RError.error(this, RError.Message.SET_INVALID_CLASS_ATTR);
                         }
-                        attrObj.setClassAttr(RDataFactory.createStringVectorFromScalar((String) attrValue), false);
+                        attrObj.setClassAttr(UpdateAttr.convertClassAttrFromObject(attrValue), false);
                     } else {
                         attrObj.setAttr(attrName, list.getDataAt(i));
                     }
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 749204b562..cfeb5ab385 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
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 2016, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -80,7 +80,7 @@ public abstract class UpdateNames extends RInvisibleBuiltinNode {
         String[] names = new String[result.getLength()];
         Arrays.fill(names, RRuntime.STRING_NA);
         names[0] = name;
-        RStringVector namesVector = RDataFactory.createStringVector(names, names.length <= 1);
+        RStringVector namesVector = RDataFactory.createStringVector(names, names.length <= 1 && names[0] != RRuntime.STRING_NA);
         result.setNames(namesVector);
         return result;
     }
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RFactor.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RFactor.java
index 757bbe0875..70dbd6f8b4 100644
--- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RFactor.java
+++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RFactor.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2016, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -239,7 +239,13 @@ public final class RFactor implements RShareable, RAbstractContainer {
     }
 
     public RVector getLevels(RAttributeProfiles attrProfiles) {
-        return (RVector) vector.getAttr(attrProfiles, RRuntime.LEVELS_ATTR_KEY);
+        Object attr = vector.getAttr(attrProfiles, RRuntime.LEVELS_ATTR_KEY);
+        if (attr instanceof RVector) {
+            return (RVector) attr;
+        } else {
+            // Scalar, must convert
+            return (RVector) RRuntime.asAbstractVector(attr);
+        }
     }
 
     public int getNLevels(RAttributeProfiles attrProfiles) {
-- 
GitLab