[Lldb-commits] [PATCH] D17724: Replace getopt with llvm::cl in lldb driver

[Greg Clayton via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

So this is nice that you cleaned up this one place where we can use LLVM's option parsing stuff, but it now makes the driver inconsistent with the rest of LLDB. It also makes it so you have to link some LLVM .a files into the lldb command line driver which we didn't have before. If we are going to make a change away from getopt_long(), I would rather change everything over. Otherwise we have our command line that behaves one way, and all of our command interpreter commands behaving another. And if we use option parsing from the driver, I would rather have the one consistent way that we handle options be exported through the lldb::SB API with something like:

lldb::SBOptionParser
lldb::SBOption

So while it seems nice to update just the driver, I would really rather us have one consistent way that we parse all command line options. For this to happen we would need to:

- switch over to using LLVM's option parser everywhere
- export the option stuff through the lldb::SB API so the driver can use it without having to directly link against llvm
- make the LLVM option parser self contained so it doesn't use global variables where each option parser has its own data (thread safe and safe to use concurrently)

In general we have been trying to do this:

- lldb.so or LLDB.framework can use llvm and clang APIs directly, nothing outside should
- if LLDB needs to export something that is fragile (like the LLVM option parser) it needs to wrap it up in a public API that will remain stable


http://reviews.llvm.org/D17724