From dcb6d212fdfb1db60bc5ba1c877897cb5ba0546c Mon Sep 17 00:00:00 2001 From: Justin Bogner Date: Mon, 14 Aug 2023 14:07:04 -0700 Subject: [PATCH] Reapply "[Option] Add "Visibility" field and clone the OptTable APIs to use it" This reverts commit 4e3b89483a6922d3f48670bb1c50a37f342918c6, with fixes for places I'd missed updating in lld and lldb. I've also renamed OptionVisibility::Default to "DefaultVis" to avoid ambiguity since the undecorated name has to be available anywhere Options.inc is included. Original message follows: This splits OptTable's "Flags" field into "Flags" and "Visibility", updates the places where we instantiate Option tables, and adds variants of the OptTable APIs that use Visibility mask instead of Include/Exclude flags. We need to do this to clean up a bunch of complexity in the clang driver's option handling - there's a whole slew of flags like CoreOption, NoDriverOption, and FlangOnlyOption there today to try to handle all of the permutations of flags that the various drivers need, but it really doesn't scale well, as can be seen by things like the somewhat recently introduced CLDXCOption. Instead, we'll provide an additive model for visibility that's separate from the other flags. For things like "HelpHidden", which is used as a "subtractive" modifier for option visibility, we leave that in "Flags" and handle it as a special case. Note that we don't actually update the users of the Include/Exclude APIs here or change the flags that exist in clang at all - that will come in a follow up that refactors clang's Options.td to use the increased flexibility this change allows. Differential Revision: https://reviews.llvm.org/D157149 --- clang-tools-extra/clangd/CompileCommands.cpp | 4 +- clang/lib/Frontend/CompilerInvocation.cpp | 12 +- lld/COFF/DriverUtils.cpp | 1 + lld/MachO/DriverUtils.cpp | 10 +- lld/MinGW/Driver.cpp | 11 +- lld/wasm/Driver.cpp | 11 +- lldb/tools/driver/Driver.cpp | 2 + lldb/tools/lldb-server/lldb-gdbserver.cpp | 2 + lldb/tools/lldb-vscode/lldb-vscode.cpp | 2 + llvm/include/llvm/Option/OptParser.td | 14 +- llvm/include/llvm/Option/OptTable.h | 127 +++++++++++------ llvm/include/llvm/Option/Option.h | 9 ++ .../JITLink/COFFDirectiveParser.cpp | 1 + llvm/lib/Option/OptTable.cpp | 128 +++++++++++++++--- .../llvm-dlltool/DlltoolDriver.cpp | 1 + llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp | 1 + llvm/tools/dsymutil/dsymutil.cpp | 1 + llvm/tools/llvm-cvtres/llvm-cvtres.cpp | 1 + llvm/tools/llvm-cxxfilt/llvm-cxxfilt.cpp | 1 + .../tools/llvm-debuginfod/llvm-debuginfod.cpp | 1 + llvm/tools/llvm-dwarfutil/llvm-dwarfutil.cpp | 1 + llvm/tools/llvm-dwp/llvm-dwp.cpp | 1 + llvm/tools/llvm-lipo/llvm-lipo.cpp | 1 + llvm/tools/llvm-mt/llvm-mt.cpp | 1 + llvm/tools/llvm-objcopy/ObjcopyOptions.cpp | 1 + llvm/tools/llvm-rc/llvm-rc.cpp | 1 + llvm/tools/llvm-strings/llvm-strings.cpp | 1 + .../tools/llvm-symbolizer/llvm-symbolizer.cpp | 1 + .../llvm-tli-checker/llvm-tli-checker.cpp | 1 + .../Option/OptionMarshallingTest.cpp | 6 +- llvm/unittests/Option/OptionParsingTest.cpp | 47 +++++++ llvm/unittests/Option/Opts.td | 7 + llvm/utils/TableGen/OptParserEmitter.cpp | 21 ++- 33 files changed, 349 insertions(+), 81 deletions(-) diff --git a/clang-tools-extra/clangd/CompileCommands.cpp b/clang-tools-extra/clangd/CompileCommands.cpp index 00f4c4ca9fef0a..3fce1b8bcb490c 100644 --- a/clang-tools-extra/clangd/CompileCommands.cpp +++ b/clang-tools-extra/clangd/CompileCommands.cpp @@ -494,7 +494,7 @@ llvm::ArrayRef ArgStripper::rulesFor(llvm::StringRef Arg) { static constexpr llvm::ArrayRef NAME( \ NAME##_init, std::size(NAME##_init) - 1); #define OPTION(PREFIX, PREFIXED_NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, \ - FLAGS, PARAM, HELP, METAVAR, VALUES) \ + FLAGS, VISIBILITY, PARAM, HELP, METAVAR, VALUES) \ Prefixes[DriverID::OPT_##ID] = PREFIX; #include "clang/Driver/Options.inc" #undef OPTION @@ -506,7 +506,7 @@ llvm::ArrayRef ArgStripper::rulesFor(llvm::StringRef Arg) { const void *AliasArgs; } AliasTable[] = { #define OPTION(PREFIX, PREFIXED_NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, \ - FLAGS, PARAM, HELP, METAVAR, VALUES) \ + FLAGS, VISIBILITY, PARAM, HELP, METAVAR, VALUES) \ {DriverID::OPT_##ID, DriverID::OPT_##ALIAS, ALIASARGS}, #include "clang/Driver/Options.inc" #undef OPTION diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index 87190093767b05..0d7a497cbd42af 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -423,9 +423,9 @@ static T extractMaskValue(T KeyPath) { #define PARSE_OPTION_WITH_MARSHALLING( \ ARGS, DIAGS, PREFIX_TYPE, SPELLING, ID, KIND, GROUP, ALIAS, ALIASARGS, \ - FLAGS, PARAM, HELPTEXT, METAVAR, VALUES, SHOULD_PARSE, ALWAYS_EMIT, \ - KEYPATH, DEFAULT_VALUE, IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, \ - DENORMALIZER, MERGER, EXTRACTOR, TABLE_INDEX) \ + FLAGS, VISIBILITY, PARAM, HELPTEXT, METAVAR, VALUES, SHOULD_PARSE, \ + ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE, IMPLIED_CHECK, IMPLIED_VALUE, \ + NORMALIZER, DENORMALIZER, MERGER, EXTRACTOR, TABLE_INDEX) \ if ((FLAGS)&options::CC1Option) { \ KEYPATH = MERGER(KEYPATH, DEFAULT_VALUE); \ if (IMPLIED_CHECK) \ @@ -440,9 +440,9 @@ static T extractMaskValue(T KeyPath) { // with lifetime extension of the reference. #define GENERATE_OPTION_WITH_MARSHALLING( \ CONSUMER, PREFIX_TYPE, SPELLING, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, \ - PARAM, HELPTEXT, METAVAR, VALUES, SHOULD_PARSE, ALWAYS_EMIT, KEYPATH, \ - DEFAULT_VALUE, IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER, \ - MERGER, EXTRACTOR, TABLE_INDEX) \ + VISIBILKITY, PARAM, HELPTEXT, METAVAR, VALUES, SHOULD_PARSE, ALWAYS_EMIT, \ + KEYPATH, DEFAULT_VALUE, IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, \ + DENORMALIZER, MERGER, EXTRACTOR, TABLE_INDEX) \ if ((FLAGS)&options::CC1Option) { \ [&](const auto &Extracted) { \ if (ALWAYS_EMIT || \ diff --git a/lld/COFF/DriverUtils.cpp b/lld/COFF/DriverUtils.cpp index ca9f8a47b0235f..fa15f816fb5fcd 100644 --- a/lld/COFF/DriverUtils.cpp +++ b/lld/COFF/DriverUtils.cpp @@ -38,6 +38,7 @@ #include using namespace llvm::COFF; +using namespace llvm::opt; using namespace llvm; using llvm::sys::Process; diff --git a/lld/MachO/DriverUtils.cpp b/lld/MachO/DriverUtils.cpp index b0be96f3592c4a..17499451382a55 100644 --- a/lld/MachO/DriverUtils.cpp +++ b/lld/MachO/DriverUtils.cpp @@ -44,9 +44,13 @@ using namespace lld::macho; // Create table mapping all options defined in Options.td static constexpr OptTable::Info optInfo[] = { -#define OPTION(X1, X2, ID, KIND, GROUP, ALIAS, X7, X8, X9, X10, X11, X12) \ - {X1, X2, X10, X11, OPT_##ID, Option::KIND##Class, \ - X9, X8, OPT_##GROUP, OPT_##ALIAS, X7, X12}, +#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, \ + VISIBILITY, PARAM, HELPTEXT, METAVAR, VALUES) \ + {PREFIX, NAME, HELPTEXT, \ + METAVAR, OPT_##ID, opt::Option::KIND##Class, \ + PARAM, FLAGS, VISIBILITY, \ + OPT_##GROUP, OPT_##ALIAS, ALIASARGS, \ + VALUES}, #include "Options.inc" #undef OPTION }; diff --git a/lld/MinGW/Driver.cpp b/lld/MinGW/Driver.cpp index 67c45051a703a8..3a6d52ffbd5510 100644 --- a/lld/MinGW/Driver.cpp +++ b/lld/MinGW/Driver.cpp @@ -51,6 +51,7 @@ #endif using namespace lld; +using namespace llvm::opt; using namespace llvm; // Create OptTable @@ -71,9 +72,13 @@ enum { // Create table mapping all options defined in Options.td static constexpr opt::OptTable::Info infoTable[] = { -#define OPTION(X1, X2, ID, KIND, GROUP, ALIAS, X7, X8, X9, X10, X11, X12) \ - {X1, X2, X10, X11, OPT_##ID, opt::Option::KIND##Class, \ - X9, X8, OPT_##GROUP, OPT_##ALIAS, X7, X12}, +#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, \ + VISIBILITY, PARAM, HELPTEXT, METAVAR, VALUES) \ + {PREFIX, NAME, HELPTEXT, \ + METAVAR, OPT_##ID, opt::Option::KIND##Class, \ + PARAM, FLAGS, VISIBILITY, \ + OPT_##GROUP, OPT_##ALIAS, ALIASARGS, \ + VALUES}, #include "Options.inc" #undef OPTION }; diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp index c98d3522a5a227..00920fd5b4236a 100644 --- a/lld/wasm/Driver.cpp +++ b/lld/wasm/Driver.cpp @@ -39,6 +39,7 @@ using namespace llvm; using namespace llvm::object; +using namespace llvm::opt; using namespace llvm::sys; using namespace llvm::wasm; @@ -111,9 +112,13 @@ bool link(ArrayRef args, llvm::raw_ostream &stdoutOS, // Create table mapping all options defined in Options.td static constexpr opt::OptTable::Info optInfo[] = { -#define OPTION(X1, X2, ID, KIND, GROUP, ALIAS, X7, X8, X9, X10, X11, X12) \ - {X1, X2, X10, X11, OPT_##ID, opt::Option::KIND##Class, \ - X9, X8, OPT_##GROUP, OPT_##ALIAS, X7, X12}, +#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, \ + VISIBILITY, PARAM, HELPTEXT, METAVAR, VALUES) \ + {PREFIX, NAME, HELPTEXT, \ + METAVAR, OPT_##ID, opt::Option::KIND##Class, \ + PARAM, FLAGS, VISIBILITY, \ + OPT_##GROUP, OPT_##ALIAS, ALIASARGS, \ + VALUES}, #include "Options.inc" #undef OPTION }; diff --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp index 3460842ea8d197..f6e17a63dbec90 100644 --- a/lldb/tools/driver/Driver.cpp +++ b/lldb/tools/driver/Driver.cpp @@ -50,6 +50,8 @@ using namespace lldb; using namespace llvm; namespace { +using namespace llvm::opt; + enum ID { OPT_INVALID = 0, // This is not an option ID. #define OPTION(...) LLVM_MAKE_OPT_ID(__VA_ARGS__), diff --git a/lldb/tools/lldb-server/lldb-gdbserver.cpp b/lldb/tools/lldb-server/lldb-gdbserver.cpp index 0b829d2d0f0540..38c00e83e7b079 100644 --- a/lldb/tools/lldb-server/lldb-gdbserver.cpp +++ b/lldb/tools/lldb-server/lldb-gdbserver.cpp @@ -272,6 +272,8 @@ void ConnectToRemote(MainLoop &mainloop, } namespace { +using namespace llvm::opt; + enum ID { OPT_INVALID = 0, // This is not an option ID. #define OPTION(...) LLVM_MAKE_OPT_ID(__VA_ARGS__), diff --git a/lldb/tools/lldb-vscode/lldb-vscode.cpp b/lldb/tools/lldb-vscode/lldb-vscode.cpp index b6917a37d83174..407259690bbc4b 100644 --- a/lldb/tools/lldb-vscode/lldb-vscode.cpp +++ b/lldb/tools/lldb-vscode/lldb-vscode.cpp @@ -76,6 +76,8 @@ typedef int socklen_t; using namespace lldb_vscode; namespace { +using namespace llvm::opt; + enum ID { OPT_INVALID = 0, // This is not an option ID. #define OPTION(...) LLVM_MAKE_OPT_ID(__VA_ARGS__), diff --git a/llvm/include/llvm/Option/OptParser.td b/llvm/include/llvm/Option/OptParser.td index 94b945defac110..7bbee1da643b8c 100644 --- a/llvm/include/llvm/Option/OptParser.td +++ b/llvm/include/llvm/Option/OptParser.td @@ -73,6 +73,13 @@ def RenderJoined : OptionFlag; // (only sensible on joined options). def RenderSeparate : OptionFlag; +// Define Visibility categories + +class OptionVisibility {} + +// Explicit specifier for default visibility +def DefaultVis : OptionVisibility; + // Define the option group class. class OptionGroup { @@ -81,6 +88,7 @@ class OptionGroup { string HelpText = ?; OptionGroup Group = ?; list Flags = []; + list Visibility = []; } // Define the option class. @@ -97,6 +105,7 @@ class Option prefixes, string name, OptionKind kind> { string Values = ?; code ValuesCode = ?; list Flags = []; + list Visibility = [DefaultVis]; OptionGroup Group = ?; Option Alias = ?; list AliasArgs = []; @@ -141,6 +150,9 @@ class Alias