<div dir="ltr"><div dir="ltr">On Fri, Jul 2, 2021 at 11:27 AM Fāng-ruì Sòng <<a href="mailto:maskray@google.com">maskray@google.com</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">On Fri, Jul 2, 2021 at 11:17 AM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">On Fri, Jul 2, 2021 at 10:15 AM Fāng-ruì Sòng via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">llvm/tools/ include some binary utilities used as replacement for GNU binutils, e.g. llvm-objcopy, llvm-symbolizer, llvm-nm.<br>In some old threads people discussed some drawbacks of using cl::opt for user-facing utilities (I cannot find them now).<br></div></blockquote><div><br>Would be good to describe some of the known drawbacks/expected benefits.<br></div></div></div></blockquote><div><br></div><div>The summary is the list of benefits:)</div></div></div></blockquote><div><br>Ah, it looks more like a list of changes, not necessarily benefits - removing certain syntaxes seems generally like a cost to me (potential to break existing users), rather than an outright benefit.<br><br>The API benefits sound nice, though presumably some could be retrofitted to cl::opt if that was the only goal. Side benefits in addition to removing global ctors are nice to have.<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>The drawback is some initial boilerplate (e.g. llvm-tblgen -gen-opt-parser-defs in CMakeLists.txt, class NmOptTable in code).</div><div>The handling of comma separated options -arch=x86_64,arm64 doesn't have direct OptTable support. llvm::SplitString is needed (just search for SplitString in <a href="https://reviews.llvm.org/D105330" target="_blank">https://reviews.llvm.org/D105330</a>)</div><div>But this doesn't tend to increase complexity because the cl::list<std::string> will need per-value verification anyway.</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>One potential one (though I don't recall it being discussed recently) would be that maybe this addresses the issue of global ctors in cl::opt? Does OptTable avoid/not use global constructors? That would be nice - it's an ongoing issue that LLVM library users pay for command line argument support they have no need for in the form of global ctor execution time.</div></div></div></blockquote><div><br></div><div>OptTable is used as a local variable. So yes, it avoids global constructors, </div></div></div></blockquote><div><br></div><div>Nice :)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>thus avoiding cl::opt option name collision.</div><div>"If we decide to support binary utility multiplexing" below mentioned this point.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Switching to OptTable is an appealing solution. I have prepared two patches for two binary utilities: llvm-nm and llvm-strings.<br><br>* llvm-strings <a href="https://reviews.llvm.org/D104889" target="_blank">https://reviews.llvm.org/D104889</a><br>* llvm-nm <a href="https://reviews.llvm.org/D105330" target="_blank">https://reviews.llvm.org/D105330</a><br><br>llvm-symbolizer was switched last year. llvm-objdump was switched by thakis earlier this year.<br><br>The switch can fix some corners with lib/Support/CommandLine.cpp. Here is a summary:<br><br>* -t=d is removed (equal sign after a short option). Use -t d instead.<br>* --demangle=0 (=0 to disable a boolean option) is removed. Omit the option or use --no-demangle instead.<br>* To support boolean options (e.g. --demangle --no-demangle), we don't need to compare their positions (if (NoDemangle.getPosition() > Demangle.getPosition()) , see llvm-nm.cpp)<br>* grouped short options can be specified with one line `setGroupedShortOptions`, instead of adding cl::Grouping to every short options.<br>* We don't need to add cl::cat to every option and call `HideUnrelatedOptions` to hide unrelated options from --help. The issue would happen with cl::opt tools if linker garbage collection is disabled or libLLVM-13git.so is used. (See <a href="https://reviews.llvm.org/D104363" target="_blank">https://reviews.llvm.org/D104363</a>)<br>* If we decide to support binary utility multiplexting (<a href="https://reviews.llvm.org/D104686" target="_blank">https://reviews.llvm.org/D104686</a>), we will not get conflicting options. An option may have different meanings in different utilities (especially for one-letter options).<br><br><b>I expect that most users will not observe any difference.</b><br><br>There is a related topic whether we should disallow the single-dash `-long-option` form.</div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">(Discussed in 2019: <a href="https://lists.llvm.org/pipermail/llvm-dev/2019-April/131786.html" target="_blank">https://lists.llvm.org/pipermail/llvm-dev/2019-April/131786.html</a> Accept --long-option but not -long-option for llvm binary utilities)<br><b>I'd like to disallow -long-option but may want to do this in a separate change.</b></div></blockquote><div><br>I'd say definitely do this as a separate change. I expect there'd be a long tail of users after this change ships in an LLVM release, etc, such that we may want to undo some amount of it a long time after the change is made.<br></div></div></div></blockquote><div><br></div><div>Thanks for chiming in. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br>The main point is that (1) grouped short options have syntax conflict with one-dash long options. (2) the GNU getopt_long style two-dash long option is much more popular.<br><br>I can think of potential pushback for some Mach-O specific options, e.g. nm -arch<br><a href="http://www.manpagez.com/man/1/nm/osx-10.12.6.php" target="_blank">http://www.manpagez.com/man/1/nm/osx-10.12.6.php</a> says `-arch` has one dash.<br>If such options may have problems, we can keep supporting one dash forms.<br>With OptTable, allowing one-dash forms for a specific option is easy.<br></div><br></blockquote></div></div></blockquote></div></div>
</blockquote></div></div>