[PATCH] D143414: [clang] refactor FileManager::GetUniqueIDMapping

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

In general, I think this approach makes sense.



================
Comment at: clang/lib/Serialization/ASTWriter.cpp:1925
 
-  for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
-    const FileEntry *File = FilesByUID[UID];
-    if (!File)
-      continue;
+  for (auto &File : FileEntries) {
 
----------------
Nit: Probably not necessary to take `FileEntryRef` by reference.


================
Comment at: clang/lib/Serialization/ASTWriter.cpp:1939
     // Massage the file path into an appropriate form.
     StringRef Filename = File->getName();
     SmallString<128> FilenameTmp(Filename);
----------------
This will insert duplicate entries (with the same key) into the on-disk hash table. I don't know if that's problematic, just thought I'd call it out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143414