================
@@ -893,6 +911,80 @@ 
APINotesReader::Implementation::getIdentifier(llvm::StringRef Str) {
   return *Known;
 }
 
+std::optional<llvm::StringRef>
+APINotesReader::Implementation::getIdentifierString(IdentifierID ID) {
+  if (!IdentifierTable)
+    return std::nullopt;
+
+  if (ID == IdentifierID(0))
+    return llvm::StringRef();
+
+  if (!IdentifierStringsInitialized) {
+    IdentifierStringsInitialized = true;
+    // keys() and data() iterate over the same serialized entries in lockstep,
+    // so build the reverse cache without doing a lookup for each key.
+    auto Identifiers = IdentifierTable->keys();
+    auto IDs = IdentifierTable->data();
+    auto Identifier = Identifiers.begin();
+    auto KnownID = IDs.begin();
+    auto IdentifierEnd = Identifiers.end();
+    auto KnownIDEnd = IDs.end();
+    for (; Identifier != IdentifierEnd && KnownID != KnownIDEnd;
+         ++Identifier, ++KnownID) {
+      unsigned Index = static_cast<unsigned>(*KnownID);
+      if (IdentifierStrings.size() <= Index)
----------------
StoeckOverflow wrote:

The current `resize(ID + 1)` form is needed because the vector index is the 
identifier ID. Even if the table visits IDs out of order, we still store each
string at `IdentifierStrings[ID]`. `push_back` would change the meaning of the 
vector index to iteration order. That would only be correct if the table 
visited identifiers densely in increasing ID order, which 
`OnDiskIterableChainedHashTable` does not guarantee.

I agree that the vector shape is not ideal here. A `DenseMap<uint32_t,
StringRef>` would keep the same ID-based lookup semantics without depending on 
ID order or growing the vector to fit the largest identifier ID seen, so I 
think that is the cleaner choice.

https://github.com/llvm/llvm-project/pull/209408
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to