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.kos...@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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits