From e1635e5914252c6bc58d8aa63c5dc67b5bf9f87d Mon Sep 17 00:00:00 2001
From: stepan <stepan.sindelar@oracle.com>
Date: Tue, 23 Jan 2018 13:42:56 +0100
Subject: [PATCH] Fix options built-in: named list is used as option value
---
.../nodes/builtin/base/OptionsFunctions.java | 78 ++++++++-----------
.../truffle/r/test/ExpectedTestOutput.test | 26 +++++++
.../r/test/builtins/TestBuiltin_options.java | 4 +-
3 files changed, 62 insertions(+), 46 deletions(-)
diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/OptionsFunctions.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/OptionsFunctions.java
index 747c673fc0..53722a742e 100644
--- a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/OptionsFunctions.java
+++ b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/OptionsFunctions.java
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2014, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2018, 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
@@ -29,11 +29,11 @@ import static com.oracle.truffle.r.runtime.builtins.RBehavior.READS_STATE;
import static com.oracle.truffle.r.runtime.builtins.RBuiltinKind.INTERNAL;
import java.text.Collator;
+import java.util.ArrayList;
import java.util.Arrays;
import java.util.Comparator;
import java.util.Locale;
import java.util.Map;
-import java.util.Map.Entry;
import java.util.Set;
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
@@ -48,6 +48,7 @@ import com.oracle.truffle.r.runtime.RError;
import com.oracle.truffle.r.runtime.RError.Message;
import com.oracle.truffle.r.runtime.RLocale;
import com.oracle.truffle.r.runtime.ROptions;
+import com.oracle.truffle.r.runtime.ROptions.ContextStateImpl;
import com.oracle.truffle.r.runtime.ROptions.OptionsException;
import com.oracle.truffle.r.runtime.builtins.RBuiltin;
import com.oracle.truffle.r.runtime.context.RContext;
@@ -134,12 +135,12 @@ public class OptionsFunctions {
ROptions.ContextStateImpl options = RContext.getInstance().stateROptions;
Object[] values = args.getArguments();
ArgumentsSignature signature = args.getSignature();
- Object[] data = new Object[values.length];
- String[] names = new String[values.length];
+ ArrayList<Object> data = new ArrayList<>(values.length);
+ ArrayList<String> names = new ArrayList<>(values.length);
for (int i = 0; i < values.length; i++) {
String argName = signature.getName(i);
Object value = values[i];
- if (argNameNull.profile(argName == null || value instanceof RList)) {
+ if (argNameNull.profile(argName == null)) {
// getting
String optionName = null;
if (value instanceof RStringVector) {
@@ -149,63 +150,50 @@ public class OptionsFunctions {
optionName = (String) value;
} else if (value instanceof RList) {
// setting
- RList list = (RList) value;
- RStringVector thisListnames = null;
- Object nn = list.getNames();
- if (nn instanceof RStringVector) {
- thisListnames = (RStringVector) nn;
- } else {
- throw RError.error(RError.SHOW_CALLER, Message.LIST_NO_VALID_NAMES);
- }
- Object[] listData = new Object[list.getLength()];
- String[] listNames = new String[listData.length];
- for (int j = 0; j < listData.length; j++) {
- String name = thisListnames.getDataAt(j);
- Object previousVal = options.getValue(name);
- listData[j] = previousVal == null ? RNull.instance : previousVal;
- listNames[j] = name;
- options.setValue(name, list.getDataAtAsObject(j));
- }
+ // named lists are set the "as-is", which makes the options hierarchical
+ // not named list is un-listed and each value (with its name) is used as
+ // "top-level" option
+ handleUnnamedList(data, names, (RList) value, options);
// any settings means result is invisible
visible = false;
- // if this is the only argument, no need to copy, can just return
- if (values.length == 1) {
- data = listData;
- names = listNames;
- break;
- } else {
- // resize and copy
- int newSize = values.length - 1 + listData.length;
- Object[] newData = new Object[newSize];
- String[] newNames = new String[newSize];
- System.arraycopy(data, 0, newData, 0, i);
- System.arraycopy(names, 0, newNames, 0, i);
- System.arraycopy(listData, 0, newData, i, listData.length);
- System.arraycopy(listNames, 0, newNames, i, listNames.length);
- data = newData;
- names = newNames;
- }
} else {
throw error(Message.INVALID_UNNAMED_ARGUMENT);
}
Object optionVal = options.getValue(optionName);
- data[i] = optionVal == null ? RNull.instance : optionVal;
- names[i] = optionName;
+ data.add(optionVal == null ? RNull.instance : optionVal);
+ names.add(optionName);
} else {
// setting
Object previousVal = options.getValue(argName);
- data[i] = previousVal == null ? RNull.instance : previousVal;
- names[i] = argName;
+ data.add(previousVal == null ? RNull.instance : previousVal);
+ names.add(argName);
options.setValue(argName, value);
// any settings means result is invisible
visible = false;
}
}
- RList result = RDataFactory.createList(data, RDataFactory.createStringVector(names, RDataFactory.COMPLETE_VECTOR));
+ RList result = RDataFactory.createList(data.toArray(), RDataFactory.createStringVector(names.toArray(new String[names.size()]), RDataFactory.COMPLETE_VECTOR));
return new ResultWithVisibility(result, visible);
}
- protected boolean isMissing(RArgsValuesAndNames args) {
+ private static void handleUnnamedList(ArrayList<Object> data, ArrayList<String> names, RList list, ContextStateImpl options) throws OptionsException {
+ RStringVector thisListnames;
+ Object nn = list.getNames();
+ if (nn instanceof RStringVector) {
+ thisListnames = (RStringVector) nn;
+ } else {
+ throw RError.error(RError.SHOW_CALLER, Message.LIST_NO_VALID_NAMES);
+ }
+ for (int j = 0; j < list.getLength(); j++) {
+ String name = thisListnames.getDataAt(j);
+ Object previousVal = options.getValue(name);
+ data.add(previousVal == null ? RNull.instance : previousVal);
+ names.add(name);
+ options.setValue(name, list.getDataAtAsObject(j));
+ }
+ }
+
+ boolean isMissing(RArgsValuesAndNames args) {
return args.isEmpty(); // length() == 1 && args.getValue(0) == RMissing.instance;
}
}
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 1f503b574c..34a016ec0b 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
@@ -46482,6 +46482,32 @@ $width
[1] 80
+##com.oracle.truffle.r.test.builtins.TestBuiltin_options.testOptions#
+#{ options(list(aaa = 42, bbb = 'hello')); x <- options('aaa', 'bbb'); options(aaa=NULL, bbb=NULL); x }
+$aaa
+[1] 42
+
+$bbb
+[1] "hello"
+
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_options.testOptions#
+#{ options(lll = list(aaa = 42, bbb = 'hello')); x <- options('lll', 'aaa', 'bbb'); options(aaa=NULL, bbb=NULL, lll=NULL); x }
+$lll
+$lll$aaa
+[1] 42
+
+$lll$bbb
+[1] "hello"
+
+
+$aaa
+NULL
+
+$bbb
+NULL
+
+
##com.oracle.truffle.r.test.builtins.TestBuiltin_options.testOptions#
#{ options(options(digits = 5)) }
diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_options.java b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_options.java
index 63411ea2a1..0ec78e5583 100644
--- a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_options.java
+++ b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_options.java
@@ -4,7 +4,7 @@
* http://www.gnu.org/licenses/gpl-2.0.html
*
* Copyright (c) 2014, Purdue University
- * Copyright (c) 2014, 2017, Oracle and/or its affiliates
+ * Copyright (c) 2014, 2018, Oracle and/or its affiliates
*
* All rights reserved.
*/
@@ -55,6 +55,8 @@ public class TestBuiltin_options extends TestBase {
assertEval("{ getOption(character()) }");
assertEval("{ options(\"timeout\", \"width\") }");
assertEval("{ options(options(digits = 5)) }");
+ assertEval("{ options(list(aaa = 42, bbb = 'hello')); x <- options('aaa', 'bbb'); options(aaa=NULL, bbb=NULL); x }");
+ assertEval("{ options(lll = list(aaa = 42, bbb = 'hello')); x <- options('lll', 'aaa', 'bbb'); options(aaa=NULL, bbb=NULL, lll=NULL); x }");
}
@Test
--
GitLab