[lldb-vscode] Convert launch_info and attach_info to local variables

[anton.kolesov]

Summary

Those fields inside of the global variable can be local variables because
they are used in only inside of one function: request_launch for launch_info
and request_attach for attach_info.

To avoid confusion an already existing local variable attach_info of
request_attach has been renamed to better reflect its purpose.

[labath]

Sounds like a good idea. Could you do the same for attach_info (it looks like it should be possible)? Otherwise, one is left wondering about what's the difference...

[anton.kolesov]

In D76593#1936337, @labath wrote:

Sounds like a good idea. Could you do the same for attach_info (it looks like it should be possible)? Otherwise, one is left wondering about what's the difference...

I have that as my next patch - it seems like something that should be in a separate commit, but in the same pull request, but I don't know how to properly submit patch series for review at Phabricator - only through "related revisions", I presume?

[labath]

In D76593#1936757, @anton.kolesov wrote:

In D76593#1936337, @labath wrote:

Sounds like a good idea. Could you do the same for attach_info (it looks like it should be possible)? Otherwise, one is left wondering about what's the difference...

I have that as my next patch - it seems like something that should be in a separate commit, but in the same pull request, but I don't know how to properly submit patch series for review at Phabricator - only through "related revisions", I presume?

You can (and should) use the "related revisions" functionality to denote the relationship between patches, but the purpose of this relationship is to make it easier to review by explaining what depends on what. We don't really have a concept of a "pull request" consisting of multiple patches, which get all committed as a group. Normally each patch should make sense on its own, even if it's not immediately useful (e.g. because it introduces an interface that is not yet used by anyone). The "related" patches can then explain/demonstrate how it's going to be used, but the preceding patches don't need to wait for a green light on the entire group to be committed. If there's some deviation from this (e.g. a preparatory refactoring patch makes some code substantially more complicated and so it may not be desired if the latter patch is not accepted, or there is some uncertainty about the overall direction of the patchset), it should be called out explicitly.

But none of this is really relevant for this patch, which is so simple you could have done it in one commit (and it could be argued that it *should* be done in a single commit, as doing it partway introduces a somewhat inconsistent state, which is undesirable).

[clayborg]

Yes, this works now that we are creating the target in this function.

[anton.kolesov]

Added attach_info conversion into the same revision.

[labath]

still lgtm, just make sure to clang-format before committing.

lldb/tools/lldb-vscode/VSCode.h

 enum VSCodeBroadcasterBits { eBroadcastBitStopEventThread = 1u << 0 };
 
 struct VSCode {
   InputStream input;
   OutputStream output;
   lldb::SBDebugger debugger;
   lldb::SBTarget target;
   lldb::SBAttachInfo attach_info;
-  lldb::SBLaunchInfo launch_info;
   lldb::SBValueList variables;
   lldb::SBBroadcaster broadcaster;
   int64_t num_regs;
   int64_t num_locals;
   int64_t num_globals;
   std::thread event_thread;
   std::unique_ptr<std::ofstream> log;
   llvm::DenseMap<lldb::addr_t, int64_t> addr_to_source_ref;

lldb/tools/lldb-vscode/VSCode.cpp

 
 using namespace lldb_vscode;
 
 namespace lldb_vscode {
 
 VSCode g_vsc;
 
 VSCode::VSCode()
-    : launch_info(nullptr), variables(), broadcaster("lldb-vscode"),
+    : variables(), broadcaster("lldb-vscode"),
       num_regs(0), num_locals(0), num_globals(0), log(),
       exception_breakpoints(
           {{"cpp_catch", "C++ Catch", lldb::eLanguageTypeC_plus_plus},
            {"cpp_throw", "C++ Throw", lldb::eLanguageTypeC_plus_plus},
            {"objc_catch", "Objective C Catch", lldb::eLanguageTypeObjC},
            {"objc_throw", "Objective C Throw", lldb::eLanguageTypeObjC},
            {"swift_catch", "Swift Catch", lldb::eLanguageTypeSwift},
            {"swift_throw", "Swift Throw", lldb::eLanguageTypeSwift}}),

lldb/tools/lldb-vscode/lldb-vscode.cpp

void request_launch(const llvm::json::Object &request) {
   if (status.Fail()) {
     response["success"] = llvm::json::Value(false);
     EmplaceSafeString(response, "message", status.GetCString());
     g_vsc.SendJSON(llvm::json::Value(std::move(response)));
     return;
   }
 
   // Instantiate a launch info instance for the target.
-  g_vsc.launch_info = g_vsc.target.GetLaunchInfo();
+  auto launch_info = g_vsc.target.GetLaunchInfo();
 
   // Grab the current working directory if there is one and set it in the
   // launch info.
   const auto cwd = GetString(arguments, "cwd");
   if (!cwd.empty())
-    g_vsc.launch_info.SetWorkingDirectory(cwd.data());
+    launch_info.SetWorkingDirectory(cwd.data());
 
   // Extract any extra arguments and append them to our program arguments for
   // when we launch
   auto args = GetStrings(arguments, "args");
   if (!args.empty())
-    g_vsc.launch_info.SetArguments(MakeArgv(args).data(), true);
+    launch_info.SetArguments(MakeArgv(args).data(), true);
 
   // Pass any environment variables along that the user specified.
   auto envs = GetStrings(arguments, "env");
   if (!envs.empty())
-    g_vsc.launch_info.SetEnvironmentEntries(MakeArgv(envs).data(), true);
+    launch_info.SetEnvironmentEntries(MakeArgv(envs).data(), true);
 
-  auto flags = g_vsc.launch_info.GetLaunchFlags();
+  auto flags = launch_info.GetLaunchFlags();
 
   if (GetBoolean(arguments, "disableASLR", true))
     flags |= lldb::eLaunchFlagDisableASLR;
   if (GetBoolean(arguments, "disableSTDIO", false))
     flags |= lldb::eLaunchFlagDisableSTDIO;
   if (GetBoolean(arguments, "shellExpandArguments", false))
     flags |= lldb::eLaunchFlagShellExpandArguments;
   const bool detatchOnError = GetBoolean(arguments, "detachOnError", false);
-  g_vsc.launch_info.SetDetachOnError(detatchOnError);
-  g_vsc.launch_info.SetLaunchFlags(flags | lldb::eLaunchFlagDebug |
-                                   lldb::eLaunchFlagStopAtEntry);
+  launch_info.SetDetachOnError(detatchOnError);
+  launch_info.SetLaunchFlags(flags | lldb::eLaunchFlagDebug |
 
   // Run any pre run LLDB commands the user specified in the launch.json
   g_vsc.RunPreRunCommands();
   if (launchCommands.empty()) {
     // Disable async events so the launch will be successful when we return from
     // the launch call and the launch will happen synchronously
     g_vsc.debugger.SetAsync(false);
-    g_vsc.target.Launch(g_vsc.launch_info, error);
+    g_vsc.target.Launch(launch_info, error);
     g_vsc.debugger.SetAsync(true);
   } else {
     g_vsc.RunLLDBCommands("Running launchCommands:", launchCommands);
     // The custom commands might have created a new target so we should use the
     // selected target after these commands are run.
     g_vsc.target = g_vsc.debugger.GetSelectedTarget();
   }