Add EchoCommentCommands to CommandInterpreterRunOptions in addition to the existing EchoCommands and expose both as interpreter settings.

Summary:
Add settings to control command echoing:
```
(lldb) settings set interpreter.echo-commands true
(lldb) settings set interpreter.echo-comment-commands true
```

Both settings default to true, which keeps LLDB's existing behavior in non-interactive mode (echo all command inputs to the output).

So far the only way to change this behavior was the `--source-quietly` flag, which disables all output including evaluation results.
Now `echo-commands` allows to turn off echoing for commands, while evaluation results are still printed. No effect if `--source-quietly` was present.
`echo-comment-commands` allows to turn off echoing for commands in case they are pure comment lines. No effect if `echo-commands` is false.

Note that the behavior does not change immediately! The new settings take effect only with the next command source.

LLDB lit test are the main motivation for this feature. So far incoming `#CHECK` line have always been echoed to the output and so they could never fail. Now we can disable it in lit-lldb-init.
Todos: Finish test for this feature. Add to lit-lldb-init. Check for failing lit tests.

Reviewers: aprantl, jasonmolenda, JDevlieghere

Subscribers: friss, lldb-commits

Differential Revision: https://reviews.llvm.org/D52788

llvm-svn: 343859

lldb/include/lldb/API/SBCommandInterpreter.h

@@ -45,6 +45,10 @@ public:
 
   void SetEchoCommands(bool);
 
+  bool GetEchoCommentCommands() const;
+
+  void SetEchoCommentCommands(bool echo);
+
   bool GetPrintResults() const;
 
   void SetPrintResults(bool);

lldb/include/lldb/Interpreter/CommandInterpreter.h

@@ -35,55 +35,60 @@ namespace lldb_private {
 class CommandInterpreterRunOptions {
 public:
   //------------------------------------------------------------------
-  /// Construct a CommandInterpreterRunOptions object.
-  /// This class is used to control all the instances where we run multiple
-  /// commands, e.g.
+  /// Construct a CommandInterpreterRunOptions object. This class is used to
+  /// control all the instances where we run multiple commands, e.g.
   /// HandleCommands, HandleCommandsFromFile, RunCommandInterpreter.
+  ///
   /// The meanings of the options in this object are:
   ///
   /// @param[in] stop_on_continue
-  ///    If \b true execution will end on the first command that causes the
-  ///    process in the
-  ///    execution context to continue.  If \false, we won't check the execution
-  ///    status.
+  ///    If \b true, execution will end on the first command that causes the
+  ///    process in the execution context to continue. If \b false, we won't
+  ///    check the execution status.
   /// @param[in] stop_on_error
-  ///    If \b true execution will end on the first command that causes an
+  ///    If \b true, execution will end on the first command that causes an
   ///    error.
   /// @param[in] stop_on_crash
-  ///    If \b true when a command causes the target to run, and the end of the
-  ///    run is a
-  ///    signal or exception, stop executing the commands.
+  ///    If \b true, when a command causes the target to run, and the end of the
+  ///    run is a signal or exception, stop executing the commands.
   /// @param[in] echo_commands
-  ///    If \b true echo the command before executing it.  If \false, execute
+  ///    If \b true, echo the command before executing it. If \b false, execute
   ///    silently.
+  /// @param[in] echo_comments
+  ///    If \b true, echo command even if it is a pure comment line. If
+  ///    \b false, print no ouput in this case. This setting has an effect only
+  ///    if \param echo_commands is \b true.
   /// @param[in] print_results
-  ///    If \b true print the results of the command after executing it.  If
-  ///    \false, execute silently.
+  ///    If \b true print the results of the command after executing it. If
+  ///    \b false, execute silently.
   /// @param[in] add_to_history
-  ///    If \b true add the commands to the command history.  If \false, don't
+  ///    If \b true add the commands to the command history. If \b false, don't
   ///    add them.
   //------------------------------------------------------------------
   CommandInterpreterRunOptions(LazyBool stop_on_continue,
                                LazyBool stop_on_error, LazyBool stop_on_crash,
-                               LazyBool echo_commands, LazyBool print_results,
-                               LazyBool add_to_history)
+                               LazyBool echo_commands, LazyBool echo_comments,
+                               LazyBool print_results, LazyBool add_to_history)
       : m_stop_on_continue(stop_on_continue), m_stop_on_error(stop_on_error),
         m_stop_on_crash(stop_on_crash), m_echo_commands(echo_commands),
-        m_print_results(print_results), m_add_to_history(add_to_history) {}
+        m_echo_comment_commands(echo_comments), m_print_results(print_results),
+        m_add_to_history(add_to_history) {}
 
   CommandInterpreterRunOptions()
       : m_stop_on_continue(eLazyBoolCalculate),
         m_stop_on_error(eLazyBoolCalculate),
         m_stop_on_crash(eLazyBoolCalculate),
         m_echo_commands(eLazyBoolCalculate),
+        m_echo_comment_commands(eLazyBoolCalculate),
         m_print_results(eLazyBoolCalculate),
         m_add_to_history(eLazyBoolCalculate) {}
 
   void SetSilent(bool silent) {
     LazyBool value = silent ? eLazyBoolNo : eLazyBoolYes;
 
-    m_echo_commands = value;
     m_print_results = value;
+    m_echo_commands = value;
+    m_echo_comment_commands = value;
     m_add_to_history = value;
   }
   // These return the default behaviors if the behavior is not
@@ -97,7 +102,7 @@ public:
     m_stop_on_continue = stop_on_continue ? eLazyBoolYes : eLazyBoolNo;
   }
 
-  bool GetStopOnError() const { return DefaultToNo(m_stop_on_continue); }
+  bool GetStopOnError() const { return DefaultToNo(m_stop_on_error); }
 
   void SetStopOnError(bool stop_on_error) {
     m_stop_on_error = stop_on_error ? eLazyBoolYes : eLazyBoolNo;
@@ -115,6 +120,14 @@ public:
     m_echo_commands = echo_commands ? eLazyBoolYes : eLazyBoolNo;
   }
 
+  bool GetEchoCommentCommands() const {
+    return DefaultToYes(m_echo_comment_commands);
+  }
+
+  void SetEchoCommentCommands(bool echo_comments) {
+    m_echo_comment_commands = echo_comments ? eLazyBoolYes : eLazyBoolNo;
+  }
+
   bool GetPrintResults() const { return DefaultToYes(m_print_results); }
 
   void SetPrintResults(bool print_results) {
@@ -131,6 +144,7 @@ public:
   LazyBool m_stop_on_error;
   LazyBool m_stop_on_crash;
   LazyBool m_echo_commands;
+  LazyBool m_echo_comment_commands;
   LazyBool m_print_results;
   LazyBool m_add_to_history;
 
@@ -459,6 +473,12 @@ public:
 
   void SetPromptOnQuit(bool b);
 
+  bool GetEchoCommands() const;
+  void SetEchoCommands(bool b);
+
+  bool GetEchoCommentCommands() const;
+  void SetEchoCommentCommands(bool b);
+
   //------------------------------------------------------------------
   /// Specify if the command interpreter should allow that the user can
   /// specify a custom exit code when calling 'quit'.
@@ -542,6 +562,9 @@ private:
   // An interruptible wrapper around the stream output
   void PrintCommandOutput(Stream &stream, llvm::StringRef str);
 
+  bool EchoCommandNonInteractive(llvm::StringRef line,
+                                 const Flags &io_handler_flags) const;
+
   // A very simple state machine which models the command handling transitions
   enum class CommandHandlingState {
     eIdle,

lldb/lit/Settings/Inputs/EchoCommandsAll.out

@@ -0,0 +1,6 @@
+# CHECK: (lldb) command source -s {{.*\n}}
+# CHECK: (lldb) command source -s {{.*\n}}
+# CHECK-NEXT: Executing commands in {{.*\n}}
+# CHECK-NEXT: Evaluate expr 1+2
+# CHECK-NEXT: expr 1+2
+# CHECK-NEXT: (int) $0 = 3

lldb/lit/Settings/Inputs/EchoCommandsNoComments.out

@@ -0,0 +1,5 @@
+# CHECK: (lldb) command source -s {{.*\n}}
+# CHECK: (lldb) command source -s {{.*\n}}
+# CHECK-NEXT: Executing commands in {{.*\n}}
+# CHECK-NEXT: expr 1+2
+# CHECK-NEXT: (int) $0 = 3

lldb/lit/Settings/Inputs/EchoCommandsNone.out

@@ -0,0 +1,4 @@
+# CHECK: (lldb) command source -s {{.*\n}}
+# CHECK: (lldb) command source -s {{.*\n}}
+# CHECK-NEXT: Executing commands in {{.*\n}}
+# CHECK-NEXT: (int) $0 = 3

lldb/lit/Settings/Inputs/EchoCommandsQuiet.out

@@ -0,0 +1,2 @@
+# CHECK: (lldb) command source -s 1 {{.*\n}}
+# CHECK-NEXT: (lldb) command source -s 1 {{.*\n}}

lldb/lit/Settings/Inputs/EchoCommandsTest.in

@@ -0,0 +1,2 @@
+# Evaluate expr 1+2
+expr 1+2

lldb/lit/Settings/TestEchoCommands.test

@@ -0,0 +1,4 @@
+# RUN: %lldb -x -b -o 'settings set interpreter.echo-comment-commands true'  -s %S/Inputs/EchoCommandsTest.in | FileCheck %S/Inputs/EchoCommandsAll.out
+# RUN: %lldb -x -b -o 'settings set interpreter.echo-comment-commands false' -s %S/Inputs/EchoCommandsTest.in | FileCheck %S/Inputs/EchoCommandsNoComments.out
+# RUN: %lldb -x -b -o 'settings set interpreter.echo-commands false'         -s %S/Inputs/EchoCommandsTest.in | FileCheck %S/Inputs/EchoCommandsNone.out
+# RUN: %lldb -x -b --source-quietly                                          -s %S/Inputs/EchoCommandsTest.in | FileCheck %S/Inputs/EchoCommandsQuiet.out

lldb/lit/lit-lldb-init

@@ -1,2 +1,3 @@
 # LLDB init file for the LIT tests.
 settings set symbols.enable-external-lookup false
+settings set interpreter.echo-comment-commands false

lldb/source/API/SBCommandInterpreter.cpp

@@ -71,6 +71,14 @@ void SBCommandInterpreterRunOptions::SetEchoCommands(bool echo_commands) {
   m_opaque_up->SetEchoCommands(echo_commands);
 }
 
+bool SBCommandInterpreterRunOptions::GetEchoCommentCommands() const {
+  return m_opaque_up->GetEchoCommentCommands();
+}
+
+void SBCommandInterpreterRunOptions::SetEchoCommentCommands(bool echo) {
+  m_opaque_up->SetEchoCommentCommands(echo);
+}
+
 bool SBCommandInterpreterRunOptions::GetPrintResults() const {
   return m_opaque_up->GetPrintResults();
 }

lldb/source/Commands/CommandObjectCommands.cpp

@@ -319,8 +319,15 @@ protected:
       CommandInterpreterRunOptions options;
       options.SetStopOnContinue(m_options.m_stop_on_continue.GetCurrentValue());
       options.SetStopOnError(m_options.m_stop_on_error.GetCurrentValue());
-      options.SetEchoCommands(!m_options.m_silent_run.GetCurrentValue());
-      options.SetPrintResults(!m_options.m_silent_run.GetCurrentValue());
+
+      // Individual silent setting is override for global command echo settings.
+      if (m_options.m_silent_run.GetCurrentValue()) {
+        options.SetSilent(true);
+      } else {
+        options.SetPrintResults(true);
+        options.SetEchoCommands(m_interpreter.GetEchoCommands());
+        options.SetEchoCommentCommands(m_interpreter.GetEchoCommentCommands());
+      }
 
       m_interpreter.HandleCommandsFromFile(cmd_file, exe_ctx, options, result);
     } else {

lldb/source/Interpreter/CommandInterpreter.cpp

@@ -76,26 +76,43 @@ using namespace lldb_private;
 
 static const char *k_white_space = " \t\v";
 
+static constexpr bool NoGlobalSetting = true;
+static constexpr uintptr_t DefaultValueTrue = true;
+static constexpr uintptr_t DefaultValueFalse = false;
+static constexpr const char *NoCStrDefault = nullptr;
+
 static constexpr PropertyDefinition g_properties[] = {
-    {"expand-regex-aliases", OptionValue::eTypeBoolean, true, false, nullptr,
-     {}, "If true, regular expression alias commands will show the "
-              "expanded command that will be executed. This can be used to "
-              "debug new regular expression alias commands."},
-    {"prompt-on-quit", OptionValue::eTypeBoolean, true, true, nullptr, {},
+    {"expand-regex-aliases", OptionValue::eTypeBoolean, NoGlobalSetting,
+     DefaultValueFalse, NoCStrDefault, {},
+     "If true, regular expression alias commands will show the "
+     "expanded command that will be executed. This can be used to "
+     "debug new regular expression alias commands."},
+    {"prompt-on-quit", OptionValue::eTypeBoolean, NoGlobalSetting,
+     DefaultValueTrue, NoCStrDefault, {},
      "If true, LLDB will prompt you before quitting if there are any live "
      "processes being debugged. If false, LLDB will quit without asking in any "
      "case."},
-    {"stop-command-source-on-error", OptionValue::eTypeBoolean, true, true,
-     nullptr, {}, "If true, LLDB will stop running a 'command source' "
-                  "script upon encountering an error."},
-    {"space-repl-prompts", OptionValue::eTypeBoolean, true, false, nullptr, {},
-     "If true, blank lines will be printed between between REPL submissions."}};
+    {"stop-command-source-on-error", OptionValue::eTypeBoolean, NoGlobalSetting,
+     DefaultValueTrue, NoCStrDefault, {},
+     "If true, LLDB will stop running a 'command source' "
+     "script upon encountering an error."},
+    {"space-repl-prompts", OptionValue::eTypeBoolean, NoGlobalSetting,
+     DefaultValueFalse, NoCStrDefault, {},
+     "If true, blank lines will be printed between between REPL submissions."},
+    {"echo-commands", OptionValue::eTypeBoolean, NoGlobalSetting,
+     DefaultValueTrue, NoCStrDefault, {},
+     "If true, commands will be echoed before they are evaluated."},
+    {"echo-comment-commands", OptionValue::eTypeBoolean, NoGlobalSetting,
+     DefaultValueTrue, NoCStrDefault, {},
+     "If true, commands will be echoed even if they are pure comment lines."}};
 
 enum {
   ePropertyExpandRegexAliases = 0,
   ePropertyPromptOnQuit = 1,
   ePropertyStopCmdSourceOnError = 2,
-  eSpaceReplPrompts = 3
+  eSpaceReplPrompts = 3,
+  eEchoCommands = 4,
+  eEchoCommentCommands = 5
 };
 
 ConstString &CommandInterpreter::GetStaticBroadcasterClass() {
@@ -142,6 +159,28 @@ void CommandInterpreter::SetPromptOnQuit(bool b) {
   m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, b);
 }
 
+bool CommandInterpreter::GetEchoCommands() const {
+  const uint32_t idx = eEchoCommands;
+  return m_collection_sp->GetPropertyAtIndexAsBoolean(
+      nullptr, idx, g_properties[idx].default_uint_value != 0);
+}
+
+void CommandInterpreter::SetEchoCommands(bool b) {
+  const uint32_t idx = eEchoCommands;
+  m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, b);
+}
+
+bool CommandInterpreter::GetEchoCommentCommands() const {
+  const uint32_t idx = eEchoCommentCommands;
+  return m_collection_sp->GetPropertyAtIndexAsBoolean(
+      nullptr, idx, g_properties[idx].default_uint_value != 0);
+}
+
+void CommandInterpreter::SetEchoCommentCommands(bool b) {
+  const uint32_t idx = eEchoCommentCommands;
+  m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, b);
+}
+
 void CommandInterpreter::AllowExitCodeOnQuit(bool allow) {
   m_allow_exit_code = allow;
   if (!allow)
@@ -2296,8 +2335,9 @@ enum {
   eHandleCommandFlagStopOnContinue = (1u << 0),
   eHandleCommandFlagStopOnError = (1u << 1),
   eHandleCommandFlagEchoCommand = (1u << 2),
-  eHandleCommandFlagPrintResult = (1u << 3),
-  eHandleCommandFlagStopOnCrash = (1u << 4)
+  eHandleCommandFlagEchoCommentCommand = (1u << 3),
+  eHandleCommandFlagPrintResult = (1u << 4),
+  eHandleCommandFlagStopOnCrash = (1u << 5)
 };
 
 void CommandInterpreter::HandleCommandsFromFile(
@@ -2339,9 +2379,10 @@ void CommandInterpreter::HandleCommandsFromFile(
         flags |= eHandleCommandFlagStopOnError;
       }
 
+      // stop-on-crash can only be set, if it is present in all levels of 
+      // pushed flag sets.
       if (options.GetStopOnCrash()) {
         if (m_command_source_flags.empty()) {
-          // Echo command by default
           flags |= eHandleCommandFlagStopOnCrash;
         } else if (m_command_source_flags.back() &
                    eHandleCommandFlagStopOnCrash) {
@@ -2361,6 +2402,19 @@ void CommandInterpreter::HandleCommandsFromFile(
         flags |= eHandleCommandFlagEchoCommand;
       }
 
+      // We will only ever ask for this flag, if we echo commands in general.
+      if (options.m_echo_comment_commands == eLazyBoolCalculate) {
+        if (m_command_source_flags.empty()) {
+          // Echo comments by default
+          flags |= eHandleCommandFlagEchoCommentCommand;
+        } else if (m_command_source_flags.back() &
+                   eHandleCommandFlagEchoCommentCommand) {
+          flags |= eHandleCommandFlagEchoCommentCommand;
+        }
+      } else if (options.m_echo_comment_commands == eLazyBoolYes) {
+        flags |= eHandleCommandFlagEchoCommentCommand;
+      }
+
       if (options.m_print_results == eLazyBoolCalculate) {
         if (m_command_source_flags.empty()) {
           // Print output by default
@@ -2682,6 +2736,21 @@ void CommandInterpreter::PrintCommandOutput(Stream &stream,
   }
 }
 
+bool CommandInterpreter::EchoCommandNonInteractive(
+    llvm::StringRef line, const Flags &io_handler_flags) const {
+  if (!io_handler_flags.Test(eHandleCommandFlagEchoCommand))
+    return false;
+
+  llvm::StringRef command = line.trim();
+  if (command.empty())
+    return true;
+
+  if (command.front() == m_comment_char)
+    return io_handler_flags.Test(eHandleCommandFlagEchoCommentCommand);
+
+  return true;
+}
+
 void CommandInterpreter::IOHandlerInputComplete(IOHandler &io_handler,
                                                 std::string &line) {
     // If we were interrupted, bail out...
@@ -2700,7 +2769,7 @@ void CommandInterpreter::IOHandlerInputComplete(IOHandler &io_handler,
     // When using a non-interactive file handle (like when sourcing commands
     // from a file) we need to echo the command out so we don't just see the
     // command output and no command...
-    if (io_handler.GetFlags().Test(eHandleCommandFlagEchoCommand))
+    if (EchoCommandNonInteractive(line, io_handler.GetFlags()))
       io_handler.GetOutputStreamFile()->Printf("%s%s\n", io_handler.GetPrompt(),
                                                line.c_str());
   }
@@ -2874,6 +2943,8 @@ CommandInterpreter::GetIOHandler(bool force_create,
         flags |= eHandleCommandFlagStopOnCrash;
       if (options->m_echo_commands != eLazyBoolNo)
         flags |= eHandleCommandFlagEchoCommand;
+      if (options->m_echo_comment_commands != eLazyBoolNo)
+        flags |= eHandleCommandFlagEchoCommentCommand;
       if (options->m_print_results != eLazyBoolNo)
         flags |= eHandleCommandFlagPrintResult;
     } else {