nik updated this revision to Diff 174022.
nik added a comment.
Addressed comments.
Repository:
rC Clang
https://reviews.llvm.org/D41005
Files:
include/clang/Frontend/ASTUnit.h
lib/Frontend/ASTUnit.cpp
lib/Frontend/PrecompiledPreamble.cpp
unittests/Frontend/PCHPreambleTest.cpp
Index: unittests/Frontend/PCHPreambleTest.cpp
===================================================================
--- unittests/Frontend/PCHPreambleTest.cpp
+++ unittests/Frontend/PCHPreambleTest.cpp
@@ -53,7 +53,10 @@
FileSystemOptions FSOpts;
public:
- void SetUp() override {
+ void SetUp() override { ResetVFS(); }
+ void TearDown() override {}
+
+ void ResetVFS() {
VFS = new ReadCountingInMemoryFileSystem();
// We need the working directory to be set to something absolute,
// otherwise it ends up being inadvertently set to the current
@@ -64,9 +67,6 @@
VFS->setCurrentWorkingDirectory("//./");
}
- void TearDown() override {
- }
-
void AddFile(const std::string &Filename, const std::string &Contents) {
::time_t now;
::time(&now);
@@ -124,6 +124,72 @@
}
};
+TEST_F(PCHPreambleTest, ReparseReusesPreambleWithUnsavedFileNotExistingOnDisk) {
+ std::string Header1 = "//./header1.h";
+ std::string MainName = "//./main.cpp";
+ AddFile(MainName, R"cpp(
+#include "//./header1.h"
+int main() { return ZERO; }
+)cpp");
+ RemapFile(Header1, "#define ZERO 0\n");
+
+ // Parse with header file provided as unsaved file, which does not exist on
+ // disk.
+ std::unique_ptr<ASTUnit> AST(ParseAST(MainName));
+ ASSERT_TRUE(AST.get());
+ ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+
+ // Reparse and check that the preamble was reused.
+ ASSERT_TRUE(ReparseAST(AST));
+ ASSERT_EQ(AST->getPreambleCounter(), 1U);
+}
+
+TEST_F(PCHPreambleTest, ReparseReusesPreambleAfterUnsavedFileWasCreatedOnDisk) {
+ std::string Header1 = "//./header1.h";
+ std::string MainName = "//./main.cpp";
+ AddFile(MainName, R"cpp(
+#include "//./header1.h"
+int main() { return ZERO; }
+)cpp");
+ RemapFile(Header1, "#define ZERO 0\n");
+
+ // Parse with header file provided as unsaved file, which does not exist on
+ // disk.
+ std::unique_ptr<ASTUnit> AST(ParseAST(MainName));
+ ASSERT_TRUE(AST.get());
+ ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+
+ // Create the unsaved file also on disk and check that preamble was reused.
+ AddFile(Header1, "#define ZERO 0\n");
+ ASSERT_TRUE(ReparseAST(AST));
+ ASSERT_EQ(AST->getPreambleCounter(), 1U);
+}
+
+TEST_F(PCHPreambleTest,
+ ReparseReusesPreambleAfterUnsavedFileWasRemovedFromDisk) {
+ std::string Header1 = "//./foo/../header1.h";
+ std::string MainName = "//./main.cpp";
+ std::string MainFileContent = R"cpp(
+#include "//./header1.h"
+int main() { return ZERO; }
+)cpp";
+ AddFile(MainName, MainFileContent);
+ AddFile(Header1, "#define ZERO 0\n");
+ RemapFile(Header1, "#define ZERO 0\n");
+
+ // Parse with header file provided as unsaved file, which exists on disk.
+ std::unique_ptr<ASTUnit> AST(ParseAST(MainName));
+ ASSERT_TRUE(AST.get());
+ ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+ ASSERT_EQ(AST->getPreambleCounter(), 1U);
+
+ // Remove the unsaved file from disk and check that the preamble was reused.
+ ResetVFS();
+ AddFile(MainName, MainFileContent);
+ ASSERT_TRUE(ReparseAST(AST));
+ ASSERT_EQ(AST->getPreambleCounter(), 1U);
+}
+
TEST_F(PCHPreambleTest, ReparseWithOverriddenFileDoesNotInvalidatePreamble) {
std::string Header1 = "//./header1.h";
std::string Header2 = "//./header2.h";
Index: lib/Frontend/PrecompiledPreamble.cpp
===================================================================
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -28,6 +28,7 @@
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Mutex.h"
#include "llvm/Support/MutexGuard.h"
+#include "llvm/Support/Path.h"
#include "llvm/Support/Process.h"
#include "llvm/Support/VirtualFileSystem.h"
#include <limits>
@@ -221,6 +222,14 @@
return true;
}
+template <typename IteratorType>
+llvm::SmallString<256> PathNormalized(IteratorType Start, IteratorType End) {
+ llvm::SmallString<256> Path{Start, End};
+ llvm::sys::path::remove_dots(Path, /*remove_dot_dots=*/true);
+ Path = llvm::sys::path::convert_to_slash(Path);
+ return Path;
+}
+
} // namespace
PreambleBounds clang::ComputePreambleBounds(const LangOptions &LangOpts,
@@ -367,13 +376,15 @@
const FileEntry *File = Clang->getFileManager().getFile(Filename);
if (!File || File == SourceMgr.getFileEntryForID(SourceMgr.getMainFileID()))
continue;
+ auto FilePath =
+ PathNormalized(File->getName().begin(), File->getName().end());
if (time_t ModTime = File->getModificationTime()) {
- FilesInPreamble[File->getName()] =
+ FilesInPreamble[FilePath] =
PrecompiledPreamble::PreambleFileHash::createForFile(File->getSize(),
ModTime);
} else {
llvm::MemoryBuffer *Buffer = SourceMgr.getMemoryBufferForFile(File);
- FilesInPreamble[File->getName()] =
+ FilesInPreamble[FilePath] =
PrecompiledPreamble::PreambleFileHash::createForMemoryBuffer(Buffer);
}
}
@@ -449,20 +460,34 @@
Status.getSize(), llvm::sys::toTimeT(Status.getLastModificationTime()));
}
+ llvm::StringMap<PreambleFileHash> OverridenFileBuffers;
for (const auto &RB : PreprocessorOpts.RemappedFileBuffers) {
- llvm::vfs::Status Status;
- if (!moveOnNoError(VFS->status(RB.first), Status))
- return false;
-
- OverriddenFiles[Status.getUniqueID()] =
+ const PrecompiledPreamble::PreambleFileHash PreambleHash =
PreambleFileHash::createForMemoryBuffer(RB.second);
+ llvm::vfs::Status Status;
+ if (moveOnNoError(VFS->status(RB.first), Status)) {
+ OverriddenFiles[Status.getUniqueID()] = PreambleHash;
+ } else {
+ auto FilePath = PathNormalized(RB.first.begin(), RB.first.end());
+ OverridenFileBuffers[FilePath] = PreambleHash;
+ }
}
// Check whether anything has changed.
for (const auto &F : FilesInPreamble) {
+ auto OverridenFileBuffer = OverridenFileBuffers.find(F.first());
+ if (OverridenFileBuffer != OverridenFileBuffers.end()) {
+ // The file's buffer was remapped; check whether it matches up
+ // with the previous mapping.
+ if (OverridenFileBuffer->second != F.second)
+ return false;
+ continue;
+ }
+
llvm::vfs::Status Status;
if (!moveOnNoError(VFS->status(F.first()), Status)) {
- // If we can't stat the file, assume that something horrible happened.
+ // If the file's buffer is not remapped and we can't stat it,
+ // assume that something horrible happened.
return false;
}
@@ -476,9 +501,10 @@
continue;
}
- // The file was not remapped; check whether it has changed on disk.
+ // Neither the file's buffer nor the file itself was remapped;
+ // check whether it has changed on disk.
if (Status.getSize() != uint64_t(F.second.Size) ||
- llvm::sys::toTimeT(Status.getLastModificationTime()) !=
+ llvm::sys::toTimeT(Status.getLastModificationTime()) !=
F.second.ModTime)
return false;
}
Index: lib/Frontend/ASTUnit.cpp
===================================================================
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -1308,22 +1308,22 @@
PreambleInvocationIn.getDiagnosticOpts());
getDiagnostics().setNumWarnings(NumWarningsInPreamble);
- PreambleRebuildCounter = 1;
+ PreambleRebuildCountdown = 1;
return MainFileBuffer;
} else {
Preamble.reset();
PreambleDiagnostics.clear();
TopLevelDeclsInPreamble.clear();
PreambleSrcLocCache.clear();
- PreambleRebuildCounter = 1;
+ PreambleRebuildCountdown = 1;
}
}
// If the preamble rebuild counter > 1, it's because we previously
// failed to build a preamble and we're not yet ready to try
// again. Decrement the counter and return a failure.
- if (PreambleRebuildCounter > 1) {
- --PreambleRebuildCounter;
+ if (PreambleRebuildCountdown > 1) {
+ --PreambleRebuildCountdown;
return nullptr;
}
@@ -1360,18 +1360,20 @@
if (NewPreamble) {
Preamble = std::move(*NewPreamble);
- PreambleRebuildCounter = 1;
+ ++PreambleCounter;
+ PreambleRebuildCountdown = 1;
} else {
switch (static_cast<BuildPreambleError>(NewPreamble.getError().value())) {
case BuildPreambleError::CouldntCreateTempFile:
// Try again next time.
- PreambleRebuildCounter = 1;
+ ++PreambleCounter;
+ PreambleRebuildCountdown = 1;
return nullptr;
case BuildPreambleError::CouldntCreateTargetInfo:
case BuildPreambleError::BeginSourceFileFailed:
case BuildPreambleError::CouldntEmitPCH:
// These erros are more likely to repeat, retry after some period.
- PreambleRebuildCounter = DefaultPreambleRebuildInterval;
+ PreambleRebuildCountdown = DefaultPreambleRebuildInterval;
return nullptr;
}
llvm_unreachable("unexpected BuildPreambleError");
@@ -1511,7 +1513,7 @@
AST->OnlyLocalDecls = OnlyLocalDecls;
AST->CaptureDiagnostics = CaptureDiagnostics;
if (PrecompilePreambleAfterNParses > 0)
- AST->PreambleRebuildCounter = PrecompilePreambleAfterNParses;
+ AST->PreambleRebuildCountdown = PrecompilePreambleAfterNParses;
AST->TUKind = Action ? Action->getTranslationUnitKind() : TU_Complete;
AST->ShouldCacheCodeCompletionResults = CacheCodeCompletionResults;
AST->IncludeBriefCommentsInCodeCompletion
@@ -1645,7 +1647,7 @@
std::unique_ptr<llvm::MemoryBuffer> OverrideMainBuffer;
if (PrecompilePreambleAfterNParses > 0) {
- PreambleRebuildCounter = PrecompilePreambleAfterNParses;
+ PreambleRebuildCountdown = PrecompilePreambleAfterNParses;
OverrideMainBuffer =
getMainBufferWithPrecompiledPreamble(PCHContainerOps, *Invocation, VFS);
getDiagnostics().Reset();
@@ -1823,7 +1825,7 @@
// If we have a preamble file lying around, or if we might try to
// build a precompiled preamble, do so now.
std::unique_ptr<llvm::MemoryBuffer> OverrideMainBuffer;
- if (Preamble || PreambleRebuildCounter > 0)
+ if (Preamble || PreambleRebuildCountdown > 0)
OverrideMainBuffer =
getMainBufferWithPrecompiledPreamble(PCHContainerOps, *Invocation, VFS);
Index: include/clang/Frontend/ASTUnit.h
===================================================================
--- include/clang/Frontend/ASTUnit.h
+++ include/clang/Frontend/ASTUnit.h
@@ -206,7 +206,10 @@
/// we'll attempt to rebuild the precompiled header. This way, if
/// building the precompiled preamble fails, we won't try again for
/// some number of calls.
- unsigned PreambleRebuildCounter = 0;
+ unsigned PreambleRebuildCountdown = 0;
+
+ /// Counter indicating how often the preamble was build in total.
+ unsigned PreambleCounter = 0;
/// Cache pairs "filename - source location"
///
@@ -575,6 +578,8 @@
mapLocationToPreamble(R.getEnd()));
}
+ unsigned getPreambleCounter() const { return PreambleCounter; }
+
// Retrieve the diagnostics associated with this AST
using stored_diag_iterator = StoredDiagnostic *;
using stored_diag_const_iterator = const StoredDiagnostic *;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
