[PATCH] D83530: [llvm-symbolizer] Switch command line parsing from llvm::cl to OptTable

Fri Jul 17 11:03:54 PDT 2020

MaskRay added a comment.

In D83530#2146686 <https://reviews.llvm.org/D83530#2146686>, @jhenderson wrote:

> Honestly, I'm not convinced by the change, looking at this patch, at least not without much more stuff being added to common library code. In particular, I don't like how significant code needs adding in this patch to handle a number of things that were previously handled by the CommandLine code, for example help handling, type validation, and unknown argument detection.

I think the library complexity is OK - 50 lines in OptTable.cpp which can be shared with other utilities.

> In high-level terms, I don't think I really care either way which command line processing option is used, if it is straightforward to use, so I'm not opposed to a switchover, but at the moment it seems to be significantly complicating things, not simplifying, for relatively little gain.

`cl::ParseCommandLineOptions` has a baked-in interface, with a bunch of customizations the user may or may not want to accept. For example, `-bool=0` & `-bool=false` may feel exotic for users coming from GNU getopt_long. `--flag` & `--no-flag` requires explicit position comparison.

OptTable appears to be a lighter interface with less customization. It does not call `Tbl.findNearest`, so the tool needs to do it by itself. Other than the few boilerplate lines I don't find the complexity is significant. We could move spelling checking to OptTable if more utilities want to use OptTable.

> (P.S. Don't forget to update documentation).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83530