r211330 - Avoid invalidating successfully loaded module files
Ben Langmuir
blangmuir at apple.com
Thu Jun 19 17:24:56 PDT 2014
Author: benlangmuir
Date: Thu Jun 19 19:24:56 2014
New Revision: 211330
URL: http://llvm.org/viewvc/llvm-project?rev=211330&view=rev
Log:
Avoid invalidating successfully loaded module files
Successfully loaded module files may be referenced in other
ModuleManagers, so don't invalidate them. Two related things are fixed:
1) I thought the last module in the manager was always the one that
failed, but it isn't. So check explicitly against the list of
vetted modules from ReadASTCore.
2) We now keep the file descriptor of pcm file open, which avoids the
possibility of having two different pcms for the same module loaded when
building in parallel with headers being modified during a build.
<rdar://problem/16835846>
Modified:
cfe/trunk/include/clang/Basic/FileManager.h
cfe/trunk/include/clang/Serialization/ModuleManager.h
cfe/trunk/lib/Basic/FileManager.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ModuleManager.cpp
Modified: cfe/trunk/include/clang/Basic/FileManager.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileManager.h?rev=211330&r1=211329&r2=211330&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/FileManager.h (original)
+++ cfe/trunk/include/clang/Basic/FileManager.h Thu Jun 19 19:24:56 2014
@@ -242,7 +242,8 @@ public:
/// MemoryBuffer if successful, otherwise returning null.
llvm::MemoryBuffer *getBufferForFile(const FileEntry *Entry,
std::string *ErrorStr = nullptr,
- bool isVolatile = false);
+ bool isVolatile = false,
+ bool ShouldCloseOpenFile = true);
llvm::MemoryBuffer *getBufferForFile(StringRef Filename,
std::string *ErrorStr = nullptr);
Modified: cfe/trunk/include/clang/Serialization/ModuleManager.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ModuleManager.h?rev=211330&r1=211329&r2=211330&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ModuleManager.h (original)
+++ cfe/trunk/include/clang/Serialization/ModuleManager.h Thu Jun 19 19:24:56 2014
@@ -18,6 +18,7 @@
#include "clang/Basic/FileManager.h"
#include "clang/Serialization/Module.h"
#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallPtrSet.h"
namespace clang {
@@ -194,6 +195,7 @@ public:
/// \brief Remove the given set of modules.
void removeModules(ModuleIterator first, ModuleIterator last,
+ llvm::SmallPtrSetImpl<ModuleFile *> &LoadedSuccessfully,
ModuleMap *modMap);
/// \brief Add an in-memory buffer the list of known buffers
Modified: cfe/trunk/lib/Basic/FileManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=211330&r1=211329&r2=211330&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/FileManager.cpp (original)
+++ cfe/trunk/lib/Basic/FileManager.cpp Thu Jun 19 19:24:56 2014
@@ -388,7 +388,7 @@ void FileManager::FixupRelativePath(Smal
llvm::MemoryBuffer *FileManager::
getBufferForFile(const FileEntry *Entry, std::string *ErrorStr,
- bool isVolatile) {
+ bool isVolatile, bool ShouldCloseOpenFile) {
std::unique_ptr<llvm::MemoryBuffer> Result;
std::error_code ec;
@@ -405,7 +405,10 @@ getBufferForFile(const FileEntry *Entry,
/*RequiresNullTerminator=*/true, isVolatile);
if (ErrorStr)
*ErrorStr = ec.message();
- Entry->closeFile();
+ // FIXME: we need a set of APIs that can make guarantees about whether a
+ // FileEntry is open or not.
+ if (ShouldCloseOpenFile)
+ Entry->closeFile();
return Result.release();
}
Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=211330&r1=211329&r2=211330&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Thu Jun 19 19:24:56 2014
@@ -3464,8 +3464,13 @@ ASTReader::ASTReadResult ASTReader::Read
case OutOfDate:
case VersionMismatch:
case ConfigurationMismatch:
- case HadErrors:
+ case HadErrors: {
+ llvm::SmallPtrSet<ModuleFile *, 4> LoadedSet;
+ for (const ImportedModule &IM : Loaded)
+ LoadedSet.insert(IM.Mod);
+
ModuleMgr.removeModules(ModuleMgr.begin() + NumModules, ModuleMgr.end(),
+ LoadedSet,
Context.getLangOpts().Modules
? &PP.getHeaderSearchInfo().getModuleMap()
: nullptr);
@@ -3475,7 +3480,7 @@ ASTReader::ASTReadResult ASTReader::Read
GlobalIndex.reset();
ModuleMgr.setGlobalIndex(nullptr);
return ReadResult;
-
+ }
case Success:
break;
}
Modified: cfe/trunk/lib/Serialization/ModuleManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ModuleManager.cpp?rev=211330&r1=211329&r2=211330&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ModuleManager.cpp (original)
+++ cfe/trunk/lib/Serialization/ModuleManager.cpp Thu Jun 19 19:24:56 2014
@@ -109,8 +109,15 @@ ModuleManager::addModule(StringRef FileN
ec = llvm::MemoryBuffer::getSTDIN(New->Buffer);
if (ec)
ErrorStr = ec.message();
- } else
- New->Buffer.reset(FileMgr.getBufferForFile(FileName, &ErrorStr));
+ } else {
+ // Leave the FileEntry open so if it gets read again by another
+ // ModuleManager it must be the same underlying file.
+ // FIXME: Because FileManager::getFile() doesn't guarantee that it will
+ // give us an open file, this may not be 100% reliable.
+ New->Buffer.reset(FileMgr.getBufferForFile(New->File, &ErrorStr,
+ /*IsVolatile*/false,
+ /*ShouldClose*/false));
+ }
if (!New->Buffer)
return Missing;
@@ -135,31 +142,16 @@ ModuleManager::addModule(StringRef FileN
return NewModule? NewlyLoaded : AlreadyLoaded;
}
-static void getModuleFileAncestors(
- ModuleFile *F,
- llvm::SmallPtrSetImpl<ModuleFile *> &Ancestors) {
- Ancestors.insert(F);
- for (ModuleFile *Importer : F->ImportedBy)
- getModuleFileAncestors(Importer, Ancestors);
-}
-
-void ModuleManager::removeModules(ModuleIterator first, ModuleIterator last,
- ModuleMap *modMap) {
+void ModuleManager::removeModules(
+ ModuleIterator first, ModuleIterator last,
+ llvm::SmallPtrSetImpl<ModuleFile *> &LoadedSuccessfully,
+ ModuleMap *modMap) {
if (first == last)
return;
// Collect the set of module file pointers that we'll be removing.
llvm::SmallPtrSet<ModuleFile *, 4> victimSet(first, last);
- // The last module file caused the load failure, so it and its ancestors in
- // the module dependency tree will be rebuilt (or there was an error), so
- // there should be no references to them. Collect the files to remove from
- // the cache below, since rebuilding them will create new files at the old
- // locations.
- llvm::SmallPtrSet<ModuleFile *, 4> Ancestors;
- getModuleFileAncestors(*(last-1), Ancestors);
- assert(Ancestors.count(*first) && "non-dependent module loaded");
-
// Remove any references to the now-destroyed modules.
for (unsigned i = 0, n = Chain.size(); i != n; ++i) {
Chain[i]->ImportedBy.remove_if([&](ModuleFile *MF) {
@@ -178,7 +170,10 @@ void ModuleManager::removeModules(Module
}
}
- if (Ancestors.count(*victim))
+ // Files that didn't make it through ReadASTCore successfully will be
+ // rebuilt (or there was an error). Invalidate them so that we can load the
+ // new files that will be renamed over the old ones.
+ if (LoadedSuccessfully.count(*victim) == 0)
FileMgr.invalidateCache((*victim)->File);
delete *victim;
More information about the cfe-commits
mailing list