[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

[Ilya Biryukov via Phabricator via cfe-commits]

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