Fāng-ruì Sòng via llvm-dev
2021-Jul-02 18:27 UTC
[llvm-dev] Binary utilities: switch command line parsing from llvm::cl to OptTable (byproduct: drop -long-option?)
On Fri, Jul 2, 2021 at 11:17 AM David Blaikie <dblaikie at gmail.com> wrote:> On Fri, Jul 2, 2021 at 10:15 AM Fāng-ruì Sòng via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > >> llvm/tools/ include some binary utilities used as replacement for GNU >> binutils, e.g. llvm-objcopy, llvm-symbolizer, llvm-nm. >> In some old threads people discussed some drawbacks of using cl::opt for >> user-facing utilities (I cannot find them now). >> > > Would be good to describe some of the known drawbacks/expected benefits. >The summary is the list of benefits:) The drawback is some initial boilerplate (e.g. llvm-tblgen -gen-opt-parser-defs in CMakeLists.txt, class NmOptTable in code). 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 https://reviews.llvm.org/D105330) But this doesn't tend to increase complexity because the cl::list<std::string> will need per-value verification anyway.> 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. >OptTable is used as a local variable. So yes, it avoids global constructors, thus avoiding cl::opt option name collision. "If we decide to support binary utility multiplexing" below mentioned this point. Switching to OptTable is an appealing solution. I have prepared two patches>> for two binary utilities: llvm-nm and llvm-strings. >> >> * llvm-strings https://reviews.llvm.org/D104889 >> * llvm-nm https://reviews.llvm.org/D105330 >> >> llvm-symbolizer was switched last year. llvm-objdump was switched by >> thakis earlier this year. >> >> The switch can fix some corners with lib/Support/CommandLine.cpp. Here is >> a summary: >> >> * -t=d is removed (equal sign after a short option). Use -t d instead. >> * --demangle=0 (=0 to disable a boolean option) is removed. Omit the >> option or use --no-demangle instead. >> * 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) >> * grouped short options can be specified with one line >> `setGroupedShortOptions`, instead of adding cl::Grouping to every short >> options. >> * 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 https://reviews.llvm.org/D104363) >> * If we decide to support binary utility multiplexting ( >> https://reviews.llvm.org/D104686), we will not get conflicting options. >> An option may have different meanings in different utilities (especially >> for one-letter options). >> >> *I expect that most users will not observe any difference.* >> >> There is a related topic whether we should disallow the single-dash >> `-long-option` form. >> > (Discussed in 2019: >> https://lists.llvm.org/pipermail/llvm-dev/2019-April/131786.html Accept >> --long-option but not -long-option for llvm binary utilities) >> *I'd like to disallow -long-option but may want to do this in a separate >> change.* >> > > 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. >Thanks for chiming in.> >> 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. >> >> I can think of potential pushback for some Mach-O specific options, e.g. >> nm -arch >> http://www.manpagez.com/man/1/nm/osx-10.12.6.php says `-arch` has one >> dash. >> If such options may have problems, we can keep supporting one dash forms. >> With OptTable, allowing one-dash forms for a specific option is easy. >> >>-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210702/165d4bf4/attachment.html>
David Blaikie via llvm-dev
2021-Jul-02 18:40 UTC
[llvm-dev] Binary utilities: switch command line parsing from llvm::cl to OptTable (byproduct: drop -long-option?)
On Fri, Jul 2, 2021 at 11:27 AM Fāng-ruì Sòng <maskray at google.com> wrote:> On Fri, Jul 2, 2021 at 11:17 AM David Blaikie <dblaikie at gmail.com> wrote: > >> On Fri, Jul 2, 2021 at 10:15 AM Fāng-ruì Sòng via llvm-dev < >> llvm-dev at lists.llvm.org> wrote: >> >>> llvm/tools/ include some binary utilities used as replacement for GNU >>> binutils, e.g. llvm-objcopy, llvm-symbolizer, llvm-nm. >>> In some old threads people discussed some drawbacks of using cl::opt for >>> user-facing utilities (I cannot find them now). >>> >> >> Would be good to describe some of the known drawbacks/expected benefits. >> > > The summary is the list of benefits:) >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. 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.> The drawback is some initial boilerplate (e.g. llvm-tblgen > -gen-opt-parser-defs in CMakeLists.txt, class NmOptTable in code). > 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 https://reviews.llvm.org/D105330) > But this doesn't tend to increase complexity because the > cl::list<std::string> will need per-value verification anyway. > > >> 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. >> > > OptTable is used as a local variable. So yes, it avoids global > constructors, >Nice :)> thus avoiding cl::opt option name collision. > "If we decide to support binary utility multiplexing" below mentioned this > point. > > Switching to OptTable is an appealing solution. I have prepared two >>> patches for two binary utilities: llvm-nm and llvm-strings. >>> >>> * llvm-strings https://reviews.llvm.org/D104889 >>> * llvm-nm https://reviews.llvm.org/D105330 >>> >>> llvm-symbolizer was switched last year. llvm-objdump was switched by >>> thakis earlier this year. >>> >>> The switch can fix some corners with lib/Support/CommandLine.cpp. Here >>> is a summary: >>> >>> * -t=d is removed (equal sign after a short option). Use -t d instead. >>> * --demangle=0 (=0 to disable a boolean option) is removed. Omit the >>> option or use --no-demangle instead. >>> * 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) >>> * grouped short options can be specified with one line >>> `setGroupedShortOptions`, instead of adding cl::Grouping to every short >>> options. >>> * 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 https://reviews.llvm.org/D104363) >>> * If we decide to support binary utility multiplexting ( >>> https://reviews.llvm.org/D104686), we will not get conflicting options. >>> An option may have different meanings in different utilities (especially >>> for one-letter options). >>> >>> *I expect that most users will not observe any difference.* >>> >>> There is a related topic whether we should disallow the single-dash >>> `-long-option` form. >>> >> (Discussed in 2019: >>> https://lists.llvm.org/pipermail/llvm-dev/2019-April/131786.html Accept >>> --long-option but not -long-option for llvm binary utilities) >>> *I'd like to disallow -long-option but may want to do this in a separate >>> change.* >>> >> >> 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. >> > > Thanks for chiming in. > > >> >>> 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. >>> >>> I can think of potential pushback for some Mach-O specific options, e.g. >>> nm -arch >>> http://www.manpagez.com/man/1/nm/osx-10.12.6.php says `-arch` has one >>> dash. >>> If such options may have problems, we can keep supporting one dash forms. >>> With OptTable, allowing one-dash forms for a specific option is easy. >>> >>>-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210702/7ed61578/attachment.html>