jansvoboda11 added inline comments.
================ Comment at: clang/include/clang/Serialization/ModuleFile.h:72 + bool TopLevel; + bool ModuleMap; }; ---------------- benlangmuir wrote: > Is there something using this new split? It seems like every user is using > `&&` for these You're right. I recall needing this on (now probably abandoned) patch, so I thought I might as well add the extra bit now, sine I'm changing the format anyways. I'm fine with removing this. ================ 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; ---------------- 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. ================ Comment at: clang/lib/Serialization/ASTReader.cpp:9323 +void ASTReader::visitInputFileInfos( + serialization::ModuleFile &MF, bool IncludeSystem, ---------------- benlangmuir wrote: > Can we rewrite `visitInputFiles` on top of this? I can try consolidating these in a follow-up if you're fine with that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157066/new/ https://reviews.llvm.org/D157066 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
