Moving 'program', 'targetTriple' and 'platform' to the protocol::Configuration since they're shared between launch and attach.

Additionally, I tried to be more consistent with how we apply default flags state in the protocol definition.

Author: ashgti

lldb/tools/lldb-dap/DAP.cpp

@@ -654,10 +654,7 @@ void DAP::RunTerminateCommands() {
                   configuration.terminateCommands);
 }
 
-lldb::SBTarget DAP::CreateTargetFromArguments(llvm::StringRef program,
-                                              llvm::StringRef targetTriple,
-                                              llvm::StringRef platformName,
-                                              lldb::SBError &error) {
+lldb::SBTarget DAP::CreateTarget(lldb::SBError &error) {
   // Grab the name of the program we need to debug and create a target using
   // the given program as an argument. Executable file can be a source of target
   // architecture and platform, if they differ from the host. Setting exe path
@@ -668,16 +665,17 @@ lldb::SBTarget DAP::CreateTargetFromArguments(llvm::StringRef program,
   // enough information to determine correct arch and platform (or ELF can be
   // omitted at all), so it is good to leave the user an apportunity to specify
   // those. Any of those three can be left empty.
-  auto target = this->debugger.CreateTarget(program.data(), targetTriple.data(),
-                                            platformName.data(),
-                                            true, // Add dependent modules.
-                                            error);
+  auto target = this->debugger.CreateTarget(
+      configuration.program.value_or("").data(),
+      configuration.targetTriple.value_or("").data(),
+      configuration.platformName.value_or("").data(),
+      true, // Add dependent modules.
+      error);
 
   if (error.Fail()) {
     // Update message if there was an error.
     error.SetErrorStringWithFormat(
-        "Could not create a target for a program '%s': %s.", program.data(),
-        error.GetCString());
+        "Could not create a target for a program: %s.", error.GetCString());
   }
 
   return target;

lldb/tools/lldb-dap/DAP.h

@@ -316,19 +316,14 @@ struct DAP {
   void RunTerminateCommands();
 
   /// Create a new SBTarget object from the given request arguments.
-  /// \param[in] arguments
-  ///     Launch configuration arguments.
   ///
   /// \param[out] error
   ///     An SBError object that will contain an error description if
   ///     function failed to create the target.
   ///
   /// \return
   ///     An SBTarget object.
-  lldb::SBTarget CreateTargetFromArguments(llvm::StringRef program,
-                                           llvm::StringRef targetTriple,
-                                           llvm::StringRef platformName,
-                                           lldb::SBError &error);
+  lldb::SBTarget CreateTarget(lldb::SBError &error);
 
   /// Set given target object as a current target for lldb-dap and start
   /// listeing for its breakpoint events.

lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp

@@ -85,6 +85,9 @@ void AttachRequestHandler::operator()(const llvm::json::Object &request) const {
       GetBoolean(arguments, "displayExtendedBacktrace").value_or(false);
   dap.configuration.commandEscapePrefix =
       GetString(arguments, "commandEscapePrefix").value_or("`");
+  dap.configuration.program = GetString(arguments, "program");
+  dap.configuration.targetTriple = GetString(arguments, "targetTriple");
+  dap.configuration.platformName = GetString(arguments, "platformName");
   dap.SetFrameFormat(GetString(arguments, "customFrameFormat").value_or(""));
   dap.SetThreadFormat(GetString(arguments, "customThreadFormat").value_or(""));
 
@@ -107,14 +110,8 @@ void AttachRequestHandler::operator()(const llvm::json::Object &request) const {
 
   SetSourceMapFromArguments(*arguments);
 
-  llvm::StringRef program = GetString(arguments, "program").value_or("");
-  llvm::StringRef target_triple =
-      GetString(arguments, "targetTriple").value_or("");
-  llvm::StringRef platform_name =
-      GetString(arguments, "platformName").value_or("");
   lldb::SBError status;
-  dap.SetTarget(dap.CreateTargetFromArguments(program, target_triple,
-                                              platform_name, status));
+  dap.SetTarget(dap.CreateTarget(status));
   if (status.Fail()) {
     response["success"] = llvm::json::Value(false);
     EmplaceSafeString(response, "message", status.GetCString());

lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp

@@ -11,6 +11,7 @@
 #include "JSONUtils.h"
 #include "Protocol/ProtocolRequests.h"
 #include "RequestHandler.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 
 namespace lldb_dap {
@@ -20,7 +21,12 @@ llvm::Expected<protocol::LaunchResponseBody> LaunchRequestHandler::Run(
     const protocol::LaunchRequestArguments &arguments) const {
   dap.SetConfiguration(arguments.configuration, /*is_attach=*/false);
   dap.last_launch_request = arguments;
-  dap.stop_at_entry = arguments.stopOnEntry.value_or(false);
+  dap.stop_at_entry = arguments.stopOnEntry;
+
+  if (!arguments.launchCommands.empty() && arguments.runInTerminal) {
+    return llvm::make_error<DAPError>("launchCommands and runInTerminal cannot "
+                                      "both be set, use one or the other.");
+  }
 
   PrintWelcomeMessage();
 
@@ -41,9 +47,7 @@ llvm::Expected<protocol::LaunchResponseBody> LaunchRequestHandler::Run(
   dap.ConfigureSourceMaps();
 
   lldb::SBError status;
-  dap.SetTarget(dap.CreateTargetFromArguments(
-      arguments.program.value_or(""), arguments.targetTriple.value_or(""),
-      arguments.platformName.value_or(""), status));
+  dap.SetTarget(dap.CreateTarget(status));
   if (status.Fail())
     return llvm::make_error<DAPError>(status.GetCString());
 

lldb/tools/lldb-dap/Handler/RequestHandler.cpp

@@ -38,14 +38,12 @@ MakeArgv(const llvm::ArrayRef<std::string> &strs) {
   return argv;
 }
 
-static uint32_t SetLaunchFlag(uint32_t flags, std::optional<bool> flag,
+static uint32_t SetLaunchFlag(uint32_t flags, bool flag,
                               lldb::LaunchFlags mask) {
-  if (flag) {
-    if (*flag)
-      flags |= mask;
-    else
-      flags &= ~mask;
-  }
+  if (flag)
+    flags |= mask;
+  else
+    flags &= ~mask;
 
   return flags;
 }
@@ -107,7 +105,8 @@ RunInTerminal(DAP &dap, const protocol::LaunchRequestArguments &arguments) {
     return llvm::make_error<DAPError>("Cannot use runInTerminal, feature is "
                                       "not supported by the connected client");
 
-  if (!arguments.program || arguments.program->empty())
+  if (!arguments.configuration.program ||
+      arguments.configuration.program->empty())
     return llvm::make_error<DAPError>(
         "program must be set to when using runInTerminal");
 
@@ -128,7 +127,7 @@ RunInTerminal(DAP &dap, const protocol::LaunchRequestArguments &arguments) {
 #endif
 
   llvm::json::Object reverse_request = CreateRunInTerminalReverseRequest(
-      *arguments.program, arguments.args, arguments.env,
+      *arguments.configuration.program, arguments.args, arguments.env,
       arguments.cwd.value_or(""), comm_file.m_path, debugger_pid);
   dap.SendReverseRequest<LogFailureResponseHandler>("runInTerminal",
                                                     std::move(reverse_request));
@@ -223,23 +222,21 @@ llvm::Error BaseRequestHandler::LaunchProcess(
     launch_info.SetEnvironment(env, true);
   }
 
-  auto flags = launch_info.GetLaunchFlags();
+  launch_info.SetDetachOnError(arguments.detachOnError);
+  launch_info.SetShellExpandArguments(arguments.shellExpandArguments);
 
+  auto flags = launch_info.GetLaunchFlags();
   flags =
       SetLaunchFlag(flags, arguments.disableASLR, lldb::eLaunchFlagDisableASLR);
   flags = SetLaunchFlag(flags, arguments.disableSTDIO,
                         lldb::eLaunchFlagDisableSTDIO);
-  flags = SetLaunchFlag(flags, arguments.shellExpandArguments,
-                        lldb::eLaunchFlagShellExpandArguments);
-  if (arguments.detachOnError)
-    launch_info.SetDetachOnError(*arguments.detachOnError);
   launch_info.SetLaunchFlags(flags | lldb::eLaunchFlagDebug |
                              lldb::eLaunchFlagStopAtEntry);
 
   if (arguments.runInTerminal) {
     if (llvm::Error err = RunInTerminal(dap, arguments))
       return err;
-  } else if (!launchCommands) {
+  } else if (launchCommands.empty()) {
     lldb::SBError error;
     // Disable async events so the launch will be successful when we return from
     // the launch call and the launch will happen synchronously
@@ -252,7 +249,7 @@ llvm::Error BaseRequestHandler::LaunchProcess(
     // Set the launch info so that run commands can access the configured
     // launch details.
     dap.target.SetLaunchInfo(launch_info);
-    if (llvm::Error err = dap.RunLaunchCommands(*launchCommands))
+    if (llvm::Error err = dap.RunLaunchCommands(launchCommands))
       return err;
 
     // The custom commands might have created a new target so we should use the

lldb/tools/lldb-dap/Handler/RestartRequestHandler.cpp

@@ -108,7 +108,7 @@ void RestartRequestHandler::operator()(
       // Update DAP configuration based on the latest copy of the launch
       // arguments.
       dap.SetConfiguration(updated_arguments.configuration, false);
-      dap.stop_at_entry = updated_arguments.stopOnEntry.value_or(false);
+      dap.stop_at_entry = updated_arguments.stopOnEntry;
       dap.ConfigureSourceMaps();
     }
   }