[lldb-vscode] Use libOption with tablegen to parse command line options.

[ivanhernandez13]

Summary

This change will bring lldb-vscode in line with how several other llvm tools process command line arguments and make it easier to add future options.

[ivanhernandez13]

Assuming this looks good, can I get some pointers on how and where I should add a test for this?

I based this change on the one for the LLDB driver (https://reviews.llvm.org/D54692) which added lldb/trunk/lit/Driver/TestCommands.test. Should I look into adding something similar or should I be using lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py and adding something in lldb/test/API/tools/lldb-vscode?

This is what the output for help looks like:

~/llvm-project ❯❯❯ build/Debug/bin/lldb-vscode --help                                                                                                                               [INSERT]
OVERVIEW: LLDB VSCode

USAGE: lldb-vscodeoptions

OPTIONS:
  -g                  Alias for --wait-for-debugger
  --help              Prints out the usage information for the LLDB debug adapter tool.
  -h                  Alias for --help
  --port <port>       What port to listen on.
  -p <value>          Alias for --port
  --wait-for-debugger Pause the program at startup.

EXAMPLES:
  The debug adapter can be started in two modes.

  Running lldb-vscode without any arguments will start communicating with the
  parent over stdio. Passing a port number causes lldb-vscode to start listening
  for connections on that port.

    lldb-vscode -p <port>

  Passing --wait-for-debugger will pause the process at startup and wait for a
  debugger to attach to the process.

    lldb-vscode -w
  %

[clayborg]

Looks good. I would add a .test file like the command line driver to cover this.

[JDevlieghere]

LGTM with the inline comments addressed and a test in Shell/VSCode.

lldb/tools/lldb-vscode/CMakeLists.txt

20

Spurious newline

24

Please use Options.td for consistency with the lldb driver.

[ivanhernandez13]

• Rename opts to options to be consistent with the lldb driver and add a test case around setting an invalid port.

[ivanhernandez13]

I added the test file but atm the two existing arguments (--wait-for-debugger and --port) will cause lldb-vscode to hang and await some sort of action from an external process causing the test runner to hang. I added a single test case for handling an invalid port since that will cause the process to exit immediately but not certain on how to handle the other cases.

[JDevlieghere]

In D74798#1882994, @ivanhernandez13 wrote:

I added the test file but atm the two existing arguments (--wait-for-debugger and --port) will cause lldb-vscode to hang and await some sort of action from an external process causing the test runner to hang. I added a single test case for handling an invalid port since that will cause the process to exit immediately but not certain on how to handle the other cases.

I think for these particular cases it's sufficient to check the help output and make sure the flags are there.

[clayborg]

In D74798#1883015, @JDevlieghere wrote:

In D74798#1882994, @ivanhernandez13 wrote:

I added the test file but atm the two existing arguments (--wait-for-debugger and --port) will cause lldb-vscode to hang and await some sort of action from an external process causing the test runner to hang. I added a single test case for handling an invalid port since that will cause the process to exit immediately but not certain on how to handle the other cases.

I think for these particular cases it's sufficient to check the help output and make sure the flags are there.

yeah just test for the options themselves in the help output. --wait-for-debugger is only when you need to debug lldb-vscode's initial packets with a debugger, so it is not a user facing argument. We might choose to hide this option by default. I know LLVM option parsing can do this.

[ivanhernandez13]

• Update ldb-vscode options test to simply look for the expected flags when passing the --help option.

[JDevlieghere]

lldb/test/Shell/VSCode/TestOptions.test

9

Missing newline

[ivanhernandez13]

• Add missing newline.

[clayborg]

LTGM.

[ivanhernandez13]

Thanks for the guidance!

When there are no additional concerns, would someone mind committing this change for me? I don't have commit access.

[JDevlieghere]

In D74798#1886477, @ivanhernandez13 wrote:

Thanks for the guidance!

When there are no additional concerns, would someone mind committing this change for me? I don't have commit access.

Done!

To github.com:llvm/llvm-project.git
   de0dda54d38..c47e0e2d37d  master -> master

lldb/test/Shell/VSCode/TestOptions.test

+# RUN: lldb-vscode --help | FileCheck %s
+# CHECK: -g
+# CHECK: --help
+# CHECK: -h
+# CHECK: --port
+# CHECK: -p
+# CHECK: --wait-for-debugger
+

lldb/test/Shell/helper/toolchain.py

primary_tools = [
                   extra_args=dsargs,
                   unresolved='ignore'),
         ToolSubst('%platformserver',
                   command=FindTool('lldb-server'),
                   extra_args=['platform'],
                   unresolved='ignore'),
         'lldb-test',
         'lldb-instr',
+        'lldb-vscode',
         ToolSubst('%build',
                   command="'" + sys.executable + "'",
                   extra_args=build_script_args)
         ]
 
     _disallow(config, 'lldb')
     _disallow(config, 'debugserver')
     _disallow(config, 'platformserver')

lldb/tools/lldb-vscode/CMakeLists.txt

if(APPLE)
   configure_file(
     ${CMAKE_CURRENT_SOURCE_DIR}/lldb-vscode-Info.plist.in
     ${CMAKE_CURRENT_BINARY_DIR}/lldb-vscode-Info.plist
     )
   # Inline info plist in binary (use target_link_options for this as soon as CMake 3.13 is available)
   set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,-sectcreate,__TEXT,__info_plist,${CMAKE_CURRENT_BINARY_DIR}/lldb-vscode-Info.plist")
 endif()
 
 # We need to include the llvm components we depend on manually, as liblldb does
 # not re-export those.
 set(LLVM_LINK_COMPONENTS Support)
+set(LLVM_TARGET_DEFINITIONS Options.td)
+tablegen(LLVM Options.inc -gen-opt-parser-defs)
+add_public_tablegen_target(LLDBVSCodeOptionsTableGen)
 add_lldb_tool(lldb-vscode
   lldb-vscode.cpp
   BreakpointBase.cpp
   ExceptionBreakpoint.cpp
   FunctionBreakpoint.cpp
   IOStream.cpp
   JSONUtils.cpp
   LLDBUtils.cpp
   SourceBreakpoint.cpp
   VSCode.cpp
 
   LINK_LIBS
     liblldb
     ${host_lib}
     ${extra_libs}
 
   LINK_COMPONENTS
+    Option
     Support
   )
 
 if(LLDB_BUILD_FRAMEWORK)
   # In the build-tree, we know the exact path to the framework directory.
   # The installed framework can be in different locations.
   lldb_setup_rpaths(lldb-vscode
     BUILD_RPATH
       "${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}"
     INSTALL_RPATH
       "@loader_path/../../../SharedFrameworks"
       "@loader_path/../../System/Library/PrivateFrameworks"
       "@loader_path/../../Library/PrivateFrameworks"
   )
 endif()

lldb/tools/lldb-vscode/Options.td

+include "llvm/Option/OptParser.td"
+
+class F<string name>: Flag<["--", "-"], name>;
+class S<string name>: Separate<["--", "-"], name>;
+class R<list<string> prefixes, string name>
+  : Option<prefixes, name, KIND_REMAINING_ARGS>;
+
+def help: F<"help">,
+  HelpText<"Prints out the usage information for the LLDB VSCode tool.">;
+def: Flag<["-"], "h">,
+  Alias<help>,
+  HelpText<"Alias for --help">;
+
+def wait_for_debugger: F<"wait-for-debugger">,
+  HelpText<"Pause the program at startup.">;
+def: Flag<["-"], "g">,
+  Alias<wait_for_debugger>,
+  HelpText<"Alias for --wait-for-debugger">;
+
+def port: Separate<["--", "-"], "port">,
+  MetaVarName<"<port>">,
+  HelpText<"Communicate with the lldb-vscode tool over the defined port.">;
+def: Separate<["-"], "p">,
+  Alias<port>,
+  HelpText<"Alias for --port">;

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

 #include <map>
 #include <memory>
 #include <mutex>
 #include <set>
 #include <sstream>
 #include <thread>
 
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/Option/Arg.h"
+#include "llvm/Option/ArgList.h"
+#include "llvm/Option/Option.h"
 #include "llvm/Support/Errno.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 
 #include "JSONUtils.h"
 #include "LLDBUtils.h"
 #include "VSCode.h"
 
 #if defined(_WIN32)
 #ifndef PATH_MAX
 #define PATH_MAX MAX_PATH
 #endif
 typedef int socklen_t;
 constexpr const char *dev_null_path = "nul";
 
 #else
 constexpr const char *dev_null_path = "/dev/null";
 
 #endif
 
 using namespace lldb_vscode;
 
 namespace {
+enum ID {
+  OPT_INVALID = 0, // This is not an option ID.
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+               HELPTEXT, METAVAR, VALUES)                                      \
+  OPT_##ID,
+#include "Options.inc"
+#undef OPTION
+};
+
+#define PREFIX(NAME, VALUE) const char *const NAME[] = VALUE;
+#include "Options.inc"
+#undef PREFIX
+
+static const llvm::opt::OptTable::Info InfoTable[] = {
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+               HELPTEXT, METAVAR, VALUES)                                      \
+  {PREFIX,      NAME,      HELPTEXT,                                           \
+   METAVAR,     OPT_##ID,  llvm::opt::Option::KIND##Class,                     \
+   PARAM,       FLAGS,     OPT_##GROUP,                                        \
+   OPT_##ALIAS, ALIASARGS, VALUES},
+#include "Options.inc"
+#undef OPTION
+};
+class LLDBVSCodeOptTable : public llvm::opt::OptTable {
+public:
+  LLDBVSCodeOptTable() : OptTable(InfoTable, true) {}
+};
 
 typedef void (*RequestCallback)(const llvm::json::Object &command);
 
 enum LaunchMethod { Launch, Attach, AttachForSuspendedLaunch };
 
 SOCKET AcceptConnection(int portno) {
   // Accept a socket connection from any host on "portno".
   SOCKET newsockfd = -1;
static std::map<std::string, RequestCallback> g_request_handlers = {
       REQUEST_CALLBACK(_testGetTargetBreakpoints),
   };
 #undef REQUEST_CALLBACK
   return g_request_handlers;
 }
 
 } // anonymous namespace
 
+static void printHelp(LLDBVSCodeOptTable &table, llvm::StringRef tool_name) {
+  std::string usage_str = tool_name.str() + "options";
+  table.PrintHelp(llvm::outs(), usage_str.c_str(), "LLDB VSCode", false);
+
+  std::string examples = R"___(
+EXAMPLES:
+  The debug adapter can be started in two modes.
+
+  Running lldb-vscode without any arguments will start communicating with the
+  parent over stdio. Passing a port number causes lldb-vscode to start listening
+  for connections on that port.
+
+    lldb-vscode -p <port>
+
+  Passing --wait-for-debugger will pause the process at startup and wait for a
+  debugger to attach to the process.
+
+    lldb-vscode -g
+  )___";
+  llvm::outs() << examples;
+}
+
 int main(int argc, char *argv[]) {
 
   // Initialize LLDB first before we do anything.
   lldb::SBDebugger::Initialize();
 
-  if (argc == 2) {
-    const char *arg = argv[1];
+  int portno = -1;
+
+  LLDBVSCodeOptTable T;
+  unsigned MAI, MAC;
+  llvm::ArrayRef<const char *> ArgsArr = llvm::makeArrayRef(argv + 1, argc);
+  llvm::opt::InputArgList input_args = T.ParseArgs(ArgsArr, MAI, MAC);
+
+  if (input_args.hasArg(OPT_help)) {
+    printHelp(T, llvm::sys::path::filename(argv[0]));
+    return 0;
+  }
+
+  if (auto *arg = input_args.getLastArg(OPT_port)) {
+    auto optarg = arg->getValue();
+    char *remainder;
+    portno = strtol(optarg, &remainder, 0);
+    if (remainder == optarg || *remainder != '\0') {
+      fprintf(stderr, "'%s' is not a valid port number.\n", optarg);
+      exit(1);
+    }
+  }
+
 #if !defined(_WIN32)
-    if (strcmp(arg, "-g") == 0) {
-      printf("Paused waiting for debugger to attach (pid = %i)...\n", getpid());
-      pause();
-    } else {
-#else
-    {
+  if (input_args.hasArg(OPT_wait_for_debugger)) {
+  }
 #endif
-      int portno = atoi(arg);
-      printf("Listening on port %i...\n", portno);
-      SOCKET socket_fd = AcceptConnection(portno);
-      if (socket_fd >= 0) {
-        g_vsc.input.descriptor = StreamDescriptor::from_socket(socket_fd, true);
-        g_vsc.output.descriptor =
-            StreamDescriptor::from_socket(socket_fd, false);
-      } else {
-        exit(1);
-      }
-    }
+  if (portno != -1) {
+      g_vsc.output.descriptor = StreamDescriptor::from_socket(socket_fd, false);
   } else {
     g_vsc.input.descriptor = StreamDescriptor::from_file(fileno(stdin), false);
     g_vsc.output.descriptor =
         StreamDescriptor::from_file(fileno(stdout), false);
   }
   auto request_handlers = GetRequestHandlers();
   uint32_t packet_idx = 0;
   while (true) {