[StrTable] Switch the option parser to `llvm::StringTable` (#123308)

Now that we have a dedicated abstraction for string tables, switch the
option parser library's string table over to it rather than using a raw
`const char*`. Also try to use the `StringTable::Offset` type rather
than a raw `unsigned` where we can to avoid accidental increments or
other issues.

This is based on review feedback for the initial switch of options to a
string table. Happy to tweak or adjust if desired here.

1 files changed, 36 insertions, 34 deletions

diff --git a/llvm/lib/Option/OptTable.cpp b/llvm/lib/Option/OptTable.cpp
index 87e6f1f..6d10e61 100644
--- a/llvm/lib/Option/OptTable.cpp
+++ b/llvm/lib/Option/OptTable.cpp
@@ -33,11 +33,12 @@ using namespace llvm::opt;
 
 namespace {
 struct OptNameLess {
-  const char *StrTable;
-  ArrayRef<unsigned> PrefixesTable;
+  const StringTable *StrTable;
+  ArrayRef<StringTable::Offset> PrefixesTable;
 
-  explicit OptNameLess(const char *StrTable, ArrayRef<unsigned> PrefixesTable)
-      : StrTable(StrTable), PrefixesTable(PrefixesTable) {}
+  explicit OptNameLess(const StringTable &StrTable,
+                       ArrayRef<StringTable::Offset> PrefixesTable)
+      : StrTable(&StrTable), PrefixesTable(PrefixesTable) {}
 
 #ifndef NDEBUG
   inline bool operator()(const OptTable::Info &A,
@@ -45,13 +46,13 @@ struct OptNameLess {
     if (&A == &B)
       return false;
 
-    if (int Cmp = StrCmpOptionName(A.getName(StrTable, PrefixesTable),
-                                   B.getName(StrTable, PrefixesTable)))
+    if (int Cmp = StrCmpOptionName(A.getName(*StrTable, PrefixesTable),
+                                   B.getName(*StrTable, PrefixesTable)))
       return Cmp < 0;
 
     SmallVector<StringRef, 8> APrefixes, BPrefixes;
-    A.appendPrefixes(StrTable, PrefixesTable, APrefixes);
-    B.appendPrefixes(StrTable, PrefixesTable, BPrefixes);
+    A.appendPrefixes(*StrTable, PrefixesTable, APrefixes);
+    B.appendPrefixes(*StrTable, PrefixesTable, BPrefixes);
 
     if (int Cmp = StrCmpOptionPrefixes(APrefixes, BPrefixes))
       return Cmp < 0;
@@ -68,7 +69,7 @@ struct OptNameLess {
   // Support lower_bound between info and an option name.
   inline bool operator()(const OptTable::Info &I, StringRef Name) const {
     // Do not fallback to case sensitive comparison.
-    return StrCmpOptionName(I.getName(StrTable, PrefixesTable), Name, false) <
+    return StrCmpOptionName(I.getName(*StrTable, PrefixesTable), Name, false) <
            0;
   }
 };
@@ -76,9 +77,10 @@ struct OptNameLess {
 
 OptSpecifier::OptSpecifier(const Option *Opt) : ID(Opt->getID()) {}
 
-OptTable::OptTable(const char *StrTable, ArrayRef<unsigned> PrefixesTable,
+OptTable::OptTable(const StringTable &StrTable,
+                   ArrayRef<StringTable::Offset> PrefixesTable,
                    ArrayRef<Info> OptionInfos, bool IgnoreCase)
-    : StrTable(StrTable), PrefixesTable(PrefixesTable),
+    : StrTable(&StrTable), PrefixesTable(PrefixesTable),
       OptionInfos(OptionInfos), IgnoreCase(IgnoreCase) {
   // Explicitly zero initialize the error to work around a bug in array
   // value-initialization on MinGW with gcc 4.3.5.
@@ -151,13 +153,13 @@ static bool isInput(const ArrayRef<StringRef> &Prefixes, StringRef Arg) {
 }
 
 /// \returns Matched size. 0 means no match.
-static unsigned matchOption(const char *StrTable,
-                            ArrayRef<unsigned> PrefixesTable,
+static unsigned matchOption(const StringTable &StrTable,
+                            ArrayRef<StringTable::Offset> PrefixesTable,
                             const OptTable::Info *I, StringRef Str,
                             bool IgnoreCase) {
   StringRef Name = I->getName(StrTable, PrefixesTable);
-  for (unsigned PrefixOffset : I->getPrefixOffsets(PrefixesTable)) {
-    StringRef Prefix = &StrTable[PrefixOffset];
+  for (auto PrefixOffset : I->getPrefixOffsets(PrefixesTable)) {
+    StringRef Prefix = StrTable[PrefixOffset];
     if (Str.starts_with(Prefix)) {
       StringRef Rest = Str.substr(Prefix.size());
       bool Matched = IgnoreCase ? Rest.starts_with_insensitive(Name)
@@ -170,13 +172,13 @@ static unsigned matchOption(const char *StrTable,
 }
 
 // Returns true if one of the Prefixes + In.Names matches Option
-static bool optionMatches(const char *StrTable,
-                          ArrayRef<unsigned> PrefixesTable,
+static bool optionMatches(const StringTable &StrTable,
+                          ArrayRef<StringTable::Offset> PrefixesTable,
                           const OptTable::Info &In, StringRef Option) {
   StringRef Name = In.getName(StrTable, PrefixesTable);
   if (Option.consume_back(Name))
-    for (unsigned PrefixOffset : In.getPrefixOffsets(PrefixesTable))
-      if (Option == &StrTable[PrefixOffset])
+    for (auto PrefixOffset : In.getPrefixOffsets(PrefixesTable))
+      if (Option == StrTable[PrefixOffset])
         return true;
   return false;
 }
@@ -189,7 +191,7 @@ 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))
+    if (!In.Values || !optionMatches(*StrTable, PrefixesTable, In, Option))
       continue;
 
     SmallVector<StringRef, 8> Candidates;
@@ -217,9 +219,9 @@ OptTable::findByPrefix(StringRef Cur, Visibility VisibilityMask,
     if (In.Flags & DisableFlags)
       continue;
 
-    StringRef Name = In.getName(StrTable, PrefixesTable);
-    for (unsigned PrefixOffset : In.getPrefixOffsets(PrefixesTable)) {
-      StringRef Prefix = &StrTable[PrefixOffset];
+    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;
@@ -271,7 +273,7 @@ unsigned OptTable::internalFindNearest(
 
   for (const Info &CandidateInfo :
        ArrayRef<Info>(OptionInfos).drop_front(FirstSearchableIndex)) {
-    StringRef CandidateName = CandidateInfo.getName(StrTable, PrefixesTable);
+    StringRef CandidateName = CandidateInfo.getName(*StrTable, PrefixesTable);
 
     // We can eliminate some option prefix/name pairs as candidates right away:
     // * Ignore option candidates with empty names, such as "--", or names
@@ -304,9 +306,9 @@ unsigned OptTable::internalFindNearest(
     // Consider each possible prefix for each candidate to find the most
     // appropriate one. For example, if a user asks for "--helm", suggest
     // "--help" over "-help".
-    for (unsigned CandidatePrefixOffset :
+    for (auto CandidatePrefixOffset :
          CandidateInfo.getPrefixOffsets(PrefixesTable)) {
-      StringRef CandidatePrefix = &StrTable[CandidatePrefixOffset];
+      StringRef CandidatePrefix = (*StrTable)[CandidatePrefixOffset];
       // If Candidate and NormalizedName have more than 'BestDistance'
       // characters of difference, no need to compute the edit distance, it's
       // going to be greater than BestDistance. Don't bother computing Candidate
@@ -359,14 +361,14 @@ std::unique_ptr<Arg> OptTable::parseOneArgGrouped(InputArgList &Args,
   StringRef Name = Str.ltrim(PrefixChars);
   const Info *Start =
       std::lower_bound(OptionInfos.data() + FirstSearchableIndex, End, Name,
-                       OptNameLess(StrTable, PrefixesTable));
+                       OptNameLess(*StrTable, PrefixesTable));
   const Info *Fallback = nullptr;
   unsigned Prev = Index;
 
   // Search for the option which matches Str.
   for (; Start != End; ++Start) {
     unsigned ArgSize =
-        matchOption(StrTable, PrefixesTable, Start, Str, IgnoreCase);
+        matchOption(*StrTable, PrefixesTable, Start, Str, IgnoreCase);
     if (!ArgSize)
       continue;
 
@@ -449,7 +451,7 @@ std::unique_ptr<Arg> OptTable::internalParseOneArg(
 
   // Search for the first next option which could be a prefix.
   Start =
-      std::lower_bound(Start, End, Name, OptNameLess(StrTable, PrefixesTable));
+      std::lower_bound(Start, End, Name, OptNameLess(*StrTable, PrefixesTable));
 
   // Options are stored in sorted order, with '\0' at the end of the
   // alphabet. Since the only options which can accept a string must
@@ -464,7 +466,7 @@ std::unique_ptr<Arg> OptTable::internalParseOneArg(
     // Scan for first option which is a proper prefix.
     for (; Start != End; ++Start)
       if ((ArgSize =
-               matchOption(StrTable, PrefixesTable, Start, Str, IgnoreCase)))
+               matchOption(*StrTable, PrefixesTable, Start, Str, IgnoreCase)))
         break;
     if (Start == End)
       break;
@@ -787,15 +789,15 @@ void OptTable::internalPrintHelp(
   OS.flush();
 }
 
-GenericOptTable::GenericOptTable(const char *StrTable,
-                                 ArrayRef<unsigned> PrefixesTable,
+GenericOptTable::GenericOptTable(const StringTable &StrTable,
+                                 ArrayRef<StringTable::Offset> PrefixesTable,
                                  ArrayRef<Info> OptionInfos, bool IgnoreCase)
     : OptTable(StrTable, PrefixesTable, OptionInfos, IgnoreCase) {
 
   std::set<StringRef> TmpPrefixesUnion;
   for (auto const &Info : OptionInfos.drop_front(FirstSearchableIndex))
-    for (unsigned PrefixOffset : Info.getPrefixOffsets(PrefixesTable))
-      TmpPrefixesUnion.insert(StringRef(&StrTable[PrefixOffset]));
+    for (auto PrefixOffset : Info.getPrefixOffsets(PrefixesTable))
+      TmpPrefixesUnion.insert(StrTable[PrefixOffset]);
   PrefixesUnion.append(TmpPrefixesUnion.begin(), TmpPrefixesUnion.end());
   buildPrefixChars();
 }