nik updated this revision to Diff 128416.
nik added a comment.
Rebased and renamed the counter variable only.
I do not feel comfortable changing the "std::map<llvm::sys::fs::UniqueID,
PreambleFileHash> OverriddenFiles". I can do this in a follow-up change if you
want.
@Ivan: Coul you please run the tests with this change on Windows?! If it goes
well (no failures), then please also give it also a try with the additional
change (reply to "Will anything fail if we remove the map from UniqueID to
hashes of overriden files and the corresponding checks?").
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
@@ -124,6 +124,22 @@
}
};
+TEST_F(PCHPreambleTest, ReparseDoesNotInvalidatePreambleDueToNotExistingUnsavedFile) {
+ std::string Header1 = "//./header1.h";
+ std::string MainName = "//./main.cpp";
+ AddFile(MainName, "#include \"//./header1.h\"\n"
+ "int main() { return ZERO; }");
+
+ std::unique_ptr<ASTUnit> AST(ParseAST(MainName));
+ ASSERT_TRUE(AST.get());
+ ASSERT_EQ(AST->getPreambleCounter(), 1U);
+
+ RemapFile(Header1, "#define ZERO 0\n");
+ 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
@@ -446,21 +446,28 @@
Status.getSize(), llvm::sys::toTimeT(Status.getLastModificationTime()));
}
+ llvm::StringMap<PreambleFileHash> OverridenFileBuffers;
for (const auto &RB : PreprocessorOpts.RemappedFileBuffers) {
- vfs::Status Status;
- if (!moveOnNoError(VFS->status(RB.first), Status))
- return false;
-
- OverriddenFiles[Status.getUniqueID()] =
+ OverridenFileBuffers[RB.first] =
PreambleFileHash::createForMemoryBuffer(RB.second);
}
// 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;
+ }
+
vfs::Status Status;
if (!moveOnNoError(VFS->status(F.first()), Status)) {
- // If we can't stat the file, assume that something horrible happened.
- return false;
+ // If the file's buffer is not remapped and we can't stat it,
+ // assume that something horrible happened.
+ return false;
}
std::map<llvm::sys::fs::UniqueID, PreambleFileHash>::iterator Overridden =
@@ -473,9 +480,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
@@ -189,7 +189,8 @@
TUKind(TU_Complete), WantTiming(getenv("LIBCLANG_TIMING")),
OwnsRemappedFileBuffers(true),
NumStoredDiagnosticsFromDriver(0),
- PreambleRebuildCounter(0),
+ PreambleRebuildCountdownCounter(0),
+ PreambleCounter(0),
NumWarningsInPreamble(0),
ShouldCacheCodeCompletionResults(false),
IncludeBriefCommentsInCodeCompletion(false), UserFilesAreVolatile(false),
@@ -1253,21 +1254,21 @@
PreambleInvocationIn.getDiagnosticOpts());
getDiagnostics().setNumWarnings(NumWarningsInPreamble);
- PreambleRebuildCounter = 1;
+ PreambleRebuildCountdownCounter = 1;
return MainFileBuffer;
} else {
Preamble.reset();
PreambleDiagnostics.clear();
TopLevelDeclsInPreamble.clear();
- PreambleRebuildCounter = 1;
+ PreambleRebuildCountdownCounter = 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 (PreambleRebuildCountdownCounter > 1) {
+ --PreambleRebuildCountdownCounter;
return nullptr;
}
@@ -1295,20 +1296,21 @@
PCHContainerOps, /*StoreInMemory=*/false, Callbacks);
if (NewPreamble) {
Preamble = std::move(*NewPreamble);
- PreambleRebuildCounter = 1;
+ ++PreambleCounter;
+ PreambleRebuildCountdownCounter = 1;
} else {
switch (static_cast<BuildPreambleError>(NewPreamble.getError().value())) {
case BuildPreambleError::CouldntCreateTempFile:
case BuildPreambleError::PreambleIsEmpty:
// Try again next time.
- PreambleRebuildCounter = 1;
+ PreambleRebuildCountdownCounter = 1;
return nullptr;
case BuildPreambleError::CouldntCreateTargetInfo:
case BuildPreambleError::BeginSourceFileFailed:
case BuildPreambleError::CouldntEmitPCH:
case BuildPreambleError::CouldntCreateVFSOverlay:
// These erros are more likely to repeat, retry after some period.
- PreambleRebuildCounter = DefaultPreambleRebuildInterval;
+ PreambleRebuildCountdownCounter = DefaultPreambleRebuildInterval;
return nullptr;
}
llvm_unreachable("unexpected BuildPreambleError");
@@ -1450,7 +1452,7 @@
AST->OnlyLocalDecls = OnlyLocalDecls;
AST->CaptureDiagnostics = CaptureDiagnostics;
if (PrecompilePreambleAfterNParses > 0)
- AST->PreambleRebuildCounter = PrecompilePreambleAfterNParses;
+ AST->PreambleRebuildCountdownCounter = PrecompilePreambleAfterNParses;
AST->TUKind = Action ? Action->getTranslationUnitKind() : TU_Complete;
AST->ShouldCacheCodeCompletionResults = CacheCodeCompletionResults;
AST->IncludeBriefCommentsInCodeCompletion
@@ -1584,7 +1586,7 @@
std::unique_ptr<llvm::MemoryBuffer> OverrideMainBuffer;
if (PrecompilePreambleAfterNParses > 0) {
- PreambleRebuildCounter = PrecompilePreambleAfterNParses;
+ PreambleRebuildCountdownCounter = PrecompilePreambleAfterNParses;
OverrideMainBuffer =
getMainBufferWithPrecompiledPreamble(PCHContainerOps, *Invocation, VFS);
getDiagnostics().Reset();
@@ -1763,7 +1765,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 || PreambleRebuildCountdownCounter > 0)
OverrideMainBuffer =
getMainBufferWithPrecompiledPreamble(PCHContainerOps, *Invocation, VFS);
Index: include/clang/Frontend/ASTUnit.h
===================================================================
--- include/clang/Frontend/ASTUnit.h
+++ include/clang/Frontend/ASTUnit.h
@@ -190,7 +190,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;
+ unsigned PreambleRebuildCountdownCounter;
+
+ /// \brief Counter indicating how often the preamble was build in total.
+ unsigned PreambleCounter;
/// \brief Cache pairs "filename - source location"
///
@@ -547,7 +550,11 @@
return SourceRange(mapLocationToPreamble(R.getBegin()),
mapLocationToPreamble(R.getEnd()));
}
-
+
+ unsigned getPreambleCounter() const {
+ return PreambleCounter;
+ }
+
// Retrieve the diagnostics associated with this AST
typedef StoredDiagnostic *stored_diag_iterator;
typedef const StoredDiagnostic *stored_diag_const_iterator;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
