[PATCH] D69501: [CommandLine] Add inline ArgName printing

Don Hinton via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 28 10:34:01 PDT 2019


hintonda added a comment.

In D69501#1723723 <https://reviews.llvm.org/D69501#1723723>, @dsprenkels wrote:

> @jhenderson, I added the test.
>
> The only person that has modified `llvm/lib/Support/CommandLine.cpp` a couple of times in the last year is @hintonda.  I added them as a reviewer.


Thanks for the patch.

I originally added this to cleanup printing help, so I appreciate you fixing the error messages too.



================
Comment at: llvm/lib/Support/CommandLine.cpp:136
+public:
+  PrintArgInline(StringRef ArgName) : ArgName(ArgName) {}
+  friend raw_ostream &operator<<(raw_ostream &OS, const PrintArgInline &);
----------------
Instead of adding a new class that's only used 3 times, please consider passing an additional prefix padding variable that defaults to the current behavior, 2, and then pass 0 for the three error messages you fixed.  

That way, you can forward the padding down to argPrefix() and clean it up while you're at it -- it could use a bit of refactoring too.


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

https://reviews.llvm.org/D69501





More information about the llvm-commits mailing list