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

[Rui Ueyama via Phabricator via llvm-commits]

ruiu 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">;
 
----------------
grimar wrote:
> 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 ?
I think that is fine.


https://reviews.llvm.org/D35619