[PATCH] D87736: [PDB] Split TypeServerSource and extend type index map lifetime

[Alexandre Ganea via Phabricator via llvm-commits]

aganea accepted this revision.
aganea added a comment.
This revision is now accepted and ready to land.

Looks great, thanks!



================
Comment at: lld/COFF/InputFiles.cpp:793
+    if (!debugChunks.empty())
+      debugTypesObj = makeTpiSource(this);
     return;
----------------
I am fine //without// this, I simply forgot about this case, where we had symbols but no types. Would that affect your tpi-index in D87805? This empty `TpiSource` will also receive an index and will be scheduled for doing, essentially, nothing? I am fine if you want to revert to an extra `if` below, whichever you think is the best.


================
Comment at: lld/COFF/PDB.cpp:931
-
-  if (source->kind == TpiSource::PDB)
-    return; // No symbols in TypeServer PDBs
----------------
rnk wrote:
> aganea wrote:
> > I liked it better with this `if` as it is self-explantory. Is there another situation where we should return here? The `TypeServerIpiSource` is never returned, right?
> Actually, we do come here for IPI sources. We iterate over all TpiSource objects and call addDebug on them, so if we keep this if, it will need two conditions. IMO the new `if` handles it more neatly: only object files have symbols, so if this is a source for an object file, add symbols.
Agreed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87736/new/

https://reviews.llvm.org/D87736