[GR-14264] Use Truffle Options stability.

steve-s · steve-s · commit 095af90da1e8 · 2019-03-06T13:12:26.000-08:00
PullRequest: fastr/1938
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,9 @@
 # 1.0 RC 14
 
+* all FastR specific options (NOT those GNU-R compatible like `--save`) are experimental except for `--R.PrintErrorStacktracesToFile`,
+which is an option that enables logging of FastR internal errors for bug reporting purposes. Experimental options can be unlocked
+using `--experimental-options` or with `ContextBuilder#allowExperimentalOptions`.
+
 Bug fixes:
 
 * `C_numeric_deriv` gives wrong results of gradient #54
diff --git a/ci_common/common.hocon b/ci_common/common.hocon
@@ -104,6 +104,7 @@ common : ${java8Downloads} {
     PKG_TEST_ENV_miniUI : "LC_ALL=C"
     PKG_TEST_ENV_compare : "LC_ALL=C"
     FASTR_OPTION_PrintErrorStacktracesToFile: "true"
+    GRAALVM_CHECK_EXPERIMENTAL_OPTIONS : "true"
   }
 }
 
diff --git a/com.oracle.truffle.r.launcher/src/com/oracle/truffle/r/launcher/RMain.java b/com.oracle.truffle.r.launcher/src/com/oracle/truffle/r/launcher/RMain.java
@@ -171,26 +171,26 @@ protected void validateArguments(Map<String, String> polyglotOptions) {
     }
 
     @Override
-    protected void launch(Builder contextBuilder) {
+    protected void launch(Builder contextBuilderIn) {
         StartupTiming.timestamp("RMain.launch");
         assert client != null;
         if (rArguments == null) {
             // validateArguments did not set the value
             return;
         }
         this.consoleHandler = ConsoleHandler.createConsoleHandler(options, null, inStream, outStream);
-        Builder contextBuilderAllowAll = contextBuilder.allowAllAccess(true);
-        if (ignoreJvmArguments) {
-            contextBuilderAllowAll = contextBuilderAllowAll.allowHostAccess(useJVM);
+        Builder contextBuilder = contextBuilderIn;
+        if (!ignoreJvmArguments) {
+            contextBuilder = contextBuilder.allowHostAccess(useJVM);
         }
 
         Context context;
         String tracedLibs = System.getenv("DEBUG_LLVM_LIBS");
         if (tracedLibs != null) {
-            context = preparedContext = contextBuilderAllowAll.option("inspect", "true").option("llvm.enableLVI", "true").option("llvm.llDebug", "true").arguments("R", rArguments).in(
+            context = preparedContext = contextBuilder.option("inspect", "true").option("llvm.enableLVI", "true").option("llvm.llDebug", "true").arguments("R", rArguments).in(
                             consoleHandler.createInputStream()).out(outStream).err(errStream).build();
         } else {
-            context = preparedContext = contextBuilderAllowAll.arguments("R", rArguments).in(consoleHandler.createInputStream()).out(outStream).err(errStream).build();
+            context = preparedContext = contextBuilder.arguments("R", rArguments).in(consoleHandler.createInputStream()).out(outStream).err(errStream).build();
         }
 
         this.consoleHandler.setContext(context);
diff --git a/com.oracle.truffle.r.nodes.test/src/com/oracle/truffle/r/nodes/castsTests/CastBuilderTest.java b/com.oracle.truffle.r.nodes.test/src/com/oracle/truffle/r/nodes/castsTests/CastBuilderTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2019, 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
@@ -939,7 +939,7 @@ public void testCastForeignObjectDisabled() {
     }
 
     private static void execInContext(Runnable r) {
-        Context context = Context.newBuilder("R", "llvm").allowAllAccess(true).build();
+        Context context = Context.newBuilder("R", "llvm").allowExperimentalOptions(true).allowAllAccess(true).build();
         context.eval("R", "1"); // initialize context
         context.enter();
         try {
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/context/FastROptions.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/context/FastROptions.java
@@ -38,6 +38,7 @@
 import org.graalvm.options.OptionDescriptor;
 import org.graalvm.options.OptionDescriptors;
 import org.graalvm.options.OptionKey;
+import org.graalvm.options.OptionStability;
 import org.graalvm.options.OptionValues;
 
 /**
@@ -56,7 +57,7 @@ public class FastROptions {
 
     @Option(category = OptionCategory.INTERNAL, help = "Prints Java and R stack traces for all errors") //
     public static final OptionKey<Boolean> PrintErrorStacktraces = new OptionKey<>(false);
-    @Option(category = OptionCategory.USER, help = "Dumps Java and R stack traces to 'fastr_errors.log' for all errors") //
+    @Option(category = OptionCategory.USER, stability = OptionStability.STABLE, help = "Dumps Java and R stack traces to 'fastr_errors-{context ID}_{PID}.log' for all internal errors") //
     public static final OptionKey<Boolean> PrintErrorStacktracesToFile = new OptionKey<>(false);
 
     @Option(category = OptionCategory.INTERNAL, help = "Debug=name1,name2,...; Turn on debugging output for 'name1', 'name2', etc.")//
@@ -83,7 +84,7 @@ public class FastROptions {
     public static final OptionKey<Boolean> SearchPathForcePromises = new OptionKey<>(false);
     @Option(category = OptionCategory.EXPERT, help = "Load native code of packages, including builtin packages.") //
     public static final OptionKey<Boolean> LoadPackagesNativeCode = new OptionKey<>(!FastRConfig.ManagedMode);
-    @Option(category = OptionCategory.EXPERT, help = "Allow only one thread to enter packages' native code") //
+    @Option(category = OptionCategory.EXPERT, help = "Allow only one thread to enter native code of packages") //
     public static final OptionKey<Boolean> SynchronizeNativeCode = new OptionKey<>(true);
     // Promises optimizations
     @Option(category = OptionCategory.INTERNAL, help = "If enabled, overrides all other EagerEval switches (see EagerEvalHelper)") //
@@ -114,9 +115,9 @@ public class FastROptions {
     // Dicontinued since rc12
     // only a warning is printed to use the default logger mechaninsm
     // TODO remove at some later point
-    @Option(category = OptionCategory.INTERNAL, help = "Print FastR performance warning") //
+    @Option(category = OptionCategory.EXPERT, help = "Print FastR performance warning") //
     public static final OptionKey<Boolean> PerformanceWarnings = new OptionKey<>(false);
-    @Option(category = OptionCategory.INTERNAL, help = "Print a message for each non-trivial variable lookup")//
+    @Option(category = OptionCategory.EXPERT, help = "Print a message for each non-trivial variable lookup")//
     public static final OptionKey<Boolean> PrintComplexLookups = new OptionKey<>(false);
     @Option(category = OptionCategory.INTERNAL, help = "Trace all R function calls") //
     public static final OptionKey<Boolean> TraceCalls = new OptionKey<>(false);
diff --git a/com.oracle.truffle.r.test.packages/r/install.packages.R b/com.oracle.truffle.r.test.packages/r/install.packages.R
@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2015, 2019, 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
@@ -204,7 +204,7 @@ fastr.test.jvm.args <- function() {
             return (opts)
 	    }
     })
-    return ("'--R.IgnoreGraphicsCalls=true'") 
+    return ("'--experimental-options --R.IgnoreGraphicsCalls=true'")
 }
 
 # returns a vector of package names that are the direct dependents of pkg
diff --git a/com.oracle.truffle.r.test.packages/test.mx.args b/com.oracle.truffle.r.test.packages/test.mx.args
@@ -1,2 +1,3 @@
+--experimental-options
 --R.IgnoreGraphicsCalls
 --R.AdditionalOptions=vdiffr_skip:T;svUnit.recordAll:F;svUnit.minTiming:10000;crayon.enabled:F
diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/generate/FastRSession.java b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/generate/FastRSession.java
@@ -136,7 +136,7 @@ public Context createContext(ContextKind contextKind, boolean allowHostAccess) {
     }
 
     public static Context.Builder getContextBuilder(String... languages) {
-        Context.Builder builder = Context.newBuilder(languages);
+        Context.Builder builder = Context.newBuilder(languages).allowExperimentalOptions(true);
         setCLIOptions(builder);
         builder.allowAllAccess(true);
         // no point in printing errors to file when running tests (that contain errors on purpose)
diff --git a/documentation/dev/managed_ffi.md b/documentation/dev/managed_ffi.md
@@ -22,5 +22,5 @@ Note that boolean FastR options are prefixed with the language id "R" and passed
 aforementioned options:
 
 ```
-mx --J @'-Dfastr.rffi.factory.type=managed' r --R.LoadPackagesNativeCode=true --R.LoadProfiles=true
+mx --J @'-Dfastr.rffi.factory.type=managed' r --experimental-options --R.LoadPackagesNativeCode=true --R.LoadProfiles=true
 ```
diff --git a/mx.fastr/mx_fastr.py b/mx.fastr/mx_fastr.py
@@ -95,6 +95,7 @@ def do_run_r(args, command, extraVmArgs=None, jdk=None, **kwargs):
 
     vmArgs += set_graal_options()
     vmArgs += _sulong_options()
+    args = _sulong_args() + args
 
     if extraVmArgs is None or not '-da' in extraVmArgs:
         # unless explicitly disabled we enable assertion checking
@@ -148,6 +149,13 @@ def set_graal_options():
     else:
         return []
 
+def _sulong_args():
+    mx_sulong = mx.suite("sulong", fatalIfMissing=False)
+    if mx_sulong:
+        return ['--experimental-options']
+    else:
+        return []
+
 def _sulong_options():
     mx_sulong = mx.suite("sulong", fatalIfMissing=False)
     if mx_sulong:
diff --git a/mx.fastr/suite.py b/mx.fastr/suite.py
@@ -7,7 +7,7 @@
             {
                "name" : "truffle",
                "subdir" : True,
-               "version" : "963639061d3af9e33d343aa817490a4bdb3a0c91",
+               "version" : "bde4bb5a346590575848bb5d56f7d6e3431b13e4",
                "urls" : [
                     {"url" : "https://github.com/graalvm/graal", "kind" : "git"},
                     {"url" : "https://curio.ssw.jku.at/nexus/content/repositories/snapshots", "kind" : "binary"},

Original file line number	Diff line number	Diff line change
`@@ -104,6 +104,7 @@ common : ${java8Downloads} {`
`104`	`104`	`PKG_TEST_ENV_miniUI : "LC_ALL=C"`
`105`	`105`	`PKG_TEST_ENV_compare : "LC_ALL=C"`
`106`	`106`	`FASTR_OPTION_PrintErrorStacktracesToFile: "true"`
	`107`	`+ GRAALVM_CHECK_EXPERIMENTAL_OPTIONS : "true"`
`107`	`108`	`}`
`108`	`109`	`}`
`109`	`110`
Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`/*`
`2`		`- * Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.`
	`2`	`+ * Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved.`
`3`	`3`	`* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.`
`4`	`4`	`*`
`5`	`5`	`* This code is free software; you can redistribute it and/or modify it`
`@@ -939,7 +939,7 @@ public void testCastForeignObjectDisabled() {`
`939`	`939`	`}`
`940`	`940`
`941`	`941`	`private static void execInContext(Runnable r) {`
`942`		`- Context context = Context.newBuilder("R", "llvm").allowAllAccess(true).build();`
	`942`	`+ Context context = Context.newBuilder("R", "llvm").allowExperimentalOptions(true).allowAllAccess(true).build();`
`943`	`943`	`context.eval("R", "1"); // initialize context`
`944`	`944`	`context.enter();`
`945`	`945`	`try {`
Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`#`
`2`		`-# Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.`
	`2`	`+# Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved.`
`3`	`3`	`# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.`
`4`	`4`	`#`
`5`	`5`	`# This code is free software; you can redistribute it and/or modify it`
`@@ -204,7 +204,7 @@ fastr.test.jvm.args <- function() {`
`204`	`204`	`return (opts)`
`205`	`205`	`}`
`206`	`206`	`})`
`207`		`- return ("'--R.IgnoreGraphicsCalls=true'")`
	`207`	`+ return ("'--experimental-options --R.IgnoreGraphicsCalls=true'")`
`208`	`208`	`}`
`209`	`209`
`210`	`210`	`# returns a vector of package names that are the direct dependents of pkg`
Original file line number	Diff line number	Diff line change
`@@ -1,2 +1,3 @@`
	`1`	`+--experimental-options`
`1`	`2`	`--R.IgnoreGraphicsCalls`
`2`	`3`	`--R.AdditionalOptions=vdiffr_skip:T;svUnit.recordAll:F;svUnit.minTiming:10000;crayon.enabled:F`
Original file line number	Diff line number	Diff line change
`@@ -136,7 +136,7 @@ public Context createContext(ContextKind contextKind, boolean allowHostAccess) {`
`136`	`136`	`}`
`137`	`137`
`138`	`138`	`public static Context.Builder getContextBuilder(String... languages) {`
`139`		`- Context.Builder builder = Context.newBuilder(languages);`
	`139`	`+ Context.Builder builder = Context.newBuilder(languages).allowExperimentalOptions(true);`
`140`	`140`	`setCLIOptions(builder);`
`141`	`141`	`builder.allowAllAccess(true);`
`142`	`142`	`// no point in printing errors to file when running tests (that contain errors on purpose)`