[PATCH] D36782: [Bash-autocompletion] Add support for static analyzer flags

Rui Ueyama via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 16 14:23:11 PDT 2017


ruiu added a comment.

This patch allows us to embed a piece of C++ code to each command line option to construct a list of argument candidates at runtime. With this patch, .inc files generated by OptParserEmitter contain C macros that in turn include other .inc files. That is a flexible mechanism but can be too flexible and fragile, as you can write any code in .td file, and pieces of C++ code you write in .td files are not verified even for syntax validation until that nested text inclusions are processed by a C++ compiler. It does two-stages of code generation, one is done by OptTable and the other is done by C macros. I can imagine that a single typo in a .td file can cause a mysterious error that is hard to debug. I feel like, even though command line processing is not that easy, I wonder if it is that complicated to require two-stage code generation.

That being said, I don't oppose to this patch, as people who know more about this code seem to like this approach. I'm just expressing my concern.



================
Comment at: llvm/include/llvm/Option/OptTable.h:60
 private:
   /// \brief The static option information table.
+  std::vector<Info> OptionInfos;
----------------
teemperor wrote:
> This is no longer static :)
Just remove "non-static" instead of saying this is non-static, as being non-const is not special.


https://reviews.llvm.org/D36782





More information about the cfe-commits mailing list