https://github.com/walter-erquinigo created 
https://github.com/llvm/llvm-project/pull/74748

There are some typescript vscode extensions that are effectively wrappers of 
lldb-dap, and they commonly configure the debug adapter in particular ways 
using initCommands, among other settings. An unfortunate side effect is that, 
at least for initCommands, their output is printed on the Debug Console, which 
doesn't look clean and might confuse some users because.
As a way to improve the experience, I'm definining a new `privateConfiguration` 
setting, which can be used by adapter wrappers for private initialization. I'm 
starting with private initCommands, whose output can also be controlled via a 
`printMode` setting. By default, the output is only printed upon errors.
This setting helps distinguish things that the adapter wrapper does privately 
from what the user might want to do using public settings in JSON.


>From c5817d7acf154798cff6ac0b8f24a80002ff202d Mon Sep 17 00:00:00 2001
From: walter erquinigo <wal...@modular.com>
Date: Wed, 29 Nov 2023 18:12:54 -0500
Subject: [PATCH] [lldb-dap] Introduce the new privateConfiguration setting

There are some typescript vscode extensions that are effectively wrappers of 
lldb-dap, and they commonly configure the debug adapter in particular ways 
using initCommands, among other settings. An unfortunate side effect is that, 
at least for initCommands, their output is printed on the Debug Console, which 
doesn't look clean and might confuse some users because.
As a way to improve the experience, I'm definining a new `privateConfiguration` 
setting, which can be used by adapter wrappers for private initialization. I'm 
starting with private initCommands, whose output can also be controlled via a 
`printMode` setting. By default, the output is only printed upon errors.
This setting helps distinguish things that the adapter wrapper does privately 
from what the user might want to do using public settings in JSON.
---
 .../test/tools/lldb-dap/dap_server.py         |  3 +
 .../test/tools/lldb-dap/lldbdap_testcase.py   |  4 +
 .../lldb-dap/privateConfiguration/Makefile    |  2 +
 .../TestDAP_privateConfiguration.py           | 77 +++++++++++++++++++
 .../lldb-dap/privateConfiguration/main.cpp    |  1 +
 lldb/tools/lldb-dap/DAP.cpp                   | 33 +++++++-
 lldb/tools/lldb-dap/DAP.h                     | 30 +++++++-
 lldb/tools/lldb-dap/LLDBUtils.cpp             | 17 ++--
 lldb/tools/lldb-dap/LLDBUtils.h               |  7 +-
 lldb/tools/lldb-dap/lldb-dap.cpp              |  7 +-
 lldb/tools/lldb-dap/package.json              | 17 ++++
 11 files changed, 183 insertions(+), 15 deletions(-)
 create mode 100644 lldb/test/API/tools/lldb-dap/privateConfiguration/Makefile
 create mode 100644 
lldb/test/API/tools/lldb-dap/privateConfiguration/TestDAP_privateConfiguration.py
 create mode 100644 lldb/test/API/tools/lldb-dap/privateConfiguration/main.cpp

diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index bb863bb8719176..4f2bff7ad5101d 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -734,6 +734,7 @@ def request_launch(
         commandEscapePrefix=None,
         customFrameFormat=None,
         customThreadFormat=None,
+        privateConfiguration=None,
     ):
         args_dict = {"program": program}
         if args:
@@ -779,6 +780,8 @@ def request_launch(
             args_dict["customFrameFormat"] = customFrameFormat
         if customThreadFormat:
             args_dict["customThreadFormat"] = customThreadFormat
+        if privateConfiguration:
+            args_dict["privateConfiguration"] = privateConfiguration
 
         args_dict["enableAutoVariableSummaries"] = enableAutoVariableSummaries
         args_dict["enableSyntheticChildDebugging"] = 
enableSyntheticChildDebugging
diff --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
index 4ccd6014e54be6..f1bf4decbe0779 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
@@ -357,6 +357,7 @@ def launch(
         commandEscapePrefix=None,
         customFrameFormat=None,
         customThreadFormat=None,
+        privateConfiguration=None,
     ):
         """Sending launch request to dap"""
 
@@ -398,6 +399,7 @@ def cleanup():
             commandEscapePrefix=commandEscapePrefix,
             customFrameFormat=customFrameFormat,
             customThreadFormat=customThreadFormat,
+            privateConfiguration=privateConfiguration,
         )
 
         if expectFailure:
@@ -437,6 +439,7 @@ def build_and_launch(
         commandEscapePrefix=None,
         customFrameFormat=None,
         customThreadFormat=None,
+        privateConfiguration=None,
     ):
         """Build the default Makefile target, create the DAP debug adaptor,
         and launch the process.
@@ -470,4 +473,5 @@ def build_and_launch(
             commandEscapePrefix=commandEscapePrefix,
             customFrameFormat=customFrameFormat,
             customThreadFormat=customThreadFormat,
+            privateConfiguration=privateConfiguration,
         )
diff --git a/lldb/test/API/tools/lldb-dap/privateConfiguration/Makefile 
b/lldb/test/API/tools/lldb-dap/privateConfiguration/Makefile
new file mode 100644
index 00000000000000..3d0b98f13f3d7b
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/privateConfiguration/Makefile
@@ -0,0 +1,2 @@
+CXX_SOURCES := main.cpp
+include Makefile.rules
diff --git 
a/lldb/test/API/tools/lldb-dap/privateConfiguration/TestDAP_privateConfiguration.py
 
b/lldb/test/API/tools/lldb-dap/privateConfiguration/TestDAP_privateConfiguration.py
new file mode 100644
index 00000000000000..0d930dcbc80e38
--- /dev/null
+++ 
b/lldb/test/API/tools/lldb-dap/privateConfiguration/TestDAP_privateConfiguration.py
@@ -0,0 +1,77 @@
+"""
+Test lldb-dap stack trace response
+"""
+
+
+import os
+
+import dap_server
+import lldbdap_testcase
+from lldbsuite.test import lldbtest, lldbutil
+from lldbsuite.test.decorators import *
+
+
+class TestDAP_privateConfiguration(lldbdap_testcase.DAPTestCaseBase):
+    def do_test_initCommands(self, printMode, includeError=False):
+        """
+        Test the initCommands property of the privateConfiguration setting and
+        its various print modes.
+        """
+        commands = [
+            "settings set target.show-hex-variable-values-with-leading-zeroes 
false"
+        ]
+        if includeError:
+            commands.append(
+                "settings set 
target.show-hex-variable-values-with-leading-zeroes fooooo"
+            )
+
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(
+            program,
+            privateConfiguration={
+                "initCommands": {
+                    "commands": commands,
+                    "printMode": printMode,
+                }
+            },
+        )
+        full_output = self.collect_console(duration=1.0)
+        expected_output = """Running privateInitCommands:
+(lldb) settings set target.show-hex-variable-values-with-leading-zeroes 
false"""
+        expected_error_output = "error: invalid boolean string value: 'fooooo'"
+
+        if printMode == "always":
+            self.assertIn(expected_output, full_output)
+            if includeError:
+                self.assertIn(expected_error_output, full_output)
+        elif printMode == "never":
+            self.assertNotIn(expected_output, full_output)
+            if includeError:
+                self.assertNotIn(expected_error_output, full_output)
+        else:
+            if includeError:
+                self.assertIn(expected_output, full_output)
+                self.assertIn(expected_error_output, full_output)
+            else:
+                self.assertNotIn(expected_output, full_output)
+                self.assertNotIn(expected_error_output, full_output)
+
+    @skipIfWindows
+    @skipIfRemote
+    def test_initCommands_with_print_mode_always(self):
+        self.do_test_initCommands("always")
+
+    @skipIfWindows
+    @skipIfRemote
+    def test_initCommands_with_print_mode_never(self):
+        self.do_test_initCommands("never", includeError=True)
+
+    @skipIfWindows
+    @skipIfRemote
+    def test_initCommands_with_print_mode_onError_with_failure(self):
+        self.do_test_initCommands("onError", includeError=True)
+
+    @skipIfWindows
+    @skipIfRemote
+    def test_initCommands_with_print_mode_onError_no_actual_failures(self):
+        self.do_test_initCommands("onError", includeError=False)
diff --git a/lldb/test/API/tools/lldb-dap/privateConfiguration/main.cpp 
b/lldb/test/API/tools/lldb-dap/privateConfiguration/main.cpp
new file mode 100644
index 00000000000000..76e8197013aabc
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/privateConfiguration/main.cpp
@@ -0,0 +1 @@
+int main() { return 0; }
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 49266209739765..75549ff3a11c09 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -435,9 +435,19 @@ ExpressionContext 
DAP::DetectExpressionContext(lldb::SBFrame &frame,
 }
 
 void DAP::RunLLDBCommands(llvm::StringRef prefix,
-                          const std::vector<std::string> &commands) {
-  SendOutput(OutputType::Console,
-             llvm::StringRef(::RunLLDBCommands(prefix, commands)));
+                          llvm::ArrayRef<std::string> commands,
+                          CommandsPrintMode print_mode) {
+  auto [output, success] = ::RunLLDBCommands(prefix, commands);
+  if (print_mode == CommandsPrintMode::Always ||
+      (!success && print_mode == CommandsPrintMode::OnError)) {
+    SendOutput(OutputType::Console, output);
+  }
+}
+
+void DAP::RunPrivateInitCommands() {
+  RunLLDBCommands("Running privateInitCommands:",
+                  private_configuration.init_commands.commands,
+                  private_configuration.init_commands.print_mode);
 }
 
 void DAP::RunInitCommands() {
@@ -854,4 +864,21 @@ void DAP::SetThreadFormat(llvm::StringRef format) {
   }
 }
 
+void DAP::ParsePrivateConfiguration(
+    const llvm::json::Object *private_configuration) {
+  if (!private_configuration)
+    return;
+  if (auto *init_commands = private_configuration->getObject("initCommands")) {
+    if (std::optional<llvm::StringRef> print_mode =
+            init_commands->getString("printMode")) {
+      this->private_configuration.init_commands.print_mode =
+          print_mode == "onError" ? CommandsPrintMode::OnError
+          : print_mode == "never" ? CommandsPrintMode::Never
+                                  : CommandsPrintMode::Always;
+    }
+    this->private_configuration.init_commands.commands =
+        GetStrings(init_commands, "commands");
+  }
+}
+
 } // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index c7d56a06bfa1fd..c067c35a1c14cc 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -143,6 +143,16 @@ struct ReplModeRequestHandler : public 
lldb::SBCommandPluginInterface {
 };
 
 struct DAP {
+  /// Enum that controls when to print the output of a series of LLDB commands.
+  enum CommandsPrintMode {
+    /// The output is always printed to the user.
+    Always,
+    /// The output is only printed if one of the commands fails.
+    OnError,
+    /// The output is never printed.
+    Never,
+  };
+
   std::string debug_adaptor_path;
   InputStream input;
   OutputStream output;
@@ -161,6 +171,14 @@ struct DAP {
   std::vector<std::string> exit_commands;
   std::vector<std::string> stop_commands;
   std::vector<std::string> terminate_commands;
+
+  struct PrivateConfiguration {
+    struct {
+      std::vector<std::string> commands;
+      CommandsPrintMode print_mode = CommandsPrintMode::OnError;
+    } init_commands;
+  } private_configuration;
+
   // A copy of the last LaunchRequest or AttachRequest so we can reuse its
   // arguments if we get a RestartRequest.
   std::optional<llvm::json::Object> last_launch_or_attach_request;
@@ -170,6 +188,7 @@ struct DAP {
   bool is_attach;
   bool enable_auto_variable_summaries;
   bool enable_synthetic_child_debugging;
+
   // The process event thread normally responds to process exited events by
   // shutting down the entire adapter. When we're restarting, we keep the id of
   // the old process here so we can detect this case and keep running.
@@ -227,9 +246,11 @@ struct DAP {
   ExpressionContext DetectExpressionContext(lldb::SBFrame &frame,
                                             std::string &text);
 
-  void RunLLDBCommands(llvm::StringRef prefix,
-                       const std::vector<std::string> &commands);
+  void
+  RunLLDBCommands(llvm::StringRef prefix, llvm::ArrayRef<std::string> commands,
+                  CommandsPrintMode print_mode = CommandsPrintMode::Always);
 
+  void RunPrivateInitCommands();
   void RunInitCommands();
   void RunPreRunCommands();
   void RunStopCommands();
@@ -312,6 +333,11 @@ struct DAP {
 
   void SetThreadFormat(llvm::StringRef format);
 
+  /// Parse the `privateConfiguration` JSON object. If \b nullptr, this method
+  /// does nothing.
+  void
+  ParsePrivateConfiguration(const llvm::json::Object *private_configuration);
+
 private:
   // Send the JSON in "json_str" to the "out" stream. Correctly send the
   // "Content-Length:" field followed by the length, followed by the raw
diff --git a/lldb/tools/lldb-dap/LLDBUtils.cpp 
b/lldb/tools/lldb-dap/LLDBUtils.cpp
index 955c11f636895b..25989f404de875 100644
--- a/lldb/tools/lldb-dap/LLDBUtils.cpp
+++ b/lldb/tools/lldb-dap/LLDBUtils.cpp
@@ -11,11 +11,13 @@
 
 namespace lldb_dap {
 
-void RunLLDBCommands(llvm::StringRef prefix,
+bool RunLLDBCommands(llvm::StringRef prefix,
                      const llvm::ArrayRef<std::string> &commands,
                      llvm::raw_ostream &strm) {
   if (commands.empty())
-    return;
+    return true;
+  bool success = true;
+
   lldb::SBCommandInterpreter interp = g_dap.debugger.GetCommandInterpreter();
   if (!prefix.empty())
     strm << prefix << "\n";
@@ -32,17 +34,20 @@ void RunLLDBCommands(llvm::StringRef prefix,
     if (error_len) {
       const char *error = result.GetError();
       strm << error;
+      success = false;
     }
   }
+  return success;
 }
 
-std::string RunLLDBCommands(llvm::StringRef prefix,
-                            const llvm::ArrayRef<std::string> &commands) {
+std::pair<std::string, bool>
+RunLLDBCommands(llvm::StringRef prefix,
+                const llvm::ArrayRef<std::string> &commands) {
   std::string s;
   llvm::raw_string_ostream strm(s);
-  RunLLDBCommands(prefix, commands, strm);
+  bool success = RunLLDBCommands(prefix, commands, strm);
   strm.flush();
-  return s;
+  return {s, success};
 }
 
 bool ThreadHasStopReason(lldb::SBThread &thread) {
diff --git a/lldb/tools/lldb-dap/LLDBUtils.h b/lldb/tools/lldb-dap/LLDBUtils.h
index a99f798835370d..70c6b416a04417 100644
--- a/lldb/tools/lldb-dap/LLDBUtils.h
+++ b/lldb/tools/lldb-dap/LLDBUtils.h
@@ -33,7 +33,7 @@ namespace lldb_dap {
 /// \param[in] strm
 ///     The stream that will receive the prefix, prompt + command and
 ///     all command output.
-void RunLLDBCommands(llvm::StringRef prefix,
+bool RunLLDBCommands(llvm::StringRef prefix,
                      const llvm::ArrayRef<std::string> &commands,
                      llvm::raw_ostream &strm);
 
@@ -52,8 +52,9 @@ void RunLLDBCommands(llvm::StringRef prefix,
 /// \return
 ///     A std::string that contains the prefix and all commands and
 ///     command output
-std::string RunLLDBCommands(llvm::StringRef prefix,
-                            const llvm::ArrayRef<std::string> &commands);
+std::pair<std::string, bool>
+RunLLDBCommands(llvm::StringRef prefix,
+                const llvm::ArrayRef<std::string> &commands);
 
 /// Check if a thread has a stop reason.
 ///
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index d6b593eba93eca..c082289e16e2f4 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -634,6 +634,7 @@ void request_attach(const llvm::json::Object &request) {
     attach_info.SetProcessID(pid);
   const auto wait_for = GetBoolean(arguments, "waitFor", false);
   attach_info.SetWaitForLaunch(wait_for, false /*async*/);
+  
g_dap.ParsePrivateConfiguration(arguments->getObject("privateConfiguration"));
   g_dap.init_commands = GetStrings(arguments, "initCommands");
   g_dap.pre_run_commands = GetStrings(arguments, "preRunCommands");
   g_dap.stop_commands = GetStrings(arguments, "stopCommands");
@@ -664,6 +665,7 @@ void request_attach(const llvm::json::Object &request) {
     llvm::sys::fs::set_current_path(debuggerRoot);
 
   // Run any initialize LLDB commands the user specified in the launch.json
+  g_dap.RunPrivateInitCommands();
   g_dap.RunInitCommands();
 
   SetSourceMapFromArguments(*arguments);
@@ -1270,7 +1272,8 @@ void request_evaluate(const llvm::json::Object &request) {
     if (frame.IsValid()) {
       g_dap.focus_tid = frame.GetThread().GetThreadID();
     }
-    auto result = RunLLDBCommands(llvm::StringRef(), 
{std::string(expression)});
+    auto result =
+        RunLLDBCommands(llvm::StringRef(), {std::string(expression)}).first;
     EmplaceSafeString(body, "result", result);
     body.try_emplace("variablesReference", (int64_t)0);
   } else {
@@ -1792,6 +1795,7 @@ void request_launch(const llvm::json::Object &request) {
   llvm::json::Object response;
   FillResponse(request, response);
   auto arguments = request.getObject("arguments");
+  
g_dap.ParsePrivateConfiguration(arguments->getObject("privateConfiguration"));
   g_dap.init_commands = GetStrings(arguments, "initCommands");
   g_dap.pre_run_commands = GetStrings(arguments, "preRunCommands");
   g_dap.stop_commands = GetStrings(arguments, "stopCommands");
@@ -1820,6 +1824,7 @@ void request_launch(const llvm::json::Object &request) {
   // Run any initialize LLDB commands the user specified in the launch.json.
   // This is run before target is created, so commands can't do anything with
   // the targets - preRunCommands are run with the target.
+  g_dap.RunPrivateInitCommands();
   g_dap.RunInitCommands();
 
   SetSourceMapFromArguments(*arguments);
diff --git a/lldb/tools/lldb-dap/package.json b/lldb/tools/lldb-dap/package.json
index ebb1103d695e17..5099ed7727c6f0 100644
--- a/lldb/tools/lldb-dap/package.json
+++ b/lldb/tools/lldb-dap/package.json
@@ -369,6 +369,23 @@
                                                                "type": 
"string",
                                                                "description": 
"If non-empty, threads will have descriptions generated based on the provided 
format. See https://lldb.llvm.org/use/formatting.html for an explanation on 
format strings for threads. If the format string contains errors, an error 
message will be displayed on the Debug Console and the default thread names 
will be used. This might come with a performance cost because debug information 
might need to be processed to generate the description.",
                                                                "default": ""
+                                                       },
+                                                       "privateConfiguration": 
{
+                                                               "description": 
"Additional settings that plugins that wrap `lldb-dap` can use to configure the 
debugger without interfering with the other settings, which are meant to be 
used directly by the end user.",
+                                                               "properties": {
+                                                                       
"initCommands": {
+                                                                               
"properties": {
+                                                                               
        "commands": {
+                                                                               
                "type": "array",
+                                                                               
                "description": "Initialization commands executed upon debugger 
startup. It is executed before the public `initCommands` and its output can be 
controlled by the `printMode` property."
+                                                                               
        },
+                                                                               
        "printMode": {
+                                                                               
                "type": "string",
+                                                                               
                "description": "If \"always\", then the output of the private 
`initCommands` is always printed. If \"onError\", then the output is only 
printed if any of the commands fails. Otherwise, if \"never\", then the output 
is never printed. Defaults to \"onError\"."
+                                                                               
        }
+                                                                               
}
+                                                                       }
+                                                               }
                                                        }
                                                }
                                        }

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to