jansvoboda11 added inline comments.
================
Comment at: clang/include/clang/Basic/FileManager.h:316
+ void getSeenFileEntries(
+ SmallVectorImpl<OptionalFileEntryRefDegradesToFileEntryPtr> &Entries)
+ const;
----------------
Since we're already modifying the two only users of this function, maybe we
could use `Optional<FileEntryRef>` instead of
`OptionalFileEntryRefDegradesToFileEntryPtr` (which we want to eventually
remove)?
================
Comment at: clang/lib/Basic/FileManager.cpp:625-627
+ Entries.push_back(
+ FileEntryRef(*reinterpret_cast<const FileEntryRef::MapEntry *>(
+ Value->V.get<const void *>())));
----------------
Why is this necessary? IIUC, `FileEntryRef` has this logic in
`getBaseMapEntry()`. I would expect `FileEntryRef(Entry)` to be correct
regardless of what's in `Value->V`.
================
Comment at: clang/lib/Serialization/ASTWriter.cpp:176
+ for (auto &File : FileEntries) {
const HeaderFileInfo *HFI =
----------------
I'm afraid that iterating over file entries in "non-deterministic" order can
cause non-determinism down the line. For example, calling
`HeaderSearch::findAllModulesForHeader()` in the loop can trigger
deserialization of `HeaderFileInfo` from loaded PCMs. That code fills the
`Module::Headers` vectors, which we serialize without sorting first.
================
Comment at: clang/lib/Serialization/ASTWriter.cpp:1919-1922
+ if (A->getUID() == B->getUID())
+ return A->getName() < B->getName();
+ return A->getUID() < B->getUID();
----------------
Nit: I'd slightly prefer:
```
if (A->getUID() != B->getUID())
return A->getUID() < B->getUID();
return A->getName() < B->getName();
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143414/new/
https://reviews.llvm.org/D143414
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
