[Lldb-commits] [PATCH] D89477: [lldb] Port lldb gdb-server to libOption

[Pavel Labath via Phabricator via lldb-commits]

labath marked 2 inline comments as done.
labath added inline comments.


================
Comment at: lldb/source/Utility/Args.cpp:178
 
+Args::Args(llvm::ArrayRef<llvm::StringRef> args) : Args() {
+  for (llvm::StringRef arg : args)
----------------
JDevlieghere wrote:
> Maybe StringList should have a ctor that takes an `ArrayRef<StringRef>`, but probably not worth the extra copies here. 
I think it can have that constructor if it's needed (though I'd rather delete that class altogether), but I think this constructor would be good to have regardless.


================
Comment at: lldb/tools/lldb-server/CMakeLists.txt:61
     LINK_COMPONENTS
       Support
 )
----------------
MaskRay wrote:
> Otherwise it fails in a BUILD_SHARED_LIBS=on build because the -Wl,-z,defs linker option requires a module to have fully specified dependencies
Thanks.


================
Comment at: lldb/tools/lldb-server/lldb-gdbserver.cpp:414
 
+  std::string HelpText =
+      "Use '" + Name + " --help' for a complete list of options.\n";
----------------
JDevlieghere wrote:
> Should we extract this code into a utility that we can use here and in the lldb driver?
I started implementing that but then I realized we the two are on the opposite sides of the SB API so we don't have a very good place to put it. I don't think this line of text is worth the trouble of figuring that out.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89477/new/

https://reviews.llvm.org/D89477