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

[Nikolai Kosjar via Phabricator via cfe-commits]

nik added inline comments.


================
Comment at: lib/Frontend/PrecompiledPreamble.cpp:461
 
     std::map<llvm::sys::fs::UniqueID, PreambleFileHash>::iterator Overridden =
         OverriddenFiles.find(Status.getUniqueID());
----------------
nik wrote:
> ilya-biryukov wrote:
> > Will anything fail if we remove the map from `UniqueID` to hashes of overriden files and the corresponding checks?
> > 
> > We should either document why having `UniqueID`-based checks is important or remove the code doing those checks.
> > Will anything fail if we remove the map from UniqueID to hashes of overriden files and the corresponding checks?
> 
> Hmm, I would need to remove/replace it and run the tests.
> 
> I see these reasons for UniqueID:
> * It's already there :)
> * Normalization of file paths is not necessary.
> * It potentially can deal with hard links, though I'm not sure whether this is real world use case at all.
> 
> > We should either document why having UniqueID-based checks is important or remove the code doing those checks.
> 
> Agree.
> 
> Will anything fail if we remove the map from UniqueID to hashes of overriden files and the corresponding checks?

OK, I've applied the following patch (on this patch set/revision here) applied and run check-clang - no failures here.

```
>From 8f755128d1e167193c8f6de1456aec01d14307f6 Mon Sep 17 00:00:00 2001
From: Nikolai Kosjar <nikolai.kosjar at qt.io>
Date: Tue, 2 Jan 2018 15:14:07 +0100
Subject: [PATCH] Use file path instead of uniqueID

---
 lib/Frontend/PrecompiledPreamble.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/Frontend/PrecompiledPreamble.cpp b/lib/Frontend/PrecompiledPreamble.cpp
index a43205a1c3..09e2b20a48 100644
--- a/lib/Frontend/PrecompiledPreamble.cpp
+++ b/lib/Frontend/PrecompiledPreamble.cpp
@@ -433,7 +433,7 @@ bool PrecompiledPreamble::CanReuse(const CompilerInvocation &Invocation,
   // Check that none of the files used by the preamble have changed.
   // First, make a record of those files that have been overridden via
   // remapping or unsaved_files.
-  std::map<llvm::sys::fs::UniqueID, PreambleFileHash> OverriddenFiles;
+  std::map<std::string, PreambleFileHash> OverriddenFiles;
   for (const auto &R : PreprocessorOpts.RemappedFiles) {
     vfs::Status Status;
     if (!moveOnNoError(VFS->status(R.second), Status)) {
@@ -442,7 +442,7 @@ bool PrecompiledPreamble::CanReuse(const CompilerInvocation &Invocation,
       return false;
     }
 
-    OverriddenFiles[Status.getUniqueID()] = PreambleFileHash::createForFile(
+    OverriddenFiles[R.first] = PreambleFileHash::createForFile(
         Status.getSize(), llvm::sys::toTimeT(Status.getLastModificationTime()));
   }
 
@@ -470,8 +470,8 @@ bool PrecompiledPreamble::CanReuse(const CompilerInvocation &Invocation,
         return false;
     }
 
-    std::map<llvm::sys::fs::UniqueID, PreambleFileHash>::iterator Overridden =
-        OverriddenFiles.find(Status.getUniqueID());
+    std::map<std::string, PreambleFileHash>::iterator Overridden =
+        OverriddenFiles.find(F.first());
     if (Overridden != OverriddenFiles.end()) {
       // This file was remapped; check whether the newly-mapped file
       // matches up with the previous mapping.
-- 
2.15.1
```


Repository:
  rC Clang

https://reviews.llvm.org/D41005