[llvm] [llvm-support][llvm-profdata] Use cl::Subcommand to organize show options. And look up in top-level as a fallback if a special subcommand doesn't have an option. (PR #71328)
Mingming Liu via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 8 21:50:01 PST 2023
minglotus-6 wrote:
> > The patch is currently adding new C++ structs to reduce the number of arguments.
> > Another option is to move option definitions (
> > https://github.com/llvm/llvm-project/blob/45ca24edc0b9f2541efb0ae3c668ee7af82e22f4/llvm/tools/llvm-profdata/llvm-profdata.cpp#L2954-L3042
> >
> > ) from function scope to a namespace (like `namespace show_prof{`), so the option definitions are visible to `showInstrProfile` and `showSampleProfile` (no need to pass long list of parameters) . The downside is that, not all options apply to both functions.
> > Thoughts about these two options?
>
> Moving show options to show_prof namespace looks reasonable to me. Need to make sure the --help option for the 'show' command works properly though.
With the usage of `cl::Subcommand`, `--help` should work out of the box. This [gist](https://gist.github.com/minglotus-6/2be4603f0706a692dfe8990f57cf86dc) is the output.
> > Since the options are now attached to ShowSubCommand, there is no longer the need to use show_options namespace.
>
> Different subcommand could have `cl::opt` variables of the same name. While `cl::SubCommand` could put registered option strings in a sub-command "namespace", , the `cl::opt` variables are put in per-subcommand namespaces.
I might need to take this back. It seems plausible to associate one `cl::opt` with multiple subcommands based on how registration works. I will try it out.
https://github.com/llvm/llvm-project/pull/71328
More information about the llvm-commits
mailing list