[PATCH] D157066: [clang][modules][deps] Create more efficient API for visitation of `ModuleFile` inputs

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.


================
Comment at: clang/lib/Serialization/ASTReader.cpp:2411
   bool Transient = FI.Transient;
-  StringRef Filename = FI.Filename;
+  StringRef Filename = FI.FilenameAsRequested;
   uint64_t StoredContentHash = FI.ContentHash;
----------------
jansvoboda11 wrote:
> benlangmuir wrote:
> > It's not clear to me why this one changed
> This actually maintains the same semantics - `FI.Filename` was previously the as-requested path, now it's the on-disk path. Without changing this line, `FileManager::getFileRef()` would get called with the on-disk path, meaning consumers of this function would get the incorrect path when calling `FileEntryRef::getNameAsRequested()` on the returned file. I recall one clang-scan-deps test failing because of it.
Just remembered: in `ModuleDepCollector.cpp`, we call `ModuleMap::getModuleMapFileForUniquing()`. This calls `SourceMgr.getFileEntryRefForID()` based on `Module::DefinitionLoc`, which triggers deserialization of the associated source location entry and ends up calling this function right here. We'd end up with the on-disk module map path for modular dependencies.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157066