[PATCH] D66431: [PDB] Fix bug when using multiple PCH header objects with the same name.

[Alexandre Ganea via Phabricator via llvm-commits]

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

Thanks for fixing this Zachary! Our precomp files are named after their project, ie. `Engine.pch.obj`

LGTM, a few comments:



================
Comment at: lld/COFF/PDB.cpp:511
+    SmallString<128> path1_norm = sys::path::convert_to_slash(path1);
+    SmallString<128> path2_norm = sys::path::convert_to_slash(path2);
 #if defined(_WIN32)
----------------
These two lines do not compile on my PC. I had to change to: `SmallString<128> path1_norm{StringRef(sys::path::convert_to_slash(path1))};`


================
Comment at: lld/COFF/PDB.cpp:521
+static ObjFile *findObjByName(StringRef filePath) {
   for (ObjFile *f : ObjFile::instances) {
+    if (equals_path(filePath, f->getName()))
----------------
rnk wrote:
> Erg, this is obviously quadratic in input objects. Whatever, it's not your problem, either it shows up in the profile or it doesn't.
This won't be an issue once the remaining steps in D59226 are done, I need to find time to send the reviews. We separately parse PCHs and (input) PDBs before any other OBJ files, and later use a hashtable for looking up the corresponding PCH.


================
Comment at: lld/COFF/PDB.cpp:554
   // the paths embedded in the OBJs are in the Windows format.
   SmallString<128> precompFileName = sys::path::filename(
       precomp.getPrecompFilePath(), sys::path::Style::windows);
----------------
You could probably remove this now, and replace usage below in `createFileError` with `precomp.getPrecompFilePath()`.


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

https://reviews.llvm.org/D66431