[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:
> > > > 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.
> What about options that have other aliases ? For example if I would do this change
> for `entry` option, its declaration should be:
> 
> defm entry: Eq<"entry">, HelpText<"Name of entry point symbol">, MetaVarName<"<entry>">;
> def alias_entry_e: JoinedOrSeparate<["-"], "e">, HelpText<"Name of entry point symbol">, Alias<entry>;
> 
> To produce output that you want:
> ```
> --entry  <entry>   Name of entry point symbol
> --entry= <entry>  Name of entry point symbol
> -e <entry>            Name of entry point symbol
> ```
> 
> Do you want to define HelpText for aliases like alias_entry_e too ?
> 
I don't care about that that much. It at least doesn't look bad to me, it is easy to understand and easy to parse. Even if we want something else, I don't like to do everything at once.


https://reviews.llvm.org/D35619