cameron314 created this revision.
cameron314 added a reviewer: rsmith.
cameron314 added a subscriber: cfe-commits.
There were two bugs relating to remapped files, that are not specific to
Windows but happen more often there:
- When remapped files were changed, they would never cause the preamble's PCH
to be invalidated, because the remapped path didn't necessarily match the
include path (e.g. slash direction). I fixed this by moving to a
`llvm::sys::fs::UniqueID`-based map instead of using strings for paths and
hoping they match.
- Fixing the above bug revealed a new bug where remapped files would always
cause the preamble's PCH to be invalidated (even if they hadn't changed)
because the file manager was replacing the remapped virtual entry (no modified
time) with a real file entry (which has a modified time) when the include
directive was processed, again because it had a slightly different path due to
slash direction.
http://reviews.llvm.org/D20137
Files:
include/clang/Basic/FileManager.h
include/clang/Frontend/ASTUnit.h
lib/Basic/FileManager.cpp
lib/Frontend/ASTUnit.cpp
Index: lib/Frontend/ASTUnit.cpp
===================================================================
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -1378,7 +1378,7 @@
// First, make a record of those files that have been overridden via
// remapping or unsaved_files.
- llvm::StringMap<PreambleFileHash> OverriddenFiles;
+ std::map<llvm::sys::fs::UniqueID, PreambleFileHash> OverriddenFiles;
for (const auto &R : PreprocessorOpts.RemappedFiles) {
if (AnyFileChanged)
break;
@@ -1391,40 +1391,47 @@
break;
}
- OverriddenFiles[R.first] = PreambleFileHash::createForFile(
+ OverriddenFiles[Status.getUniqueID()] = PreambleFileHash::createForFile(
Status.getSize(), Status.getLastModificationTime().toEpochTime());
}
for (const auto &RB : PreprocessorOpts.RemappedFileBuffers) {
if (AnyFileChanged)
break;
- OverriddenFiles[RB.first] =
+
+ vfs::Status Status;
+ if (FileMgr->getNoncachedStatValue(RB.first, Status)) {
+ AnyFileChanged = true;
+ break;
+ }
+
+ OverriddenFiles[Status.getUniqueID()] =
PreambleFileHash::createForMemoryBuffer(RB.second);
}
// Check whether anything has changed.
- for (llvm::StringMap<PreambleFileHash>::iterator
+ for (FilesInPreambleMap::iterator
F = FilesInPreamble.begin(), FEnd = FilesInPreamble.end();
!AnyFileChanged && F != FEnd;
++F) {
- llvm::StringMap<PreambleFileHash>::iterator Overridden
- = OverriddenFiles.find(F->first());
+ std::map<llvm::sys::fs::UniqueID, 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.
- if (Overridden->second != F->second)
+ if (Overridden->second != F->second.second)
AnyFileChanged = true;
continue;
}
// The file was not remapped; check whether it has changed on disk.
vfs::Status Status;
- if (FileMgr->getNoncachedStatValue(F->first(), Status)) {
+ if (FileMgr->getNoncachedStatValue(F->second.first, Status)) {
// If we can't stat the file, assume that something horrible happened.
AnyFileChanged = true;
- } else if (Status.getSize() != uint64_t(F->second.Size) ||
+ } else if (Status.getSize() != uint64_t(F->second.second.Size) ||
Status.getLastModificationTime().toEpochTime() !=
- uint64_t(F->second.ModTime))
+ uint64_t(F->second.second.ModTime))
AnyFileChanged = true;
}
@@ -1612,12 +1619,14 @@
if (!File || File == SourceMgr.getFileEntryForID(SourceMgr.getMainFileID()))
continue;
if (time_t ModTime = File->getModificationTime()) {
- FilesInPreamble[File->getName()] = PreambleFileHash::createForFile(
- File->getSize(), ModTime);
+ FilesInPreamble[File->getUniqueID()] = std::make_pair(
+ StringRef(File->getName()),
+ PreambleFileHash::createForFile(File->getSize(), ModTime));
} else {
llvm::MemoryBuffer *Buffer = SourceMgr.getMemoryBufferForFile(File);
- FilesInPreamble[File->getName()] =
- PreambleFileHash::createForMemoryBuffer(Buffer);
+ FilesInPreamble[File->getUniqueID()] = std::make_pair(
+ StringRef(File->getName()),
+ PreambleFileHash::createForMemoryBuffer(Buffer));
}
}
Index: lib/Basic/FileManager.cpp
===================================================================
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -301,6 +301,11 @@
return &UFE;
}
+ if (UFE.isVirtual()) {
+ UFE.Name = InterndFileName;
+ return &UFE;
+ }
+
// Otherwise, we don't have this file yet, add it.
UFE.Name = InterndFileName;
UFE.Size = Data.Size;
@@ -312,6 +317,7 @@
UFE.InPCH = Data.InPCH;
UFE.File = std::move(F);
UFE.IsValid = true;
+ UFE.IsVirtual = true;
return &UFE;
}
Index: include/clang/Frontend/ASTUnit.h
===================================================================
--- include/clang/Frontend/ASTUnit.h
+++ include/clang/Frontend/ASTUnit.h
@@ -31,6 +31,7 @@
#include "llvm/ADT/StringMap.h"
#include "llvm/Support/MD5.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/FileSystem.h"
#include <cassert>
#include <map>
#include <memory>
@@ -264,12 +265,15 @@
/// a line after skipping the preamble.
bool PreambleEndsAtStartOfLine;
+ typedef std::map<llvm::sys::fs::UniqueID,
+ std::pair<StringRef, PreambleFileHash>> FilesInPreambleMap;
+
/// \brief Keeps track of the files that were used when computing the
/// preamble, with both their buffer size and their modification time.
///
/// If any of the files have changed from one compile to the next,
/// the preamble must be thrown away.
- llvm::StringMap<PreambleFileHash> FilesInPreamble;
+ FilesInPreambleMap FilesInPreamble;
/// \brief When non-NULL, this is the buffer used to store the contents of
/// the main file when it has been padded for use with the precompiled
Index: include/clang/Basic/FileManager.h
===================================================================
--- include/clang/Basic/FileManager.h
+++ include/clang/Basic/FileManager.h
@@ -60,6 +60,7 @@
bool IsNamedPipe;
bool InPCH;
bool IsValid; // Is this \c FileEntry initialized and valid?
+ bool IsVirtual; // Is this \c FileEntry a virtual file?
/// \brief The open file, if it is owned by the \p FileEntry.
mutable std::unique_ptr<vfs::File> File;
@@ -69,20 +70,23 @@
public:
FileEntry()
- : UniqueID(0, 0), IsNamedPipe(false), InPCH(false), IsValid(false)
+ : UniqueID(0, 0), IsNamedPipe(false), InPCH(false), IsValid(false),
+ IsVirtual(false)
{}
// FIXME: this is here to allow putting FileEntry in std::map. Once we have
// emplace, we shouldn't need a copy constructor anymore.
/// Intentionally does not copy fields that are not set in an uninitialized
/// \c FileEntry.
FileEntry(const FileEntry &FE) : UniqueID(FE.UniqueID),
- IsNamedPipe(FE.IsNamedPipe), InPCH(FE.InPCH), IsValid(FE.IsValid) {
+ IsNamedPipe(FE.IsNamedPipe), InPCH(FE.InPCH), IsValid(FE.IsValid),
+ IsVirtual(FE.IsVirtual) {
assert(!isValid() && "Cannot copy an initialized FileEntry");
}
const char *getName() const { return Name; }
bool isValid() const { return IsValid; }
+ bool isVirtual() const { return IsVirtual; }
off_t getSize() const { return Size; }
unsigned getUID() const { return UID; }
const llvm::sys::fs::UniqueID &getUniqueID() const { return UniqueID; }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
