[PATCH] D53517: [FileCheck] Parse command-line options from FILECHECK_OPTS

[Paul Robinson via Phabricator via llvm-commits]

probinson accepted this revision.
probinson added a comment.
This revision is now accepted and ready to land.

LGTM.
A patch to report an error on multiple -D of the same variable is probably a good thing, particularly since it doesn't work properly.



================
Comment at: llvm/utils/FileCheck/FileCheck.cpp:117
   InitLLVM X(argc, argv);
-  cl::ParseCommandLineOptions(argc, argv);
+  cl::ParseCommandLineOptions(argc, argv, "", nullptr, "FILECHECK_OPTS");
 
----------------
jdenny wrote:
> probinson wrote:
> > For arguments such as "" and nullptr, generally the style is to put the parameter name in a comment so the reader knows what is being omitted.  So in this case something like:
> > ```
> > ..., /*Overview*/ "", /*Errs*/ nullptr, "FILECHECK_OPTS");
> > ```
> > 
> Thanks. Just out of curiosity, is this style documented somewhere?  I'll make the change regardless.
Interesting, I thought it was but now that I look, I don't see it in the coding standard. I do see this used in the project and I like it, so at some point I'll probably put up a patch to add it to the docs.


https://reviews.llvm.org/D53517