[PATCH] D35619: [ELF] - Introduce multiclass Eq helper for Options.td

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 20 05:25:00 PDT 2017


grimar added inline comments.


================
Comment at: ELF/Options.td:30
 
-def compress_debug_sections : J<"compress-debug-sections=">,
-  HelpText<"Compress DWARF debug sections">;
+defm compress_debug_sections : JSEq<"compress-debug-sections", "Compress DWARF debug sections">;
 
----------------
ruiu wrote:
> grimar wrote:
> > ruiu wrote:
> > > I think it is better to remove `help` from `JSEq` and keep HelpText for each option.
> > I would not do that because of next reasons:
> > 
> >   - It breaks our help output. Because libOption implementation renders all options which has help text. If I do that change,
> >      we will end up with duplicated options with the same help text.
> >   - It is inconsistent - we do not apply help text for other aliases. So we will have situations where we have `option with text and alias with text` vs `option with help text and alias without`.
> >   - It does not make sence to have help text set for alias. I mean practical sence, this information will be always probably be unused.
> > 
> > 
> > 
> I think showing both non-alias and alias options in the --help message is fine, and that's actually we want. I want everything to show up in the --help message as long as the linker accepts it.
What I mean that we probably may want:
`-library-path <value>, -library-path=<value> Some help text here`
Single line with list of options and one help text. That requires D35476 change.

But if we assign HelpText for both option and its alias, like you suggest, 
our output currently will be multilined and duplicated:
`-library-path <value> Some help text here`
`-library-path=<value> Some help text here`

That does not look as desired result, no ?


https://reviews.llvm.org/D35619





More information about the llvm-commits mailing list