[PATCH] D122372: [lld][COFF] Fix TypeServerSource lookup on GUID collisions

[Tobias Hieta via Phabricator via llvm-commits]

thieta added inline comments.


================
Comment at: lld/COFF/DebugTypes.cpp:66
+          tSrc->pdbInputFile->session->getPDBFile().getFilePath());
+      ctx.typeServerSourceMappings.erase(Guid);
+    }
----------------
aganea wrote:
> I'm having some doubts regarding this line (L66). If we have an odd number of PDBs queued with bad GUIDs, the first one will `ctx.typeServerSourceMappings.emplace()`, the second will do `ctx.typeServerSourceMappings.erase()` and the third will `ctx.typeServerSourceMappings.emplace()` again. I think we need a different strategy here to handle an arbitrary number of PDBs with bad (FFFF..) GUIDs. Any suggestions? I would be tempted to keep the entry in the map and not erase it -- that could help later with multithreading, if we ever do that. What about doing `it.first->second = nullptr;` when there's a collision, and then handle this case later in `UseTypeServerSource::getTypeServerSource()` to fallback to the lookup by path?
Yeah this is much nicer - I actually had the thought before and thought it didn't matter that much - but it's definitely the better solution. Posted a follow-up diff here: https://reviews.llvm.org/D123185


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122372