[GR-12559] Clean-up FastROptions and hide internal errors logging behind an option.

PullRequest: fastr/1841

Author: steve-s

ci_common/common.hocon

@@ -90,6 +90,7 @@ common : ${java8Downloads} ${packagesLinux}  {
   environment : {
 	PKG_TEST_ENV_miniUI : "LC_ALL=C"
 	PKG_TEST_ENV_compare : "LC_ALL=C"
+	FASTR_OPTION_PrintErrorStacktracesToFile: "true"
   }
 }
 

com.oracle.truffle.r.engine/src/com/oracle/truffle/r/engine/REngine.java

@@ -641,8 +641,8 @@ public RNode getBody() {
 
     @TruffleBoundary
     private static boolean checkResult(Object result) {
-        if (FastROptions.CheckResultCompleteness.getBooleanValue() && result instanceof RAbstractVector) {
-            assert RAbstractVector.verify((RAbstractVector) result);
+        if (result instanceof RAbstractVector) {
+            return RAbstractVector.verify((RAbstractVector) result);
         }
         return true;
     }

com.oracle.truffle.r.engine/src/com/oracle/truffle/r/engine/RRuntimeASTAccessImpl.java

@@ -54,7 +54,6 @@
 import com.oracle.truffle.r.nodes.function.ClassHierarchyNode;
 import com.oracle.truffle.r.nodes.function.FunctionDefinitionNode;
 import com.oracle.truffle.r.nodes.function.RCallNode;
-import com.oracle.truffle.r.nodes.instrumentation.RInstrumentation;
 import com.oracle.truffle.r.nodes.instrumentation.RSyntaxTags.FunctionBodyBlockTag;
 import com.oracle.truffle.r.nodes.instrumentation.RSyntaxTags.LoopTag;
 import com.oracle.truffle.r.runtime.ArgumentsSignature;
@@ -489,11 +488,6 @@ public String encodeComplex(RComplex x, int digits) {
         return ComplexVectorPrinter.encodeComplex(x, digits);
     }
 
-    @Override
-    public void checkDebugRequest(RFunction func) {
-        RInstrumentation.checkDebugRequested(func);
-    }
-
     @Override
     public Class<? extends TruffleRLanguage> getTruffleRLanguage() {
         return TruffleRLanguageImpl.class;

com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/FunctionExpressionNode.java

@@ -33,7 +33,6 @@
 import com.oracle.truffle.r.nodes.function.PromiseHelperNode.PromiseDeoptimizeFrameNode;
 import com.oracle.truffle.r.nodes.function.opt.EagerEvalHelper;
 import com.oracle.truffle.r.nodes.function.visibility.SetVisibilityNode;
-import com.oracle.truffle.r.nodes.instrumentation.RInstrumentation;
 import com.oracle.truffle.r.runtime.ArgumentsSignature;
 import com.oracle.truffle.r.runtime.RArguments;
 import com.oracle.truffle.r.runtime.data.RDataFactory;
@@ -82,9 +81,7 @@ public RFunction execute(VirtualFrame frame) {
             }
             initialized = true;
         }
-        RFunction func = RDataFactory.createFunction(RFunction.NO_NAME, RFunction.NO_NAME, callTarget, null, matFrame);
-        RInstrumentation.checkDebugRequested(func);
-        return func;
+        return RDataFactory.createFunction(RFunction.NO_NAME, RFunction.NO_NAME, callTarget, null, matFrame);
     }
 
     public RootCallTarget getCallTarget() {

com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/instrumentation/RInstrumentation.java

@@ -22,7 +22,6 @@
  */
 package com.oracle.truffle.r.nodes.instrumentation;
 
-import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
 import com.oracle.truffle.api.instrumentation.Instrumenter;
 import com.oracle.truffle.api.instrumentation.SourceSectionFilter;
 import com.oracle.truffle.api.instrumentation.SourceSectionFilter.IndexRange;
@@ -31,7 +30,6 @@
 import com.oracle.truffle.api.source.SourceSection;
 import com.oracle.truffle.r.nodes.function.FunctionDefinitionNode;
 import com.oracle.truffle.r.nodes.instrumentation.RSyntaxTags.FunctionBodyBlockTag;
-import com.oracle.truffle.r.runtime.FastROptions;
 import com.oracle.truffle.r.runtime.RSource;
 import com.oracle.truffle.r.runtime.context.RContext;
 import com.oracle.truffle.r.runtime.data.RFunction;
@@ -43,12 +41,6 @@
  */
 public class RInstrumentation {
 
-    /**
-     * The function names that were requested to be used in implicit {@code debug(f)} calls, when
-     * those functions are defined. Global to all contexts.
-     */
-    @CompilationFinal(dimensions = 1) private static String[] debugFunctionNames;
-
     public static FunctionDefinitionNode getFunctionDefinitionNode(RFunction func) {
         assert !func.isBuiltin();
         return (FunctionDefinitionNode) func.getRootNode();
@@ -58,30 +50,18 @@ public static SourceSection getSourceSection(RFunction func) {
         return func.getRootNode().getSourceSection();
     }
 
-    /**
-     * Create a filter that matches all the statement nodes in {@code func}.
-     */
-    static SourceSectionFilter.Builder createFunctionStatementFilter(RFunction func) {
-        return createFunctionFilter(func, StandardTags.StatementTag.class);
-    }
-
     public static SourceSectionFilter.Builder createFunctionStatementFilter(FunctionDefinitionNode fdn) {
         return createFunctionFilter(fdn, StandardTags.StatementTag.class);
     }
 
-    static SourceSectionFilter.Builder createFunctionFilter(RFunction func, Class<?> tag) {
-        FunctionDefinitionNode fdn = getFunctionDefinitionNode(func);
-        return createFunctionFilter(fdn, tag);
-    }
-
     public static SourceSectionFilter.Builder createFunctionFilter(FunctionDefinitionNode fdn, Class<?> tag) {
         /* Filter needs to check for statement tags in the range of the function in the Source */
         SourceSectionFilter.Builder builder = SourceSectionFilter.newBuilder();
         builder.tagIs(tag);
         SourceSection fdns = fdn.getSourceSection();
         builder.indexIn(fdns.getCharIndex(), fdns.getCharLength());
         Source source = fdns.getSource();
-        builder.sourceIs(s -> source.equals(s));
+        builder.sourceIs(source::equals);
         return builder;
     }
 
@@ -113,26 +93,11 @@ public static SourceSectionFilter.Builder createFunctionStartFilter(RFunction fu
      * global (command-line) options for tracing and timing. They are applied to every context.
      */
     public static void activate(@SuppressWarnings("unused") RContext context) {
-        String rdebugValue = FastROptions.Rdebug.getStringValue();
-        if (rdebugValue != null) {
-            debugFunctionNames = rdebugValue.split(",");
-        }
         // Check for function tracing
         RContext.getRRuntimeASTAccess().traceAllFunctions();
     }
 
     public static Instrumenter getInstrumenter() {
         return RContext.getInstance().getInstrumentationState().getInstrumenter();
     }
-
-    public static void checkDebugRequested(RFunction func) {
-        if (debugFunctionNames != null) {
-            FunctionDefinitionNode fdn = (FunctionDefinitionNode) func.getRootNode();
-            for (String debugFunctionName : debugFunctionNames) {
-                if (debugFunctionName.equals(fdn.toString())) {
-                    RContext.getRRuntimeASTAccess().enableDebug(func, false);
-                }
-            }
-        }
-    }
 }

com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/FastROptions.java

@@ -38,8 +38,8 @@
  */
 public enum FastROptions {
     PrintErrorStacktraces("Prints Java and R stack traces for all errors", false),
-    PrintErrorStacktracesToFile("Dumps Java and R stack traces to 'fastr_errors.log' for all errors", true),
-    CheckResultCompleteness("Assert completeness of results vectors after evaluating unit tests and R shell commands", true),
+    PrintErrorStacktracesToFile("Dumps Java and R stack traces to 'fastr_errors.log' for all errors", false),
+    // TODO: this should be logging category
     Debug("Debug=name1,name2,...; Turn on debugging output for 'name1', 'name2', etc.", null, true),
     TraceCalls("Trace all R function calls", false),
     TraceCallsToFile("TraceCalls output is sent to 'fastr_tracecalls.log'", false),
@@ -50,15 +50,14 @@ public enum FastROptions {
     PrintComplexLookups("Print a message for each non-trivial variable lookup", false),
     FullPrecisionSum("Use 128 bit arithmetic in sum builtin", false),
     InvisibleArgs("Argument writes do not trigger state transitions", true),
-    RefCountIncrementOnly("Disable reference count decrements for experimental state transition implementation", false),
+    RefCountIncrementOnly("Disable reference count decrements", false),
     UseInternalGridGraphics("Whether the internal (Java) grid graphics implementation should be used", true),
     UseSpecials("Whether the fast-path special call nodes should be created for simple enough arguments.", true),
     ForceSources("Generate source sections for unserialized code", false),
     SharedContexts("Whether all child contexts are to be shared contexts", true),
     SearchPathForcePromises("Whether all promises for frames on shared path are forced in presence of shared contexts", false),
     LoadPackagesNativeCode("Load native code of packages, including builtin packages.", !FastRConfig.ManagedMode),
     SynchronizeNativeCode("allow only one thread to enter packages' native code", false),
-    ForeignObjectWrappers("use wrappers for foreign objects (as opposed to full conversion)", true),
 
     // Promises optimizations
     EagerEval("If enabled, overrides all other EagerEval switches (see EagerEvalHelper)", false),
@@ -74,8 +73,7 @@ public enum FastROptions {
 
     IgnoreGraphicsCalls("Silently ignore unimplemented functions from graphics package", false),
     AdditionalOptions("List of R level options default values. Syntax: 'optionName:value;optionName2:value;'. " +
-                    "Value can be 'T' or 'F' in which case it is interpreted as boolean, otherwise as string", "", true),
-    StartupTiming("Records and prints various timestamps during initialization", false);
+                    "Value can be 'T' or 'F' in which case it is interpreted as boolean, otherwise as string", "", true);
 
     /**
      * Setting this environment variable activates the tracing of the bitcode of selected LLVM

com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/RInternalError.java

@@ -213,7 +213,8 @@ private static void reportError(String errMsg, Throwable throwable, int contextI
                         RSuicide.rSuicide("FastR internal error");
                     }
                 } else {
-                    message += ".";
+                    message += ". You can rerun FastR with --jvm.DR:+" + FastROptions.PrintErrorStacktracesToFile.name() +
+                                    " to turn on internal errors logging. Please attach the log file to the issue if possible.";
                 }
                 if (!FastROptions.PrintErrorStacktraces.getBooleanValue() && !FastROptions.PrintErrorStacktracesToFile.getBooleanValue()) {
                     System.err.println(message);

com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/RRuntimeASTAccess.java

@@ -159,8 +159,6 @@ default String getCallerSource(RCaller caller) {
 
     String encodeComplex(RComplex x, int digits);
 
-    void checkDebugRequest(RFunction func);
-
     RAbstractStringVector getClassHierarchy(RAttributable value);
 
     RContext getCurrentContext();

com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/RSerialize.java

@@ -2620,8 +2620,6 @@ public static RFunction processFunction(Object car, Object cdr, REnvironment env
             FrameSlotChangeMonitor.initializeEnclosingFrame(callTarget.getRootNode().getFrameDescriptor(), enclosingFrame);
             RFunction func = RDataFactory.createFunction(functionName, packageName, callTarget, null, enclosingFrame);
 
-            RContext.getRRuntimeASTAccess().checkDebugRequest(func);
-
             /*
              * TODO: this is missing the code that registers sources with RPackageSource!
              */