Revert "[modules] Do not cache invalid state for modules that we attempted to load."

As per comment on https://reviews.llvm.org/D72860, it is suggested to
revert this change in the meantime, since it has introduced regression.

This reverts commit 83f4c3af021cd5322ea10fd1c4e839874c1dae49.

4 files changed, 30 insertions, 14 deletions

diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index b5ca1e1..63c817b 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -4502,7 +4502,7 @@ ASTReader::ReadASTCore(StringRef FileName,
     if (ShouldFinalizePCM)
       MC.finalizePCM(FileName);
     else
-      MC.tryToRemovePCM(FileName);
+      MC.tryToDropPCM(FileName);
   });
   ModuleFile &F = *M;
   BitstreamCursor &Stream = F.Stream;
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index bf893c7..1278841 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -4317,7 +4317,7 @@ ASTFileSignature ASTWriter::WriteAST(Sema &SemaRef,
   WritingAST = false;
   if (ShouldCacheASTInMemory) {
     // Construct MemoryBuffer and update buffer manager.
-    ModuleCache.addFinalPCM(OutputFile,
+    ModuleCache.addBuiltPCM(OutputFile,
                             llvm::MemoryBuffer::getMemBufferCopy(
                                 StringRef(Buffer.begin(), Buffer.size())));
   }
diff --git a/clang/lib/Serialization/InMemoryModuleCache.cpp b/clang/lib/Serialization/InMemoryModuleCache.cpp
index 68d9411..d35fa2a 100644
--- a/clang/lib/Serialization/InMemoryModuleCache.cpp
+++ b/clang/lib/Serialization/InMemoryModuleCache.cpp
@@ -11,6 +11,16 @@
 
 using namespace clang;
 
+InMemoryModuleCache::State
+InMemoryModuleCache::getPCMState(llvm::StringRef Filename) const {
+  auto I = PCMs.find(Filename);
+  if (I == PCMs.end())
+    return Unknown;
+  if (I->second.IsFinal)
+    return Final;
+  return I->second.Buffer ? Tentative : ToBuild;
+}
+
 llvm::MemoryBuffer &
 InMemoryModuleCache::addPCM(llvm::StringRef Filename,
                             std::unique_ptr<llvm::MemoryBuffer> Buffer) {
@@ -20,11 +30,11 @@ InMemoryModuleCache::addPCM(llvm::StringRef Filename,
 }
 
 llvm::MemoryBuffer &
-InMemoryModuleCache::addFinalPCM(llvm::StringRef Filename,
+InMemoryModuleCache::addBuiltPCM(llvm::StringRef Filename,
                                  std::unique_ptr<llvm::MemoryBuffer> Buffer) {
   auto &PCM = PCMs[Filename];
   assert(!PCM.IsFinal && "Trying to override finalized PCM?");
-  assert(!PCM.Buffer && "Already has a non-final PCM");
+  assert(!PCM.Buffer && "Trying to override tentative PCM?");
   PCM.Buffer = std::move(Buffer);
   PCM.IsFinal = true;
   return *PCM.Buffer;
@@ -39,21 +49,24 @@ InMemoryModuleCache::lookupPCM(llvm::StringRef Filename) const {
 }
 
 bool InMemoryModuleCache::isPCMFinal(llvm::StringRef Filename) const {
-  auto I = PCMs.find(Filename);
-  if (I == PCMs.end())
-    return false;
-  return I->second.IsFinal;
+  return getPCMState(Filename) == Final;
+}
+
+bool InMemoryModuleCache::shouldBuildPCM(llvm::StringRef Filename) const {
+  return getPCMState(Filename) == ToBuild;
 }
 
-bool InMemoryModuleCache::tryToRemovePCM(llvm::StringRef Filename) {
+bool InMemoryModuleCache::tryToDropPCM(llvm::StringRef Filename) {
   auto I = PCMs.find(Filename);
   assert(I != PCMs.end() && "PCM to remove is unknown...");
 
   auto &PCM = I->second;
+  assert(PCM.Buffer && "PCM to remove is scheduled to be built...");
+
   if (PCM.IsFinal)
     return true;
 
-  PCMs.erase(I);
+  PCM.Buffer.reset();
   return false;
 }
 
diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp
index 9b326d2..2656220 100644
--- a/clang/lib/Serialization/ModuleManager.cpp
+++ b/clang/lib/Serialization/ModuleManager.cpp
@@ -163,7 +163,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type,
   // Load the contents of the module
   if (std::unique_ptr<llvm::MemoryBuffer> Buffer = lookupBuffer(FileName)) {
     // The buffer was already provided for us.
-    NewModule->Buffer = &ModuleCache->addFinalPCM(FileName, std::move(Buffer));
+    NewModule->Buffer = &ModuleCache->addBuiltPCM(FileName, std::move(Buffer));
     // Since the cached buffer is reused, it is safe to close the file
     // descriptor that was opened while stat()ing the PCM in
     // lookupModuleFile() above, it won't be needed any longer.
@@ -173,6 +173,11 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type,
     NewModule->Buffer = Buffer;
     // As above, the file descriptor is no longer needed.
     Entry->closeFile();
+  } else if (getModuleCache().shouldBuildPCM(FileName)) {
+    // Report that the module is out of date, since we tried (and failed) to
+    // import it earlier.
+    Entry->closeFile();
+    return OutOfDate;
   } else {
     // Open the AST file.
     llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Buf((std::error_code()));
@@ -180,9 +185,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type,
       Buf = llvm::MemoryBuffer::getSTDIN();
     } else {
       // Get a buffer of the file and close the file descriptor when done.
-      // The file is volatile because in a parallel build we expect multiple
-      // compiler processes to use the same module file rebuilding it if needed.
-      Buf = FileMgr.getBufferForFile(NewModule->File, /*isVolatile=*/true);
+      Buf = FileMgr.getBufferForFile(NewModule->File, /*isVolatile=*/false);
     }
 
     if (!Buf) {