https://github.com/MaskRay updated https://github.com/llvm/llvm-project/pull/218845
>From 5cbe94a17858f9bf6ec6aa224671293fb6fd12b7 Mon Sep 17 00:00:00 2001 From: Fangrui Song <[email protected]> Date: Tue, 25 Aug 2026 23:06:46 -0700 Subject: [PATCH 1/3] [OptTable] Store Info strings in the string table Change HelpText, MetaVar, AliasArgs, and Values from `const char *` to StringTable::Offset, making the fields smaller, and removing dynamic relocations in .data.rel.ro in PIC links. Values declared with ValuesCode are computed by generated code that TableGen cannot see, so they move into a separate OptionValuesCodeTable; only clang has any. An empty HelpText is now indistinguishable from an absent one, so an alias with HelpText<""> no longer suppresses the aliased option's help text under ShowAllAliases. --- clang-tools-extra/clangd/CompileCommands.cpp | 4 +- clang/lib/Options/DriverOptions.cpp | 4 +- llvm/include/llvm/Option/OptTable.h | 58 +++++--- llvm/include/llvm/Option/Option.h | 19 ++- llvm/lib/Option/OptTable.cpp | 45 ++++--- llvm/lib/Option/Option.cpp | 14 +- llvm/unittests/Option/OptionParsingTest.cpp | 11 ++ llvm/unittests/Option/Opts.td | 1 + llvm/utils/TableGen/OptionParserEmitter.cpp | 132 +++++++++++-------- 9 files changed, 176 insertions(+), 112 deletions(-) diff --git a/clang-tools-extra/clangd/CompileCommands.cpp b/clang-tools-extra/clangd/CompileCommands.cpp index b5965d163d7db..2ba9446aa57dc 100644 --- a/clang-tools-extra/clangd/CompileCommands.cpp +++ b/clang-tools-extra/clangd/CompileCommands.cpp @@ -495,7 +495,7 @@ llvm::ArrayRef<ArgStripper::Rule> ArgStripper::rulesFor(llvm::StringRef Arg) { struct { DriverID ID; DriverID AliasID; - const void *AliasArgs; + unsigned AliasArgsOffset; } AliasTable[] = { #define OPTION(PREFIX, PREFIXED_NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, \ FLAGS, VISIBILITY, PARAM, HELPTEXT, HELPTEXTSFORVARIANTS, \ @@ -505,7 +505,7 @@ llvm::ArrayRef<ArgStripper::Rule> ArgStripper::rulesFor(llvm::StringRef Arg) { #undef OPTION }; for (auto &E : AliasTable) - if (E.AliasID != DriverID::OPT_INVALID && E.AliasArgs == nullptr) + if (E.AliasID != DriverID::OPT_INVALID && !E.AliasArgsOffset) AddAlias(E.ID, E.AliasID); auto Result = std::make_unique<TableTy>(); diff --git a/clang/lib/Options/DriverOptions.cpp b/clang/lib/Options/DriverOptions.cpp index c9c826a439123..133fc0babae14 100644 --- a/clang/lib/Options/DriverOptions.cpp +++ b/clang/lib/Options/DriverOptions.cpp @@ -41,7 +41,9 @@ class DriverOptTable : public PrecomputedOptTable { public: DriverOptTable() : PrecomputedOptTable(OptionStrTable, OptionPrefixesTable, InfoTable, - OptionPrefixesUnion) {} + OptionPrefixesUnion, /*IgnoreCase=*/false, + /*SubCommands=*/{}, /*SubCommandIDsTable=*/{}, + OptionValuesCodeTable) {} }; } // anonymous namespace diff --git a/llvm/include/llvm/Option/OptTable.h b/llvm/include/llvm/Option/OptTable.h index 45083b31c11f4..58be2bda0d30a 100644 --- a/llvm/include/llvm/Option/OptTable.h +++ b/llvm/include/llvm/Option/OptTable.h @@ -60,11 +60,14 @@ class LLVM_ABI OptTable { const char *Usage; }; + /// An option ID together with the values produced by TableGen `ValuesCode`. + using ValuesCodeEntry = std::pair<unsigned, const char *>; + /// Entry for a single option instance in the option data table. struct Info { unsigned PrefixesOffset; StringTable::Offset PrefixedNameOffset; - const char *HelpText; + StringTable::Offset HelpTextOffset; // Help text for specific visibilities. A list of pairs, where each pair // is a list of visibilities and a specific help string for those // visibilities. If no help text is found in this list for the visibility of @@ -73,10 +76,10 @@ class LLVM_ABI OptTable { // here if you need more entries and adjust the constants in // OptionParserEmitter::EmitHelpTextsForVariants. std::array<std::pair<std::array<unsigned int, 2 /*MaxVisibilityPerHelp*/>, - const char *>, + StringTable::Offset>, 1 /*MaxVisibilityHelp*/> HelpTextsForVariants; - const char *MetaVar; + StringTable::Offset MetaVarOffset; unsigned ID; unsigned char Kind; unsigned char Param; @@ -84,8 +87,11 @@ class LLVM_ABI OptTable { unsigned int Visibility; unsigned short GroupID; unsigned short AliasID; - const char *AliasArgs; - const char *Values; + /// The alias arguments as a \0 separated list terminated by an empty + /// string, e.g. "foo\0bar\0". + StringTable::Offset AliasArgsOffset; + /// The possible values as a comma separated list; see ValuesCodeTable. + StringTable::Offset ValuesOffset; // Offset into OptTable's SubCommandIDsTable. unsigned SubCommandIDsOffset; @@ -179,6 +185,10 @@ class LLVM_ABI OptTable { /// The subcommand IDs table. ArrayRef<unsigned> SubCommandIDsTable; + /// Values of options declared with TableGen `ValuesCode`: only the generated + /// code knows them, so they cannot go in the string table. + ArrayRef<ValuesCodeEntry> ValuesCodeTable; + bool GroupedShortOptions = false; bool DashDashParsing = false; const char *EnvVar = nullptr; @@ -205,6 +215,15 @@ class LLVM_ABI OptTable { return OptionInfos[id - 1]; } + StringRef getOptionValues(const Info &I) const { + if (I.ValuesOffset.value()) + return (*StrTable)[I.ValuesOffset]; + for (const auto &[ID, Values] : ValuesCodeTable) + if (ID == I.ID) + return Values; + return StringRef(); + } + std::unique_ptr<Arg> parseOneArgGrouped(InputArgList &Args, unsigned &Index) const; @@ -215,7 +234,8 @@ class LLVM_ABI OptTable { ArrayRef<StringTable::Offset> PrefixesTable, ArrayRef<Info> OptionInfos, bool IgnoreCase = false, ArrayRef<SubCommand> SubCommands = {}, - ArrayRef<unsigned> SubCommandIDsTable = {}); + ArrayRef<unsigned> SubCommandIDsTable = {}, + ArrayRef<ValuesCodeEntry> ValuesCodeTable = {}); /// Build (or rebuild) the PrefixChars member. void buildPrefixChars(); @@ -276,27 +296,27 @@ class LLVM_ABI OptTable { } /// Get the help text to use to describe this option. - const char *getOptionHelpText(OptSpecifier id) const { + StringRef getOptionHelpText(OptSpecifier id) const { return getOptionHelpText(id, Visibility(0)); } // Get the help text to use to describe this option. // If it has visibility specific help text and that visibility is in the // visibility mask, use that text instead of the generic text. - const char *getOptionHelpText(OptSpecifier id, - Visibility VisibilityMask) const { - auto Info = getInfo(id); - for (auto [Visibilities, Text] : Info.HelpTextsForVariants) + StringRef getOptionHelpText(OptSpecifier id, + Visibility VisibilityMask) const { + const Info &I = getInfo(id); + for (auto [Visibilities, TextOffset] : I.HelpTextsForVariants) for (auto Visibility : Visibilities) if (VisibilityMask & Visibility) - return Text; - return Info.HelpText; + return (*StrTable)[TextOffset]; + return (*StrTable)[I.HelpTextOffset]; } /// Get the meta-variable name to use when describing /// this options values in the help text. - const char *getOptionMetaVar(OptSpecifier id) const { - return getInfo(id).MetaVar; + StringRef getOptionMetaVar(OptSpecifier id) const { + return (*StrTable)[getInfo(id).MetaVarOffset]; } /// Specify the environment variable where initial options should be read. @@ -481,7 +501,8 @@ class GenericOptTable : public OptTable { ArrayRef<StringTable::Offset> PrefixesTable, ArrayRef<Info> OptionInfos, bool IgnoreCase = false, ArrayRef<SubCommand> SubCommands = {}, - ArrayRef<unsigned> SubCommandIDsTable = {}); + ArrayRef<unsigned> SubCommandIDsTable = {}, + ArrayRef<ValuesCodeEntry> ValuesCodeTable = {}); }; class PrecomputedOptTable : public OptTable { @@ -492,9 +513,10 @@ class PrecomputedOptTable : public OptTable { ArrayRef<StringTable::Offset> PrefixesUnionOffsets, bool IgnoreCase = false, ArrayRef<SubCommand> SubCommands = {}, - ArrayRef<unsigned> SubCommandIDsTable = {}) + ArrayRef<unsigned> SubCommandIDsTable = {}, + ArrayRef<ValuesCodeEntry> ValuesCodeTable = {}) : OptTable(StrTable, PrefixesTable, OptionInfos, IgnoreCase, SubCommands, - SubCommandIDsTable) { + SubCommandIDsTable, ValuesCodeTable) { for (auto PrefixOffset : PrefixesUnionOffsets) PrefixesUnion.push_back(StrTable[PrefixOffset]); buildPrefixChars(); diff --git a/llvm/include/llvm/Option/Option.h b/llvm/include/llvm/Option/Option.h index eac964159dc42..53c13239e0943 100644 --- a/llvm/include/llvm/Option/Option.h +++ b/llvm/include/llvm/Option/Option.h @@ -117,14 +117,17 @@ class Option { return Owner->getOption(Info->AliasID); } - /// Get the alias arguments as a \0 separated list. - /// E.g. ["foo", "bar"] would be returned as "foo\0bar\0". + /// Get the alias arguments as a \0 separated list terminated by an empty + /// string. E.g. ["foo", "bar"] would be returned as "foo\0bar\0". const char *getAliasArgs() const { assert(Info && "Must have a valid info!"); - assert((!Info->AliasArgs || Info->AliasArgs[0] != 0) && - "AliasArgs should be either 0 or non-empty."); + assert(Owner && "Must have a valid owner!"); + return Owner->getStrTable().getCString(Info->AliasArgsOffset); + } - return Info->AliasArgs; + bool hasAliasArgs() const { + assert(Info && "Must have a valid info!"); + return Info->AliasArgsOffset.value() != 0; } /// Get the default prefix for this option. @@ -144,13 +147,15 @@ class Option { /// Get the help text for this option. StringRef getHelpText() const { assert(Info && "Must have a valid info!"); - return Info->HelpText; + assert(Owner && "Must have a valid owner!"); + return Owner->getStrTable()[Info->HelpTextOffset]; } /// Get the meta-variable list for this option. StringRef getMetaVar() const { assert(Info && "Must have a valid info!"); - return Info->MetaVar; + assert(Owner && "Must have a valid owner!"); + return Owner->getStrTable()[Info->MetaVarOffset]; } unsigned getNumArgs() const { return Info->Param; } diff --git a/llvm/lib/Option/OptTable.cpp b/llvm/lib/Option/OptTable.cpp index 8444675b847e6..ff1d27d297c4b 100644 --- a/llvm/lib/Option/OptTable.cpp +++ b/llvm/lib/Option/OptTable.cpp @@ -80,10 +80,12 @@ OptTable::OptTable(const StringTable &StrTable, ArrayRef<StringTable::Offset> PrefixesTable, ArrayRef<Info> OptionInfos, bool IgnoreCase, ArrayRef<SubCommand> SubCommands, - ArrayRef<unsigned> SubCommandIDsTable) + ArrayRef<unsigned> SubCommandIDsTable, + ArrayRef<ValuesCodeEntry> ValuesCodeTable) : StrTable(&StrTable), PrefixesTable(PrefixesTable), OptionInfos(OptionInfos), IgnoreCase(IgnoreCase), - SubCommands(SubCommands), SubCommandIDsTable(SubCommandIDsTable) { + SubCommands(SubCommands), SubCommandIDsTable(SubCommandIDsTable), + ValuesCodeTable(ValuesCodeTable) { // Explicitly zero initialize the error to work around a bug in array // value-initialization on MinGW with gcc 4.3.5. @@ -193,11 +195,12 @@ OptTable::suggestValueCompletions(StringRef Option, StringRef Arg) const { // Search all options and return possible values. for (size_t I = FirstSearchableIndex, E = OptionInfos.size(); I < E; I++) { const Info &In = OptionInfos[I]; - if (!In.Values || !optionMatches(*StrTable, PrefixesTable, In, Option)) + StringRef Values = getOptionValues(In); + if (Values.empty() || !optionMatches(*StrTable, PrefixesTable, In, Option)) continue; SmallVector<StringRef, 8> Candidates; - StringRef(In.Values).split(Candidates, ",", -1, false); + Values.split(Candidates, ",", -1, false); std::vector<std::string> Result; for (StringRef Val : Candidates) @@ -214,19 +217,20 @@ OptTable::findByPrefix(StringRef Cur, Visibility VisibilityMask, std::vector<std::string> Ret; for (size_t I = FirstSearchableIndex, E = OptionInfos.size(); I < E; I++) { const Info &In = OptionInfos[I]; - if (In.hasNoPrefix() || (!In.HelpText && !In.GroupID)) + if (In.hasNoPrefix() || (!In.HelpTextOffset.value() && !In.GroupID)) continue; if (!(In.Visibility & VisibilityMask)) continue; if (In.Flags & DisableFlags) continue; + StringRef HelpText = (*StrTable)[In.HelpTextOffset]; + StringRef Name = In.getName(*StrTable, PrefixesTable); for (auto PrefixOffset : In.getPrefixOffsets(PrefixesTable)) { StringRef Prefix = (*StrTable)[PrefixOffset]; std::string S = (Twine(Prefix) + Name + "\t").str(); - if (In.HelpText) - S += In.HelpText; + S += HelpText; if (StringRef(S).starts_with(Cur) && S != std::string(Cur) + "\t") Ret.push_back(S); } @@ -616,12 +620,12 @@ static std::string getOptionHelpName(const OptTable &Opts, OptSpecifier Id) { llvm_unreachable("Invalid option with help text."); case Option::MultiArgClass: - if (const char *MetaVarName = Opts.getOptionMetaVar(Id)) { + if (StringRef MetaVarName = Opts.getOptionMetaVar(Id); + !MetaVarName.empty()) { // For MultiArgs, metavar is full list of all argument names. Name += ' '; Name += MetaVarName; - } - else { + } else { // For MultiArgs<N>, if metavar not supplied, print <value> N times. for (unsigned i=0, e=O.getNumArgs(); i< e; ++i) { Name += " <value>"; @@ -641,7 +645,7 @@ static std::string getOptionHelpName(const OptTable &Opts, OptSpecifier Id) { [[fallthrough]]; case Option::JoinedClass: case Option::CommaJoinedClass: case Option::JoinedAndSeparateClass: - if (const char *MetaVarName = Opts.getOptionMetaVar(Id)) + if (StringRef MetaVarName = Opts.getOptionMetaVar(Id); !MetaVarName.empty()) Name += MetaVarName; else Name += "<value>"; @@ -695,7 +699,7 @@ static void PrintHelpOptionList(raw_ostream &OS, StringRef Title, } } -static const char *getOptionHelpGroup(const OptTable &Opts, OptSpecifier Id) { +static StringRef getOptionHelpGroup(const OptTable &Opts, OptSpecifier Id) { unsigned GroupID = Opts.getOptionGroupID(Id); // If not in a group, return the default help group. @@ -706,7 +710,7 @@ static const char *getOptionHelpGroup(const OptTable &Opts, OptSpecifier Id) { // name. // // FIXME: Split out option groups. - if (const char *GroupHelp = Opts.getOptionHelpText(GroupID)) + if (StringRef GroupHelp = Opts.getOptionHelpText(GroupID); !GroupHelp.empty()) return GroupHelp; // Otherwise keep looking. @@ -814,17 +818,17 @@ void OptTable::internalPrintHelp( // If an alias doesn't have a help text, show a help text for the aliased // option instead. - const char *HelpText = getOptionHelpText(Id, VisibilityMask); - if (!HelpText && ShowAllAliases) { + StringRef HelpText = getOptionHelpText(Id, VisibilityMask); + if (HelpText.empty() && ShowAllAliases) { const Option Alias = getOption(Id).getAlias(); if (Alias.isValid()) HelpText = getOptionHelpText(Alias.getID(), VisibilityMask); } - if (HelpText && (strlen(HelpText) != 0)) { - const char *HelpGroup = getOptionHelpGroup(*this, Id); + if (!HelpText.empty()) { + StringRef HelpGroup = getOptionHelpGroup(*this, Id); const std::string &OptName = getOptionHelpName(*this, Id); - GroupedOptionHelp[HelpGroup].push_back({OptName, HelpText}); + GroupedOptionHelp[std::string(HelpGroup)].push_back({OptName, HelpText}); } } @@ -841,9 +845,10 @@ GenericOptTable::GenericOptTable(const StringTable &StrTable, ArrayRef<StringTable::Offset> PrefixesTable, ArrayRef<Info> OptionInfos, bool IgnoreCase, ArrayRef<SubCommand> SubCommands, - ArrayRef<unsigned> SubCommandIDsTable) + ArrayRef<unsigned> SubCommandIDsTable, + ArrayRef<ValuesCodeEntry> ValuesCodeTable) : OptTable(StrTable, PrefixesTable, OptionInfos, IgnoreCase, SubCommands, - SubCommandIDsTable) { + SubCommandIDsTable, ValuesCodeTable) { std::set<StringRef> TmpPrefixesUnion; for (auto const &Info : OptionInfos.drop_front(FirstSearchableIndex)) diff --git a/llvm/lib/Option/Option.cpp b/llvm/lib/Option/Option.cpp index 838dd344b18a9..306c96c9d564a 100644 --- a/llvm/lib/Option/Option.cpp +++ b/llvm/lib/Option/Option.cpp @@ -29,7 +29,7 @@ Option::Option(const OptTable::Info *Info, const OptTable *Owner) assert((!Info || !getAlias().isValid() || !getAlias().getAlias().isValid()) && "Multi-level aliases are not supported."); - if (Info && getAliasArgs()) { + if (Info && hasAliasArgs()) { assert(getAlias().isValid() && "Only alias options can have alias args."); assert(getKind() == FlagClass && "Only Flag aliases can have alias args."); assert(getAlias().getKind() != FlagClass && @@ -281,15 +281,9 @@ std::unique_ptr<Arg> Option::accept(const ArgList &Args, StringRef CurArg, } // FlagClass aliases can have AliasArgs<>; add those to the unaliased arg. - if (const char *Val = getAliasArgs()) { - while (*Val != '\0') { - UnaliasedA->getValues().push_back(Val); - - // Move past the '\0' to the next argument. - Val += strlen(Val) + 1; - } - } - if (UnaliasedOption.getKind() == JoinedClass && !getAliasArgs()) + for (const char *Val = getAliasArgs(); *Val; Val += strlen(Val) + 1) + UnaliasedA->getValues().push_back(Val); + if (UnaliasedOption.getKind() == JoinedClass && !hasAliasArgs()) // A Flag alias for a Joined option must provide an argument. UnaliasedA->getValues().push_back(""); return UnaliasedA; diff --git a/llvm/unittests/Option/OptionParsingTest.cpp b/llvm/unittests/Option/OptionParsingTest.cpp index 3da015e343eb9..51030ee7018b9 100644 --- a/llvm/unittests/Option/OptionParsingTest.cpp +++ b/llvm/unittests/Option/OptionParsingTest.cpp @@ -229,6 +229,17 @@ TYPED_TEST(OptTableTest, AliasArgs) { EXPECT_EQ("bar", AL.getAllArgValues(OPT_B)[1]); } +TYPED_TEST(OptTableTest, AliasArgsMultiple) { + TypeParam T; + unsigned MAI, MAC; + + const char *MyArgs[] = {"-Jmulti"}; + InputArgList AL = T.ParseArgs(MyArgs, MAI, MAC); + EXPECT_TRUE(AL.hasArg(OPT_D)); + EXPECT_EQ((std::vector<std::string>{"foo", "bar"}), + AL.getAllArgValues(OPT_D)); +} + TYPED_TEST(OptTableTest, IgnoreCase) { TypeParam T(true); unsigned MAI, MAC; diff --git a/llvm/unittests/Option/Opts.td b/llvm/unittests/Option/Opts.td index 5be67c9decdbc..2c6c552232ebe 100644 --- a/llvm/unittests/Option/Opts.td +++ b/llvm/unittests/Option/Opts.td @@ -26,6 +26,7 @@ def I : Flag<["-"], "I">, Alias<H>, Group<my_group>; def J : Flag<["-"], "J">, Alias<B>, AliasArgs<["foo"]>; def Joo : Flag<["-"], "Joo">, Alias<B>, AliasArgs<["bar"]>; +def Jmulti : Flag<["-"], "Jmulti">, Alias<D>, AliasArgs<["foo", "bar"]>; def K : Flag<["-"], "K">, Alias<B>; diff --git a/llvm/utils/TableGen/OptionParserEmitter.cpp b/llvm/utils/TableGen/OptionParserEmitter.cpp index 829c202b495e4..98741d3c0d402 100644 --- a/llvm/utils/TableGen/OptionParserEmitter.cpp +++ b/llvm/utils/TableGen/OptionParserEmitter.cpp @@ -31,12 +31,17 @@ static std::string getOptionName(const Record &R) { return R.getValueAsString("EnumName").str(); } +// Long strings and strings that may contain "*/" need EmitComment=false. static raw_ostream &writeStrTableOffset(raw_ostream &OS, const StringToOffsetTable &Table, - llvm::StringRef Str) { - OS << Table.GetStringOffset(Str) << " /* "; - OS.write_escaped(Str); - OS << " */"; + llvm::StringRef Str, + bool EmitComment = true) { + OS << Table.GetStringOffset(Str); + if (EmitComment) { + OS << " /* "; + OS.write_escaped(Str); + OS << " */"; + } return OS; } @@ -47,6 +52,34 @@ static raw_ostream &writeCstring(raw_ostream &OS, llvm::StringRef Str) { return OS; } +// An unset field maps to the empty string, which the table keeps at offset 0. +static StringRef getOptionalString(const Record &R, StringRef Field) { + return R.getValueAsOptionalString(Field).value_or(""); +} + +static void addOptionalString(StringToOffsetTable &Table, const Record &R, + StringRef Field) { + Table.GetOrAddStringOffset(getOptionalString(R, Field)); +} + +static void writeOptionalStrOffset(raw_ostream &OS, + const StringToOffsetTable &Table, + const Record &R, StringRef Field) { + writeStrTableOffset(OS, Table, getOptionalString(R, Field), + /*EmitComment=*/false); +} + +// The alias arguments as a \0 separated list, e.g. ["foo", "bar"] becomes +// "foo\0bar\0". The string table appends the terminating empty string. +static std::string getAliasArgsBlob(const Record &R) { + std::string Blob; + for (StringRef AliasArg : R.getValueAsListOfStrings("AliasArgs")) { + Blob += AliasArg; + Blob += '\0'; + } + return Blob; +} + static std::string getOptionPrefixedName(const Record &R) { std::vector<StringRef> Prefixes = R.getValueAsListOfStrings("Prefixes"); StringRef Name = R.getValueAsString("Name"); @@ -196,8 +229,9 @@ static MarshallingInfo createMarshallingInfo(const Record &R) { } static void emitHelpTextsForVariants( - raw_ostream &OS, std::vector<std::pair<std::vector<std::string>, StringRef>> - HelpTextsForVariants) { + raw_ostream &OS, const StringToOffsetTable &Table, + std::vector<std::pair<std::vector<std::string>, StringRef>> + HelpTextsForVariants) { // OptTable must be constexpr so it uses std::arrays with these capacities. const unsigned MaxVisibilityPerHelp = 2; const unsigned MaxVisibilityHelp = 1; @@ -216,7 +250,7 @@ static void emitHelpTextsForVariants( {std::vector<std::string>(MaxVisibilityPerHelp, "0"), ""}); OS << ", (std::array<std::pair<std::array<unsigned, " << MaxVisibilityPerHelp - << ">, const char*>, " << MaxVisibilityHelp << ">{{ "; + << ">, llvm::StringTable::Offset>, " << MaxVisibilityHelp << ">{{ "; auto VisibilityHelpEnd = HelpTextsForVariants.cend(); for (auto VisibilityHelp = HelpTextsForVariants.cbegin(); @@ -229,10 +263,7 @@ static void emitHelpTextsForVariants( OS << "{std::array<unsigned, " << MaxVisibilityPerHelp << ">{{" << llvm::interleaved(Visibilities) << "}}, "; - if (Help.size()) - writeCstring(OS, Help); - else - OS << "nullptr"; + writeStrTableOffset(OS, Table, Help, /*EmitComment=*/false); OS << "}"; if (std::next(VisibilityHelp) != VisibilityHelpEnd) @@ -307,10 +338,20 @@ static void emitOptionParser(const RecordKeeper &Records, raw_ostream &OS) { // We can add all the prefixes via the union. for (const auto &Prefix : PrefixesUnion) Table.GetOrAddStringOffset(Prefix); - for (const Record &R : llvm::make_pointee_range(Groups)) + for (const Record &R : llvm::make_pointee_range(Groups)) { Table.GetOrAddStringOffset(R.getValueAsString("Name")); - for (const Record &R : llvm::make_pointee_range(Opts)) + addOptionalString(Table, R, "HelpText"); + } + for (const Record &R : llvm::make_pointee_range(Opts)) { Table.GetOrAddStringOffset(getOptionPrefixedName(R)); + addOptionalString(Table, R, "HelpText"); + addOptionalString(Table, R, "MetaVarName"); + addOptionalString(Table, R, "Values"); + Table.GetOrAddStringOffset(getAliasArgsBlob(R)); + for (const Record *VisibilityHelp : + R.getValueAsListOfDefs("HelpTextsForVariants")) + Table.GetOrAddStringOffset(VisibilityHelp->getValueAsString("Text")); + } // Dump string table. OS << "/////////\n"; @@ -401,16 +442,28 @@ static void emitOptionParser(const RecordKeeper &Records, raw_ostream &OS) { OS << "/////////\n"; OS << "// ValuesCode\n\n"; OS << "#ifdef OPTTABLE_VALUES_CODE\n"; + std::vector<const Record *> ValuesCodeOpts; for (const Record &R : llvm::make_pointee_range(Opts)) { // The option values, if any; if (!isa<UnsetInit>(R.getValueInit("ValuesCode"))) { assert(isa<UnsetInit>(R.getValueInit("Values")) && "Cannot choose between Values and ValuesCode"); + ValuesCodeOpts.push_back(&R); OS << "#define VALUES_CODE " << getOptionName(R) << "_Values\n"; OS << R.getValueAsString("ValuesCode") << "\n"; OS << "#undef VALUES_CODE\n"; } } + // Option IDs use the OPT_ prefix; a table with a different prefix cannot use + // ValuesCode. + if (!ValuesCodeOpts.empty()) { + OS << "static constexpr llvm::opt::OptTable::ValuesCodeEntry " + "OptionValuesCodeTable[] = {\n"; + for (const Record *R : ValuesCodeOpts) + OS << " {OPT_" << getOptionName(*R) << ", " << getOptionName(*R) + << "_Values},\n"; + OS << "};\n"; + } OS << "#endif\n"; OS << "/////////\n"; @@ -441,25 +494,20 @@ static void emitOptionParser(const RecordKeeper &Records, raw_ostream &OS) { OS << "INVALID"; // The other option arguments (unused for groups). - OS << ", INVALID, nullptr, 0, 0, 0"; + OS << ", INVALID, 0, 0, 0, 0"; // The option help text. - if (!isa<UnsetInit>(R.getValueInit("HelpText"))) { - OS << ",\n"; - OS << " "; - writeCstring(OS, R.getValueAsString("HelpText")); - } else { - OS << ", nullptr"; - } + OS << ", "; + writeOptionalStrOffset(OS, Table, R, "HelpText"); // Not using Visibility specific text for group help. - emitHelpTextsForVariants(OS, {}); + emitHelpTextsForVariants(OS, Table, {}); // The option meta-variable name (unused). - OS << ", nullptr"; + OS << ", 0"; // The option Values (unused for groups). - OS << ", nullptr"; + OS << ", 0"; // The option SubCommandIDsOffset. OS << ", "; @@ -505,19 +553,8 @@ static void emitOptionParser(const RecordKeeper &Records, raw_ostream &OS) { OS << "INVALID"; // The option alias arguments (if any). - // Emitted as a \0 separated list in a string, e.g. ["foo", "bar"] - // would become "foo\0bar\0". Note that the compiler adds an implicit - // terminating \0 at the end. OS << ", "; - std::vector<StringRef> AliasArgs = R.getValueAsListOfStrings("AliasArgs"); - if (AliasArgs.size() == 0) { - OS << "nullptr"; - } else { - OS << "\""; - for (StringRef AliasArg : AliasArgs) - OS << AliasArg << "\\0"; - OS << "\""; - } + writeStrTableOffset(OS, Table, getAliasArgsBlob(R), /*EmitComment=*/false); // "Flags" for the option, such as HelpHidden and Render* OS << ", "; @@ -552,13 +589,8 @@ static void emitOptionParser(const RecordKeeper &Records, raw_ostream &OS) { OS << ", " << R.getValueAsInt("NumArgs"); // The option help text. - if (!isa<UnsetInit>(R.getValueInit("HelpText"))) { - OS << ",\n"; - OS << " "; - writeCstring(OS, R.getValueAsString("HelpText")); - } else { - OS << ", nullptr"; - } + OS << ", "; + writeOptionalStrOffset(OS, Table, R, "HelpText"); std::vector<std::pair<std::vector<std::string>, StringRef>> HelpTextsForVariants; @@ -574,23 +606,15 @@ static void emitOptionParser(const RecordKeeper &Records, raw_ostream &OS) { HelpTextsForVariants.emplace_back( VisibilityNames, VisibilityHelp->getValueAsString("Text")); } - emitHelpTextsForVariants(OS, std::move(HelpTextsForVariants)); + emitHelpTextsForVariants(OS, Table, std::move(HelpTextsForVariants)); // The option meta-variable name. OS << ", "; - if (!isa<UnsetInit>(R.getValueInit("MetaVarName"))) - writeCstring(OS, R.getValueAsString("MetaVarName")); - else - OS << "nullptr"; + writeOptionalStrOffset(OS, Table, R, "MetaVarName"); // The option Values. Used for shell autocompletion. OS << ", "; - if (!isa<UnsetInit>(R.getValueInit("Values"))) - writeCstring(OS, R.getValueAsString("Values")); - else if (!isa<UnsetInit>(R.getValueInit("ValuesCode"))) - OS << getOptionName(R) << "_Values"; - else - OS << "nullptr"; + writeOptionalStrOffset(OS, Table, R, "Values"); // The option SubCommandIDsOffset. OS << ", "; >From ee2fafd2242498ae51b4acee632b8c22e0a49974 Mon Sep 17 00:00:00 2001 From: Fangrui Song <[email protected]> Date: Wed, 26 Aug 2026 00:31:21 -0700 Subject: [PATCH 2/3] Add ExplicitlyEmptyOffset to ensure clang --autocomplete=- output is byte-identical --- llvm/include/llvm/Option/OptTable.h | 16 +++-- llvm/lib/Option/OptTable.cpp | 5 +- llvm/unittests/Option/OptionParsingTest.cpp | 20 +++++- llvm/unittests/Option/Opts.td | 5 ++ llvm/utils/TableGen/OptionParserEmitter.cpp | 67 ++++++++++++--------- 5 files changed, 76 insertions(+), 37 deletions(-) diff --git a/llvm/include/llvm/Option/OptTable.h b/llvm/include/llvm/Option/OptTable.h index 58be2bda0d30a..1ed901cad5599 100644 --- a/llvm/include/llvm/Option/OptTable.h +++ b/llvm/include/llvm/Option/OptTable.h @@ -215,6 +215,15 @@ class LLVM_ABI OptTable { return OptionInfos[id - 1]; } + StringTable::Offset getHelpTextOffset(const Info &I, + Visibility VisibilityMask) const { + for (auto [Visibilities, TextOffset] : I.HelpTextsForVariants) + for (auto Visibility : Visibilities) + if (VisibilityMask & Visibility) + return TextOffset; + return I.HelpTextOffset; + } + StringRef getOptionValues(const Info &I) const { if (I.ValuesOffset.value()) return (*StrTable)[I.ValuesOffset]; @@ -305,12 +314,7 @@ class LLVM_ABI OptTable { // visibility mask, use that text instead of the generic text. StringRef getOptionHelpText(OptSpecifier id, Visibility VisibilityMask) const { - const Info &I = getInfo(id); - for (auto [Visibilities, TextOffset] : I.HelpTextsForVariants) - for (auto Visibility : Visibilities) - if (VisibilityMask & Visibility) - return (*StrTable)[TextOffset]; - return (*StrTable)[I.HelpTextOffset]; + return (*StrTable)[getHelpTextOffset(getInfo(id), VisibilityMask)]; } /// Get the meta-variable name to use when describing diff --git a/llvm/lib/Option/OptTable.cpp b/llvm/lib/Option/OptTable.cpp index ff1d27d297c4b..3f694bbb38690 100644 --- a/llvm/lib/Option/OptTable.cpp +++ b/llvm/lib/Option/OptTable.cpp @@ -819,7 +819,8 @@ void OptTable::internalPrintHelp( // If an alias doesn't have a help text, show a help text for the aliased // option instead. StringRef HelpText = getOptionHelpText(Id, VisibilityMask); - if (HelpText.empty() && ShowAllAliases) { + if (!getHelpTextOffset(CandidateInfo, VisibilityMask).value() && + ShowAllAliases) { const Option Alias = getOption(Id).getAlias(); if (Alias.isValid()) HelpText = getOptionHelpText(Alias.getID(), VisibilityMask); @@ -828,7 +829,7 @@ void OptTable::internalPrintHelp( if (!HelpText.empty()) { StringRef HelpGroup = getOptionHelpGroup(*this, Id); const std::string &OptName = getOptionHelpName(*this, Id); - GroupedOptionHelp[std::string(HelpGroup)].push_back({OptName, HelpText}); + GroupedOptionHelp[HelpGroup.str()].push_back({OptName, HelpText}); } } diff --git a/llvm/unittests/Option/OptionParsingTest.cpp b/llvm/unittests/Option/OptionParsingTest.cpp index 51030ee7018b9..04dec42e0279a 100644 --- a/llvm/unittests/Option/OptionParsingTest.cpp +++ b/llvm/unittests/Option/OptionParsingTest.cpp @@ -32,6 +32,10 @@ enum ID { #undef OPTION }; +#define OPTTABLE_VALUES_CODE +#include "Opts.inc" +#undef OPTTABLE_VALUES_CODE + #define OPTTABLE_PREFIXES_TABLE_CODE #include "Opts.inc" #undef OPTTABLE_PREFIXES_TABLE_CODE @@ -62,14 +66,16 @@ class TestOptTable : public GenericOptTable { public: TestOptTable(bool IgnoreCase = false) : GenericOptTable(OptionStrTable, OptionPrefixesTable, InfoTable, - IgnoreCase) {} + IgnoreCase, /*SubCommands=*/{}, + /*SubCommandIDsTable=*/{}, OptionValuesCodeTable) {} }; class TestPrecomputedOptTable : public PrecomputedOptTable { public: TestPrecomputedOptTable(bool IgnoreCase = false) : PrecomputedOptTable(OptionStrTable, OptionPrefixesTable, InfoTable, - OptionPrefixesUnion, IgnoreCase) {} + OptionPrefixesUnion, IgnoreCase, /*SubCommands=*/{}, + /*SubCommandIDsTable=*/{}, OptionValuesCodeTable) {} }; } @@ -240,6 +246,16 @@ TYPED_TEST(OptTableTest, AliasArgsMultiple) { AL.getAllArgValues(OPT_D)); } +TYPED_TEST(OptTableTest, SuggestValueCompletions) { + TypeParam T; + + EXPECT_EQ((std::vector<std::string>{"inline1", "inline2"}), + T.suggestValueCompletions("-values-inline=", "")); + // Values computed by ValuesCode live outside the string table. + EXPECT_EQ((std::vector<std::string>{"code1", "code2"}), + T.suggestValueCompletions("-values-from-code=", "")); +} + TYPED_TEST(OptTableTest, IgnoreCase) { TypeParam T(true); unsigned MAI, MAC; diff --git a/llvm/unittests/Option/Opts.td b/llvm/unittests/Option/Opts.td index 2c6c552232ebe..89c20359bd2a0 100644 --- a/llvm/unittests/Option/Opts.td +++ b/llvm/unittests/Option/Opts.td @@ -30,6 +30,11 @@ def Jmulti : Flag<["-"], "Jmulti">, Alias<D>, AliasArgs<["foo", "bar"]>; def K : Flag<["-"], "K">, Alias<B>; +def ValuesInline : Joined<["-"], "values-inline=">, Values<"inline1,inline2">; +def ValuesFromCode : Joined<["-"], "values-from-code=">, ValuesCode<[{ + static constexpr const char VALUES_CODE [] = "code1,code2"; +}]>; + def Slurp : Option<["-"], "slurp", KIND_REMAINING_ARGS>; def SlurpJoined : Option<["-"], "slurpjoined", KIND_REMAINING_ARGS_JOINED>; diff --git a/llvm/utils/TableGen/OptionParserEmitter.cpp b/llvm/utils/TableGen/OptionParserEmitter.cpp index 98741d3c0d402..685a15ee9cc3e 100644 --- a/llvm/utils/TableGen/OptionParserEmitter.cpp +++ b/llvm/utils/TableGen/OptionParserEmitter.cpp @@ -15,6 +15,7 @@ #include "llvm/Option/OptTable.h" #include "llvm/Support/InterleavedRange.h" #include "llvm/Support/raw_ostream.h" +#include "llvm/TableGen/Error.h" #include "llvm/TableGen/Record.h" #include "llvm/TableGen/StringToOffsetTable.h" #include "llvm/TableGen/TableGenBackend.h" @@ -31,12 +32,15 @@ static std::string getOptionName(const Record &R) { return R.getValueAsString("EnumName").str(); } -// Long strings and strings that may contain "*/" need EmitComment=false. +// Only pass EmitComment for short strings that cannot contain "*/". static raw_ostream &writeStrTableOffset(raw_ostream &OS, const StringToOffsetTable &Table, llvm::StringRef Str, - bool EmitComment = true) { - OS << Table.GetStringOffset(Str); + bool EmitComment = false) { + std::optional<unsigned> Offset = Table.GetStringOffset(Str); + if (!Offset) + PrintFatalError("string was not added to the option string table: " + Str); + OS << *Offset; if (EmitComment) { OS << " /* "; OS.write_escaped(Str); @@ -52,28 +56,35 @@ static raw_ostream &writeCstring(raw_ostream &OS, llvm::StringRef Str) { return OS; } -// An unset field maps to the empty string, which the table keeps at offset 0. -static StringRef getOptionalString(const Record &R, StringRef Field) { - return R.getValueAsOptionalString(Field).value_or(""); -} - static void addOptionalString(StringToOffsetTable &Table, const Record &R, StringRef Field) { - Table.GetOrAddStringOffset(getOptionalString(R, Field)); + if (std::optional<StringRef> S = R.getValueAsOptionalString(Field)) + Table.GetOrAddStringOffset(*S); } +// Offset zero is the empty string and stands for an unset field. A field that +// is set to the empty string, e.g. HelpText<"">, marks an option as +// deliberately undocumented and gets \p ExplicitlyEmptyOffset instead, so that +// the two stay distinguishable. static void writeOptionalStrOffset(raw_ostream &OS, const StringToOffsetTable &Table, - const Record &R, StringRef Field) { - writeStrTableOffset(OS, Table, getOptionalString(R, Field), - /*EmitComment=*/false); + const Record &R, StringRef Field, + unsigned ExplicitlyEmptyOffset) { + std::optional<StringRef> S = R.getValueAsOptionalString(Field); + if (!S) + OS << '0'; + else if (S->empty()) + OS << ExplicitlyEmptyOffset; + else + writeStrTableOffset(OS, Table, *S); } -// The alias arguments as a \0 separated list, e.g. ["foo", "bar"] becomes -// "foo\0bar\0". The string table appends the terminating empty string. +// The string table appends the empty string that terminates the list. static std::string getAliasArgsBlob(const Record &R) { std::string Blob; for (StringRef AliasArg : R.getValueAsListOfStrings("AliasArgs")) { + if (AliasArg.empty()) + PrintFatalError(R.getLoc(), "AliasArgs entries must not be empty"); Blob += AliasArg; Blob += '\0'; } @@ -240,11 +251,7 @@ static void emitHelpTextsForVariants( "Too many help text variants to store in " "OptTable::HelpTextsForVariants"); - // This function must initialise any unused elements of those arrays. - for (auto [Visibilities, _] : HelpTextsForVariants) - while (Visibilities.size() < MaxVisibilityPerHelp) - Visibilities.push_back("0"); - + // Unused visibility slots are left to aggregate value-initialization. while (HelpTextsForVariants.size() < MaxVisibilityHelp) HelpTextsForVariants.push_back( {std::vector<std::string>(MaxVisibilityPerHelp, "0"), ""}); @@ -263,7 +270,7 @@ static void emitHelpTextsForVariants( OS << "{std::array<unsigned, " << MaxVisibilityPerHelp << ">{{" << llvm::interleaved(Visibilities) << "}}, "; - writeStrTableOffset(OS, Table, Help, /*EmitComment=*/false); + writeStrTableOffset(OS, Table, Help); OS << "}"; if (std::next(VisibilityHelp) != VisibilityHelpEnd) @@ -335,6 +342,10 @@ static void emitOptionParser(const RecordKeeper &Records, raw_ostream &OS) { array_pod_sort(PrefixesUnion.begin(), PrefixesUnion.end()); llvm::StringToOffsetTable Table; + // An empty string that is not at offset zero, for fields that are explicitly + // set to it; see writeOptionalStrOffset. + const unsigned ExplicitlyEmptyOffset = + Table.GetOrAddStringOffset(StringRef("\0", 1)); // We can add all the prefixes via the union. for (const auto &Prefix : PrefixesUnion) Table.GetOrAddStringOffset(Prefix); @@ -478,7 +489,8 @@ static void emitOptionParser(const RecordKeeper &Records, raw_ostream &OS) { // The option string offset. OS << ", "; - writeStrTableOffset(OS, Table, R.getValueAsString("Name")); + writeStrTableOffset(OS, Table, R.getValueAsString("Name"), + /*EmitComment=*/true); // The option identifier name. OS << ", " << getOptionName(R); @@ -498,7 +510,7 @@ static void emitOptionParser(const RecordKeeper &Records, raw_ostream &OS) { // The option help text. OS << ", "; - writeOptionalStrOffset(OS, Table, R, "HelpText"); + writeOptionalStrOffset(OS, Table, R, "HelpText", ExplicitlyEmptyOffset); // Not using Visibility specific text for group help. emitHelpTextsForVariants(OS, Table, {}); @@ -525,7 +537,8 @@ static void emitOptionParser(const RecordKeeper &Records, raw_ostream &OS) { OS << Prefixes[PrefixKeyT(RPrefixes.begin(), RPrefixes.end())] << ", "; // The option prefixed name. - writeStrTableOffset(OS, Table, getOptionPrefixedName(R)); + writeStrTableOffset(OS, Table, getOptionPrefixedName(R), + /*EmitComment=*/true); // The option identifier name. OS << ", " << getOptionName(R); @@ -554,7 +567,7 @@ static void emitOptionParser(const RecordKeeper &Records, raw_ostream &OS) { // The option alias arguments (if any). OS << ", "; - writeStrTableOffset(OS, Table, getAliasArgsBlob(R), /*EmitComment=*/false); + writeStrTableOffset(OS, Table, getAliasArgsBlob(R)); // "Flags" for the option, such as HelpHidden and Render* OS << ", "; @@ -590,7 +603,7 @@ static void emitOptionParser(const RecordKeeper &Records, raw_ostream &OS) { // The option help text. OS << ", "; - writeOptionalStrOffset(OS, Table, R, "HelpText"); + writeOptionalStrOffset(OS, Table, R, "HelpText", ExplicitlyEmptyOffset); std::vector<std::pair<std::vector<std::string>, StringRef>> HelpTextsForVariants; @@ -610,11 +623,11 @@ static void emitOptionParser(const RecordKeeper &Records, raw_ostream &OS) { // The option meta-variable name. OS << ", "; - writeOptionalStrOffset(OS, Table, R, "MetaVarName"); + writeOptionalStrOffset(OS, Table, R, "MetaVarName", ExplicitlyEmptyOffset); // The option Values. Used for shell autocompletion. OS << ", "; - writeOptionalStrOffset(OS, Table, R, "Values"); + writeOptionalStrOffset(OS, Table, R, "Values", ExplicitlyEmptyOffset); // The option SubCommandIDsOffset. OS << ", "; >From 93ea6b55bb74022ca85e1d26635ae4420972a07d Mon Sep 17 00:00:00 2001 From: Fangrui Song <[email protected]> Date: Wed, 26 Aug 2026 00:55:04 -0700 Subject: [PATCH 3/3] . --- clang/lib/Options/DriverOptions.cpp | 2 +- llvm/include/llvm/Option/OptTable.h | 37 ++++++------ llvm/include/llvm/Option/Option.h | 5 +- llvm/lib/Option/OptTable.cpp | 37 ++++++------ llvm/lib/Option/Option.cpp | 1 + llvm/unittests/Option/OptionParsingTest.cpp | 4 +- llvm/utils/TableGen/OptionParserEmitter.cpp | 66 ++++++++++----------- 7 files changed, 77 insertions(+), 75 deletions(-) diff --git a/clang/lib/Options/DriverOptions.cpp b/clang/lib/Options/DriverOptions.cpp index 133fc0babae14..ac3c1e788c9c6 100644 --- a/clang/lib/Options/DriverOptions.cpp +++ b/clang/lib/Options/DriverOptions.cpp @@ -43,7 +43,7 @@ class DriverOptTable : public PrecomputedOptTable { : PrecomputedOptTable(OptionStrTable, OptionPrefixesTable, InfoTable, OptionPrefixesUnion, /*IgnoreCase=*/false, /*SubCommands=*/{}, /*SubCommandIDsTable=*/{}, - OptionValuesCodeTable) {} + getOptionValuesCode) {} }; } // anonymous namespace diff --git a/llvm/include/llvm/Option/OptTable.h b/llvm/include/llvm/Option/OptTable.h index 1ed901cad5599..13021884e9ee7 100644 --- a/llvm/include/llvm/Option/OptTable.h +++ b/llvm/include/llvm/Option/OptTable.h @@ -60,8 +60,9 @@ class LLVM_ABI OptTable { const char *Usage; }; - /// An option ID together with the values produced by TableGen `ValuesCode`. - using ValuesCodeEntry = std::pair<unsigned, const char *>; + /// Returns the values produced by TableGen `ValuesCode` for an option ID, or + /// the empty string. Generated code supplies this; see ValuesCodeFn. + using ValuesCodeFnTy = StringRef (*)(unsigned); /// Entry for a single option instance in the option data table. struct Info { @@ -71,7 +72,7 @@ class LLVM_ABI OptTable { // Help text for specific visibilities. A list of pairs, where each pair // is a list of visibilities and a specific help string for those // visibilities. If no help text is found in this list for the visibility of - // the program, HelpText is used instead. This cannot use std::vector + // the program, HelpTextOffset is used instead. This cannot use std::vector // because OptTable is used in constexpr contexts. Increase the array sizes // here if you need more entries and adjust the constants in // OptionParserEmitter::EmitHelpTextsForVariants. @@ -90,6 +91,8 @@ class LLVM_ABI OptTable { /// The alias arguments as a \0 separated list terminated by an empty /// string, e.g. "foo\0bar\0". StringTable::Offset AliasArgsOffset; + + bool hasAliasArgs() const { return AliasArgsOffset.value() != 0; } /// The possible values as a comma separated list; see ValuesCodeTable. StringTable::Offset ValuesOffset; // Offset into OptTable's SubCommandIDsTable. @@ -109,6 +112,10 @@ class LLVM_ABI OptTable { getNumPrefixes(PrefixesTable)); } + /// Whether the .td supplied a help text. An explicitly empty one is not the + /// same as none: it marks the option as deliberately undocumented. + bool hasHelpText() const { return HelpTextOffset.value() != 0; } + bool hasSubCommands() const { return SubCommandIDsOffset != 0; } unsigned getNumSubCommandIDs(ArrayRef<unsigned> SubCommandIDsTable) const { @@ -187,7 +194,7 @@ class LLVM_ABI OptTable { /// Values of options declared with TableGen `ValuesCode`: only the generated /// code knows them, so they cannot go in the string table. - ArrayRef<ValuesCodeEntry> ValuesCodeTable; + ValuesCodeFnTy ValuesCodeFn = nullptr; bool GroupedShortOptions = false; bool DashDashParsing = false; @@ -217,20 +224,16 @@ class LLVM_ABI OptTable { StringTable::Offset getHelpTextOffset(const Info &I, Visibility VisibilityMask) const { - for (auto [Visibilities, TextOffset] : I.HelpTextsForVariants) - for (auto Visibility : Visibilities) - if (VisibilityMask & Visibility) + for (const auto &[Visibilities, TextOffset] : I.HelpTextsForVariants) + for (auto Vis : Visibilities) + if (VisibilityMask & Vis) return TextOffset; return I.HelpTextOffset; } StringRef getOptionValues(const Info &I) const { - if (I.ValuesOffset.value()) - return (*StrTable)[I.ValuesOffset]; - for (const auto &[ID, Values] : ValuesCodeTable) - if (ID == I.ID) - return Values; - return StringRef(); + StringRef Values = (*StrTable)[I.ValuesOffset]; + return Values.empty() && ValuesCodeFn ? ValuesCodeFn(I.ID) : Values; } std::unique_ptr<Arg> parseOneArgGrouped(InputArgList &Args, @@ -244,7 +247,7 @@ class LLVM_ABI OptTable { ArrayRef<Info> OptionInfos, bool IgnoreCase = false, ArrayRef<SubCommand> SubCommands = {}, ArrayRef<unsigned> SubCommandIDsTable = {}, - ArrayRef<ValuesCodeEntry> ValuesCodeTable = {}); + ValuesCodeFnTy ValuesCodeFn = nullptr); /// Build (or rebuild) the PrefixChars member. void buildPrefixChars(); @@ -506,7 +509,7 @@ class GenericOptTable : public OptTable { ArrayRef<Info> OptionInfos, bool IgnoreCase = false, ArrayRef<SubCommand> SubCommands = {}, ArrayRef<unsigned> SubCommandIDsTable = {}, - ArrayRef<ValuesCodeEntry> ValuesCodeTable = {}); + ValuesCodeFnTy ValuesCodeFn = nullptr); }; class PrecomputedOptTable : public OptTable { @@ -518,9 +521,9 @@ class PrecomputedOptTable : public OptTable { bool IgnoreCase = false, ArrayRef<SubCommand> SubCommands = {}, ArrayRef<unsigned> SubCommandIDsTable = {}, - ArrayRef<ValuesCodeEntry> ValuesCodeTable = {}) + ValuesCodeFnTy ValuesCodeFn = nullptr) : OptTable(StrTable, PrefixesTable, OptionInfos, IgnoreCase, SubCommands, - SubCommandIDsTable, ValuesCodeTable) { + SubCommandIDsTable, ValuesCodeFn) { for (auto PrefixOffset : PrefixesUnionOffsets) PrefixesUnion.push_back(StrTable[PrefixOffset]); buildPrefixChars(); diff --git a/llvm/include/llvm/Option/Option.h b/llvm/include/llvm/Option/Option.h index 53c13239e0943..2cb0a7b32acce 100644 --- a/llvm/include/llvm/Option/Option.h +++ b/llvm/include/llvm/Option/Option.h @@ -117,8 +117,7 @@ class Option { return Owner->getOption(Info->AliasID); } - /// Get the alias arguments as a \0 separated list terminated by an empty - /// string. E.g. ["foo", "bar"] would be returned as "foo\0bar\0". + /// Get the alias arguments; see Info::AliasArgsOffset for the encoding. const char *getAliasArgs() const { assert(Info && "Must have a valid info!"); assert(Owner && "Must have a valid owner!"); @@ -127,7 +126,7 @@ class Option { bool hasAliasArgs() const { assert(Info && "Must have a valid info!"); - return Info->AliasArgsOffset.value() != 0; + return Info->hasAliasArgs(); } /// Get the default prefix for this option. diff --git a/llvm/lib/Option/OptTable.cpp b/llvm/lib/Option/OptTable.cpp index 3f694bbb38690..fabf0131ff4a4 100644 --- a/llvm/lib/Option/OptTable.cpp +++ b/llvm/lib/Option/OptTable.cpp @@ -21,7 +21,6 @@ #include <algorithm> #include <cassert> #include <cctype> -#include <cstring> #include <map> #include <set> #include <string> @@ -81,11 +80,11 @@ OptTable::OptTable(const StringTable &StrTable, ArrayRef<Info> OptionInfos, bool IgnoreCase, ArrayRef<SubCommand> SubCommands, ArrayRef<unsigned> SubCommandIDsTable, - ArrayRef<ValuesCodeEntry> ValuesCodeTable) + ValuesCodeFnTy ValuesCodeFn) : StrTable(&StrTable), PrefixesTable(PrefixesTable), OptionInfos(OptionInfos), IgnoreCase(IgnoreCase), SubCommands(SubCommands), SubCommandIDsTable(SubCommandIDsTable), - ValuesCodeTable(ValuesCodeTable) { + ValuesCodeFn(ValuesCodeFn) { // Explicitly zero initialize the error to work around a bug in array // value-initialization on MinGW with gcc 4.3.5. @@ -195,12 +194,12 @@ OptTable::suggestValueCompletions(StringRef Option, StringRef Arg) const { // Search all options and return possible values. for (size_t I = FirstSearchableIndex, E = OptionInfos.size(); I < E; I++) { const Info &In = OptionInfos[I]; - StringRef Values = getOptionValues(In); - if (Values.empty() || !optionMatches(*StrTable, PrefixesTable, In, Option)) + if (!In.ValuesOffset.value() || + !optionMatches(*StrTable, PrefixesTable, In, Option)) continue; SmallVector<StringRef, 8> Candidates; - Values.split(Candidates, ",", -1, false); + getOptionValues(In).split(Candidates, ",", -1, false); std::vector<std::string> Result; for (StringRef Val : Candidates) @@ -217,7 +216,7 @@ OptTable::findByPrefix(StringRef Cur, Visibility VisibilityMask, std::vector<std::string> Ret; for (size_t I = FirstSearchableIndex, E = OptionInfos.size(); I < E; I++) { const Info &In = OptionInfos[I]; - if (In.hasNoPrefix() || (!In.HelpTextOffset.value() && !In.GroupID)) + if (In.hasNoPrefix() || (!In.hasHelpText() && !In.GroupID)) continue; if (!(In.Visibility & VisibilityMask)) continue; @@ -620,8 +619,7 @@ static std::string getOptionHelpName(const OptTable &Opts, OptSpecifier Id) { llvm_unreachable("Invalid option with help text."); case Option::MultiArgClass: - if (StringRef MetaVarName = Opts.getOptionMetaVar(Id); - !MetaVarName.empty()) { + if (StringRef MetaVarName = O.getMetaVar(); !MetaVarName.empty()) { // For MultiArgs, metavar is full list of all argument names. Name += ' '; Name += MetaVarName; @@ -645,7 +643,7 @@ static std::string getOptionHelpName(const OptTable &Opts, OptSpecifier Id) { [[fallthrough]]; case Option::JoinedClass: case Option::CommaJoinedClass: case Option::JoinedAndSeparateClass: - if (StringRef MetaVarName = Opts.getOptionMetaVar(Id); !MetaVarName.empty()) + if (StringRef MetaVarName = O.getMetaVar(); !MetaVarName.empty()) Name += MetaVarName; else Name += "<value>"; @@ -755,7 +753,7 @@ void OptTable::internalPrintHelp( // Render help text into a map of group-name to a list of (option, help) // pairs. - std::map<std::string, std::vector<OptionInfo>> GroupedOptionHelp; + std::map<StringRef, std::vector<OptionInfo>> GroupedOptionHelp; auto ActiveSubCommand = llvm::find_if( SubCommands, [&](const auto &C) { return SubCommand == C.Name; }); @@ -818,18 +816,19 @@ void OptTable::internalPrintHelp( // If an alias doesn't have a help text, show a help text for the aliased // option instead. - StringRef HelpText = getOptionHelpText(Id, VisibilityMask); - if (!getHelpTextOffset(CandidateInfo, VisibilityMask).value() && - ShowAllAliases) { + StringTable::Offset HelpTextOffset = + getHelpTextOffset(CandidateInfo, VisibilityMask); + if (!HelpTextOffset.value() && ShowAllAliases) { const Option Alias = getOption(Id).getAlias(); if (Alias.isValid()) - HelpText = getOptionHelpText(Alias.getID(), VisibilityMask); + HelpTextOffset = + getHelpTextOffset(getInfo(Alias.getID()), VisibilityMask); } - if (!HelpText.empty()) { + if (StringRef HelpText = (*StrTable)[HelpTextOffset]; !HelpText.empty()) { StringRef HelpGroup = getOptionHelpGroup(*this, Id); const std::string &OptName = getOptionHelpName(*this, Id); - GroupedOptionHelp[HelpGroup.str()].push_back({OptName, HelpText}); + GroupedOptionHelp[HelpGroup].push_back({OptName, HelpText}); } } @@ -847,9 +846,9 @@ GenericOptTable::GenericOptTable(const StringTable &StrTable, ArrayRef<Info> OptionInfos, bool IgnoreCase, ArrayRef<SubCommand> SubCommands, ArrayRef<unsigned> SubCommandIDsTable, - ArrayRef<ValuesCodeEntry> ValuesCodeTable) + ValuesCodeFnTy ValuesCodeFn) : OptTable(StrTable, PrefixesTable, OptionInfos, IgnoreCase, SubCommands, - SubCommandIDsTable, ValuesCodeTable) { + SubCommandIDsTable, ValuesCodeFn) { std::set<StringRef> TmpPrefixesUnion; for (auto const &Info : OptionInfos.drop_front(FirstSearchableIndex)) diff --git a/llvm/lib/Option/Option.cpp b/llvm/lib/Option/Option.cpp index 306c96c9d564a..8e3638485b667 100644 --- a/llvm/lib/Option/Option.cpp +++ b/llvm/lib/Option/Option.cpp @@ -18,6 +18,7 @@ #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/raw_ostream.h" #include <cassert> +#include <cstring> using namespace llvm; using namespace llvm::opt; diff --git a/llvm/unittests/Option/OptionParsingTest.cpp b/llvm/unittests/Option/OptionParsingTest.cpp index 04dec42e0279a..baac6623948af 100644 --- a/llvm/unittests/Option/OptionParsingTest.cpp +++ b/llvm/unittests/Option/OptionParsingTest.cpp @@ -67,7 +67,7 @@ class TestOptTable : public GenericOptTable { TestOptTable(bool IgnoreCase = false) : GenericOptTable(OptionStrTable, OptionPrefixesTable, InfoTable, IgnoreCase, /*SubCommands=*/{}, - /*SubCommandIDsTable=*/{}, OptionValuesCodeTable) {} + /*SubCommandIDsTable=*/{}, getOptionValuesCode) {} }; class TestPrecomputedOptTable : public PrecomputedOptTable { @@ -75,7 +75,7 @@ class TestPrecomputedOptTable : public PrecomputedOptTable { TestPrecomputedOptTable(bool IgnoreCase = false) : PrecomputedOptTable(OptionStrTable, OptionPrefixesTable, InfoTable, OptionPrefixesUnion, IgnoreCase, /*SubCommands=*/{}, - /*SubCommandIDsTable=*/{}, OptionValuesCodeTable) {} + /*SubCommandIDsTable=*/{}, getOptionValuesCode) {} }; } diff --git a/llvm/utils/TableGen/OptionParserEmitter.cpp b/llvm/utils/TableGen/OptionParserEmitter.cpp index 685a15ee9cc3e..b38dd8815eb9b 100644 --- a/llvm/utils/TableGen/OptionParserEmitter.cpp +++ b/llvm/utils/TableGen/OptionParserEmitter.cpp @@ -33,10 +33,9 @@ static std::string getOptionName(const Record &R) { } // Only pass EmitComment for short strings that cannot contain "*/". -static raw_ostream &writeStrTableOffset(raw_ostream &OS, - const StringToOffsetTable &Table, - llvm::StringRef Str, - bool EmitComment = false) { +static void writeStrTableOffset(raw_ostream &OS, + const StringToOffsetTable &Table, + llvm::StringRef Str, bool EmitComment = false) { std::optional<unsigned> Offset = Table.GetStringOffset(Str); if (!Offset) PrintFatalError("string was not added to the option string table: " + Str); @@ -46,7 +45,6 @@ static raw_ostream &writeStrTableOffset(raw_ostream &OS, OS.write_escaped(Str); OS << " */"; } - return OS; } static raw_ostream &writeCstring(raw_ostream &OS, llvm::StringRef Str) { @@ -62,15 +60,14 @@ static void addOptionalString(StringToOffsetTable &Table, const Record &R, Table.GetOrAddStringOffset(*S); } -// Offset zero is the empty string and stands for an unset field. A field that -// is set to the empty string, e.g. HelpText<"">, marks an option as -// deliberately undocumented and gets \p ExplicitlyEmptyOffset instead, so that -// the two stay distinguishable. -static void writeOptionalStrOffset(raw_ostream &OS, - const StringToOffsetTable &Table, - const Record &R, StringRef Field, - unsigned ExplicitlyEmptyOffset) { - std::optional<StringRef> S = R.getValueAsOptionalString(Field); +// Offset zero is the empty string and stands for an unset HelpText. A +// HelpText<""> marks an option as deliberately undocumented and gets \p +// ExplicitlyEmptyOffset instead, so that the two stay distinguishable. +static void writeHelpTextOffset(raw_ostream &OS, + const StringToOffsetTable &Table, + const Record &R, + unsigned ExplicitlyEmptyOffset) { + std::optional<StringRef> S = R.getValueAsOptionalString("HelpText"); if (!S) OS << '0'; else if (S->empty()) @@ -253,8 +250,7 @@ static void emitHelpTextsForVariants( // Unused visibility slots are left to aggregate value-initialization. while (HelpTextsForVariants.size() < MaxVisibilityHelp) - HelpTextsForVariants.push_back( - {std::vector<std::string>(MaxVisibilityPerHelp, "0"), ""}); + HelpTextsForVariants.push_back({}); OS << ", (std::array<std::pair<std::array<unsigned, " << MaxVisibilityPerHelp << ">, llvm::StringTable::Offset>, " << MaxVisibilityHelp << ">{{ "; @@ -457,24 +453,22 @@ static void emitOptionParser(const RecordKeeper &Records, raw_ostream &OS) { for (const Record &R : llvm::make_pointee_range(Opts)) { // The option values, if any; if (!isa<UnsetInit>(R.getValueInit("ValuesCode"))) { - assert(isa<UnsetInit>(R.getValueInit("Values")) && - "Cannot choose between Values and ValuesCode"); + if (!isa<UnsetInit>(R.getValueInit("Values"))) + PrintFatalError(R.getLoc(), "cannot set both Values and ValuesCode"); ValuesCodeOpts.push_back(&R); OS << "#define VALUES_CODE " << getOptionName(R) << "_Values\n"; OS << R.getValueAsString("ValuesCode") << "\n"; OS << "#undef VALUES_CODE\n"; } } - // Option IDs use the OPT_ prefix; a table with a different prefix cannot use - // ValuesCode. - if (!ValuesCodeOpts.empty()) { - OS << "static constexpr llvm::opt::OptTable::ValuesCodeEntry " - "OptionValuesCodeTable[] = {\n"; - for (const Record *R : ValuesCodeOpts) - OS << " {OPT_" << getOptionName(*R) << ", " << getOptionName(*R) - << "_Values},\n"; - OS << "};\n"; - } + // A function keeps these strings out of a relocated table. Option IDs use the + // OPT_ prefix; a table with a different prefix cannot use ValuesCode. + OS << "static llvm::StringRef getOptionValuesCode(unsigned ID) {\n"; + OS << " switch (ID) {\n"; + for (const Record *R : ValuesCodeOpts) + OS << " case OPT_" << getOptionName(*R) << ": return " << getOptionName(*R) + << "_Values;\n"; + OS << " }\n return {};\n}\n"; OS << "#endif\n"; OS << "/////////\n"; @@ -510,7 +504,7 @@ static void emitOptionParser(const RecordKeeper &Records, raw_ostream &OS) { // The option help text. OS << ", "; - writeOptionalStrOffset(OS, Table, R, "HelpText", ExplicitlyEmptyOffset); + writeHelpTextOffset(OS, Table, R, ExplicitlyEmptyOffset); // Not using Visibility specific text for group help. emitHelpTextsForVariants(OS, Table, {}); @@ -603,7 +597,7 @@ static void emitOptionParser(const RecordKeeper &Records, raw_ostream &OS) { // The option help text. OS << ", "; - writeOptionalStrOffset(OS, Table, R, "HelpText", ExplicitlyEmptyOffset); + writeHelpTextOffset(OS, Table, R, ExplicitlyEmptyOffset); std::vector<std::pair<std::vector<std::string>, StringRef>> HelpTextsForVariants; @@ -623,11 +617,17 @@ static void emitOptionParser(const RecordKeeper &Records, raw_ostream &OS) { // The option meta-variable name. OS << ", "; - writeOptionalStrOffset(OS, Table, R, "MetaVarName", ExplicitlyEmptyOffset); + writeStrTableOffset(OS, Table, + R.getValueAsOptionalString("MetaVarName").value_or("")); - // The option Values. Used for shell autocompletion. + // The option Values. Used for shell autocompletion. ValuesCode options + // carry theirs outside the string table but must still test as present. OS << ", "; - writeOptionalStrOffset(OS, Table, R, "Values", ExplicitlyEmptyOffset); + if (!isa<UnsetInit>(R.getValueInit("ValuesCode"))) + OS << ExplicitlyEmptyOffset; + else + writeStrTableOffset(OS, Table, + R.getValueAsOptionalString("Values").value_or("")); // The option SubCommandIDsOffset. OS << ", "; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
