ilya-biryukov requested changes to this revision.
ilya-biryukov added a comment.
This revision now requires changes to proceed.
This looks useful. Could we avoid creating the `OverlayFileSystem`, though?
================
Comment at: include/clang/Frontend/PrecompiledPreamble.h:109
+ std::chrono::steady_clock::time_point getCreationTimePoint() const {
+ return CreationTimePoint;
----------------
Having this time stamp in the interface of `PrecompiledPreamble` doesn't really
makes sense.
I propose we remove this method and test in a different way instead.
For example, we could add a counter to `ASTUnit` that increases when preamble
is built and check this counter.
Or we could add a unit-test that uses `PrecompiledPreamble` directly.
================
Comment at: lib/Frontend/PrecompiledPreamble.cpp:437
+ vfs::OverlayFileSystem OFS(VFS);
+ IntrusiveRefCntPtr<vfs::InMemoryFileSystem> IMFS(
----------------
Can we do the same thing without creating an additional `OverlayFileSystem`?
If we add a separate map to check for those:
```
llvm::StringMap<PreambleFileHash> OverriddenFilesWithoutFS.
// Populate the map with paths not existing in VFS.
for (const auto &F : FilesInPreamble) {
vfs::Status Status;
if (!moveOnNoError(OFS.status(F.first()), Status)) {
// If we can't stat the file, check whether it was overriden
auto It = OverriddenFilesWithoutFS[F.first()];
if (It == OverriddenFilesWithoutFS.end())
return false;
//.....
}
//......
}
```
================
Comment at: lib/Frontend/PrecompiledPreamble.cpp:444
vfs::Status Status;
- if (!moveOnNoError(VFS->status(RB.first), Status))
- return false;
-
+ assert(moveOnNoError(IMFS->status(RB.first), Status));
OverriddenFiles[Status.getUniqueID()] =
----------------
`assert` macro is a no-op when `NDEBUG` is defined.
One must never put an operation with side-effects inside `assert`.
Repository:
rC Clang
https://reviews.llvm.org/D41005
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
