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

rmaz updated this revision to Diff 497309. rmaz added a comment. remove `header_file_size()` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143414/new/ https://reviews.llvm.org/D143414 Files: clang/include/clang/Basic/FileManager.h clang/includ

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

benlangmuir added a comment. +1 for determinism. Okay I spent some time trying to understand how this code is used and the non-virtual paths make some sense now. I am a bit skeptical about this on-disk-hash-table by filepath but that's separate from this patch. Comment at:

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

rmaz added a comment. In D143414#4110461 , @benlangmuir wrote: >> This should allow the path serialization of input files to use the paths >> used when looking up a file entry, instead of the last reference. > > Isn't this at odds with not having the VF

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

benlangmuir added a comment. > This should allow the path serialization of input files to use the paths used > when looking up a file entry, instead of the last reference. Isn't this at odds with not having the VFS-mapped paths? It's not obvious to me why we want these specific semantics. Else

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

rmaz updated this revision to Diff 495512. rmaz edited the summary of this revision. rmaz added a comment. Use FileEntryRef, rename method Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143414/new/ https://reviews.llvm.org/D143414 Files: clang/in

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

jansvoboda11 added inline comments. Comment at: clang/lib/Basic/FileManager.cpp:624 + // file entry. + if (Value->V.dyn_cast()) +Entries.push_back(FileEntryRef(Entry)); Nit: I think `dyn_cast()` could be replaced by `is()`. Repository: rG LL

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

jansvoboda11 added inline comments. Comment at: clang/include/clang/Basic/FileManager.h:316 + void getSeenFileEntries( + SmallVectorImpl &Entries) + const; rmaz wrote: > jansvoboda11 wrote: > > Since we're already modifying the two only users of this f

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

rmaz updated this revision to Diff 495259. rmaz added a comment. Don't return virtual file entries Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143414/new/ https://reviews.llvm.org/D143414 Files: clang/include/clang/Basic/FileManager.h clang/

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

jansvoboda11 added inline comments. Comment at: clang/lib/Basic/FileManager.cpp:625-627 +Entries.push_back( +FileEntryRef(*reinterpret_cast( +Value->V.get(; rmaz wrote: > jansvoboda11 wrote: > > Why is this necessary? IIUC,

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

rmaz added inline comments. Comment at: clang/include/clang/Basic/FileManager.h:316 + void getSeenFileEntries( + SmallVectorImpl &Entries) + const; jansvoboda11 wrote: > Since we're already modifying the two only users of this function, maybe we > cou

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

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) -

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

jansvoboda11 added inline comments. Comment at: clang/include/clang/Basic/FileManager.h:316 + void getSeenFileEntries( + SmallVectorImpl &Entries) + const; Since we're already modifying the two only users of this function, maybe we could use `Optional

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

rmaz updated this revision to Diff 495191. rmaz added a comment. Sort by UIDs, then Name Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143414/new/ https://reviews.llvm.org/D143414 Files: clang/include/clang/Basic/FileManager.h clang/lib/Basic/

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

rmaz created this revision. Herald added a subscriber: mgrang. Herald added a project: All. rmaz requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Refactor `FileManager::GetUniqueIDMapping` to populate an array of `OptionalFileEntryRefDegrad