[PATCH] D123068: Add the /nologo flag to llvm-ml
Hans Wennborg via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 6 09:51:39 PDT 2022
hans accepted this revision.
hans added a comment.
I added a nitpick, but it's also fine if you want to land this now and worry about that later (or not).
================
Comment at: llvm/tools/llvm-ml/Opts.td:70
+ HelpText<"No effect as this tool never writes copyright data. "
+ "Included for parity">;
def output_file : MLJoinedOrSeparate<"Fo">, HelpText<"Names the output file">;
----------------
ayzhao wrote:
> hans wrote:
> > For clang-cl we don't include ignored options in the /help output at all. See the cl_ignored_group in clang/include/clang/Driver/Options.td and how "nologo" is handled there. Maybe llvm-ml should handle ignored options similarly?
> I don't think there's a consistent approach here - both llvm-cvtres and llvm-mt have help text for this flag (and in fact, I copied this help text from llvm-mt).
It's a nitpick obviously, but I don't think the current help message is good:
- It doesn't explain what the flag does: it assumes that the reader already knows what it's supposed to do (print the "copyright data"), and then explains that it won't do that
- "Included for parity" is redundant, since *all* flags are included for parity with the original tool.
A better help text would be something like "Don't print startup banner (default)" or something like that. But just having no help text would be just as good.
I didn't realize llvm-cvtres and llvm-mt have the same issue, but I didn't review that code :-)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D123068/new/
https://reviews.llvm.org/D123068
More information about the llvm-commits
mailing list