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

Zhihao Yuan via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 28 23:20:26 PDT 2020


lichray added a comment.

> You seem to be throwing out all the good things to simplify a couple of things though. Here is a (possibly non-exhaustive) list of things that were previously handled by the library, and now llvm-symbolizer has to do itself:
>
> 1. Response file usage.
> 2. Spell-checking of option names.
> 3. Argument type checking and conversion (e.g. to integers, enum, etc).
> 4. Integer argument signedness checking.
> 5. Unknown argument handling.
> 6. Options specified via environment variables.
> 7. Help text printing.

When looking at refactoring, I would emphasis direction over how close it is to some "ultimate" form.  Global variables + `llvm::cl` isn't really sustainable, and the patch proposes to migrate away from it.  To us, a community dealing with languages, it is quite natural to solve the problem of command-line parsing with a language.  So I like the proposed solution.

The patch has implemented the features mentioned above, meets the minimal bar for refactoring -- not breaking anything, including user experience.  They may not be implemented in some ideal places, but the progress models separation of concern.  If we find that some demands are common enough, we can put them in `OptTable` API later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83530



More information about the llvm-commits mailing list