diff --git a/clang/include/clang/Serialization/InMemoryModuleCache.h b/clang/include/clang/Serialization/InMemoryModuleCache.h --- a/clang/include/clang/Serialization/InMemoryModuleCache.h +++ b/clang/include/clang/Serialization/InMemoryModuleCache.h @@ -45,61 +45,35 @@ llvm::StringMap PCMs; public: - /// There are four states for a PCM. It must monotonically increase. - /// - /// 1. Unknown: the PCM has neither been read from disk nor built. - /// 2. Tentative: the PCM has been read from disk but not yet imported or - /// built. It might work. - /// 3. ToBuild: the PCM read from disk did not work but a new one has not - /// been built yet. - /// 4. Final: indicating that the current PCM was either built in this - /// process or has been successfully imported. - enum State { Unknown, Tentative, ToBuild, Final }; - - /// Get the state of the PCM. - State getPCMState(llvm::StringRef Filename) const; - /// Store the PCM under the Filename. /// - /// \pre state is Unknown - /// \post state is Tentative + /// \pre PCM for the same Filename shouldn't be in cache already. /// \return a reference to the buffer as a convenience. llvm::MemoryBuffer &addPCM(llvm::StringRef Filename, std::unique_ptr Buffer); - /// Store a just-built PCM under the Filename. + /// Store a final PCM under the Filename. /// - /// \pre state is Unknown or ToBuild. - /// \pre state is not Tentative. + /// \pre PCM for the same Filename shouldn't be in cache already. /// \return a reference to the buffer as a convenience. - llvm::MemoryBuffer &addBuiltPCM(llvm::StringRef Filename, + llvm::MemoryBuffer &addFinalPCM(llvm::StringRef Filename, std::unique_ptr Buffer); - /// Try to remove a buffer from the cache. No effect if state is Final. + /// Try to remove a PCM from the cache. No effect if it is Final. /// - /// \pre state is Tentative/Final. - /// \post Tentative => ToBuild or Final => Final. - /// \return false on success, i.e. if Tentative => ToBuild. - bool tryToDropPCM(llvm::StringRef Filename); + /// \return false on success. + bool tryToRemovePCM(llvm::StringRef Filename); /// Mark a PCM as final. - /// - /// \pre state is Tentative or Final. - /// \post state is Final. void finalizePCM(llvm::StringRef Filename); - /// Get a pointer to the pCM if it exists; else nullptr. + /// Get a pointer to the PCM if it exists; else nullptr. llvm::MemoryBuffer *lookupPCM(llvm::StringRef Filename) const; /// Check whether the PCM is final and has been shown to work. /// /// \return true iff state is Final. bool isPCMFinal(llvm::StringRef Filename) const; - - /// Check whether the PCM is waiting to be built. - /// - /// \return true iff state is ToBuild. - bool shouldBuildPCM(llvm::StringRef Filename) const; }; } // end namespace clang diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -4502,7 +4502,7 @@ if (ShouldFinalizePCM) MC.finalizePCM(FileName); else - MC.tryToDropPCM(FileName); + MC.tryToRemovePCM(FileName); }); ModuleFile &F = *M; BitstreamCursor &Stream = F.Stream; diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -4304,7 +4304,7 @@ WritingAST = false; if (ShouldCacheASTInMemory) { // Construct MemoryBuffer and update buffer manager. - ModuleCache.addBuiltPCM(OutputFile, + ModuleCache.addFinalPCM(OutputFile, llvm::MemoryBuffer::getMemBufferCopy( StringRef(Buffer.begin(), Buffer.size()))); } diff --git a/clang/lib/Serialization/InMemoryModuleCache.cpp b/clang/lib/Serialization/InMemoryModuleCache.cpp --- a/clang/lib/Serialization/InMemoryModuleCache.cpp +++ b/clang/lib/Serialization/InMemoryModuleCache.cpp @@ -11,16 +11,6 @@ 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 Buffer) { @@ -30,11 +20,11 @@ } llvm::MemoryBuffer & -InMemoryModuleCache::addBuiltPCM(llvm::StringRef Filename, +InMemoryModuleCache::addFinalPCM(llvm::StringRef Filename, std::unique_ptr Buffer) { auto &PCM = PCMs[Filename]; assert(!PCM.IsFinal && "Trying to override finalized PCM?"); - assert(!PCM.Buffer && "Trying to override tentative PCM?"); + assert(!PCM.Buffer && "Already has a non-final PCM"); PCM.Buffer = std::move(Buffer); PCM.IsFinal = true; return *PCM.Buffer; @@ -49,24 +39,21 @@ } bool InMemoryModuleCache::isPCMFinal(llvm::StringRef Filename) const { - return getPCMState(Filename) == Final; -} - -bool InMemoryModuleCache::shouldBuildPCM(llvm::StringRef Filename) const { - return getPCMState(Filename) == ToBuild; + auto I = PCMs.find(Filename); + if (I == PCMs.end()) + return false; + return I->second.IsFinal; } -bool InMemoryModuleCache::tryToDropPCM(llvm::StringRef Filename) { +bool InMemoryModuleCache::tryToRemovePCM(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; - PCM.Buffer.reset(); + PCMs.erase(I); return false; } diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp --- a/clang/lib/Serialization/ModuleManager.cpp +++ b/clang/lib/Serialization/ModuleManager.cpp @@ -163,7 +163,7 @@ // Load the contents of the module if (std::unique_ptr Buffer = lookupBuffer(FileName)) { // The buffer was already provided for us. - NewModule->Buffer = &ModuleCache->addBuiltPCM(FileName, std::move(Buffer)); + NewModule->Buffer = &ModuleCache->addFinalPCM(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,11 +173,6 @@ 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> Buf((std::error_code())); @@ -185,7 +180,7 @@ Buf = llvm::MemoryBuffer::getSTDIN(); } else { // Get a buffer of the file and close the file descriptor when done. - Buf = FileMgr.getBufferForFile(NewModule->File, /*isVolatile=*/false); + Buf = FileMgr.getBufferForFile(NewModule->File, /*isVolatile=*/true); } if (!Buf) { diff --git a/clang/test/Modules/Inputs/implicit-invalidate-chain/A.h b/clang/test/Modules/Inputs/implicit-invalidate-chain/A.h deleted file mode 100644 --- a/clang/test/Modules/Inputs/implicit-invalidate-chain/A.h +++ /dev/null @@ -1,2 +0,0 @@ -// A -#include "B.h" diff --git a/clang/test/Modules/Inputs/implicit-invalidate-chain/B.h b/clang/test/Modules/Inputs/implicit-invalidate-chain/B.h deleted file mode 100644 --- a/clang/test/Modules/Inputs/implicit-invalidate-chain/B.h +++ /dev/null @@ -1,2 +0,0 @@ -// B -#include "C.h" diff --git a/clang/test/Modules/Inputs/implicit-invalidate-chain/C.h b/clang/test/Modules/Inputs/implicit-invalidate-chain/C.h deleted file mode 100644 --- a/clang/test/Modules/Inputs/implicit-invalidate-chain/C.h +++ /dev/null @@ -1,2 +0,0 @@ -// C -#include "D.h" diff --git a/clang/test/Modules/Inputs/implicit-invalidate-chain/module.modulemap b/clang/test/Modules/Inputs/implicit-invalidate-chain/module.modulemap deleted file mode 100644 --- a/clang/test/Modules/Inputs/implicit-invalidate-chain/module.modulemap +++ /dev/null @@ -1,3 +0,0 @@ -module A { header "A.h" } -module B { header "B.h" } -module C { header "C.h" } diff --git a/clang/test/Modules/implicit-invalidate-chain.c b/clang/test/Modules/implicit-invalidate-chain.c deleted file mode 100644 --- a/clang/test/Modules/implicit-invalidate-chain.c +++ /dev/null @@ -1,67 +0,0 @@ -// RUN: rm -rf %t1 %t2 %t-include -// RUN: mkdir %t-include -// RUN: echo 'module D { header "D.h" }' >> %t-include/module.modulemap - -// Run with -verify, which onliy gets remarks from the main TU. -// -// RUN: echo '#define D 0' > %t-include/D.h -// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t1 \ -// RUN: -fdisable-module-hash -fsyntax-only \ -// RUN: -I%S/Inputs/implicit-invalidate-chain -I%t-include \ -// RUN: -Rmodule-build -Rmodule-import %s -// RUN: echo '#define D 11' > %t-include/D.h -// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t1 \ -// RUN: -fdisable-module-hash -fsyntax-only \ -// RUN: -I%S/Inputs/implicit-invalidate-chain -I%t-include \ -// RUN: -Rmodule-build -Rmodule-import -verify %s - -// Run again, using FileCheck to check remarks from the module builds. This is -// the key test: after the first attempt to import an out-of-date 'D', all the -// modules have been invalidated and are not imported again until they are -// rebuilt. -// -// RUN: echo '#define D 0' > %t-include/D.h -// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t2 \ -// RUN: -fdisable-module-hash -fsyntax-only \ -// RUN: -I%S/Inputs/implicit-invalidate-chain -I%t-include \ -// RUN: -Rmodule-build -Rmodule-import %s -// RUN: echo '#define D 11' > %t-include/D.h -// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t2 \ -// RUN: -fdisable-module-hash -fsyntax-only \ -// RUN: -I%S/Inputs/implicit-invalidate-chain -I%t-include \ -// RUN: -Rmodule-build -Rmodule-import %s 2>&1 |\ -// RUN: FileCheck %s -implicit-check-not "remark:" - -#include "A.h" // \ - expected-remark-re{{importing module 'A' from '{{.*[/\\]}}A.pcm'}} \ - expected-remark-re{{importing module 'B' into 'A' from '{{.*[/\\]}}B.pcm'}} \ - expected-remark-re{{importing module 'C' into 'B' from '{{.*[/\\]}}C.pcm'}} \ - expected-remark-re{{importing module 'D' into 'C' from '{{.*[/\\]}}D.pcm'}} \ - expected-remark-re{{building module 'A' as '{{.*[/\\]}}A.pcm'}} \ - expected-remark{{finished building module 'A'}} \ - expected-remark-re{{importing module 'A' from '{{.*[/\\]}}A.pcm'}} \ - expected-remark-re{{importing module 'B' into 'A' from '{{.*[/\\]}}B.pcm'}} \ - expected-remark-re{{importing module 'C' into 'B' from '{{.*[/\\]}}C.pcm'}} \ - expected-remark-re{{importing module 'D' into 'C' from '{{.*[/\\]}}D.pcm'}} -// CHECK: remark: importing module 'A' from '{{.*[/\\]}}A.pcm' -// CHECK: remark: importing module 'B' into 'A' from '{{.*[/\\]}}B.pcm' -// CHECK: remark: importing module 'C' into 'B' from '{{.*[/\\]}}C.pcm' -// CHECK: remark: importing module 'D' into 'C' from '{{.*[/\\]}}D.pcm' -// CHECK: remark: building module 'A' -// CHECK: remark: building module 'B' -// CHECK: remark: building module 'C' -// CHECK: remark: building module 'D' -// CHECK: remark: finished building module 'D' -// CHECK: remark: importing module 'D' from '{{.*[/\\]}}D.pcm' -// CHECK: remark: finished building module 'C' -// CHECK: remark: importing module 'C' from '{{.*[/\\]}}C.pcm' -// CHECK: remark: importing module 'D' into 'C' from '{{.*[/\\]}}D.pcm' -// CHECK: remark: finished building module 'B' -// CHECK: remark: importing module 'B' from '{{.*[/\\]}}B.pcm' -// CHECK: remark: importing module 'C' into 'B' from '{{.*[/\\]}}C.pcm' -// CHECK: remark: importing module 'D' into 'C' from '{{.*[/\\]}}D.pcm' -// CHECK: remark: finished building module 'A' -// CHECK: remark: importing module 'A' from '{{.*[/\\]}}A.pcm' -// CHECK: remark: importing module 'B' into 'A' from '{{.*[/\\]}}B.pcm' -// CHECK: remark: importing module 'C' into 'B' from '{{.*[/\\]}}C.pcm' -// CHECK: remark: importing module 'D' into 'C' from '{{.*[/\\]}}D.pcm' diff --git a/clang/unittests/Frontend/FrontendActionTest.cpp b/clang/unittests/Frontend/FrontendActionTest.cpp --- a/clang/unittests/Frontend/FrontendActionTest.cpp +++ b/clang/unittests/Frontend/FrontendActionTest.cpp @@ -284,11 +284,9 @@ // Check whether the PCH was cached. if (ShouldCache) - EXPECT_EQ(InMemoryModuleCache::Final, - Compiler.getModuleCache().getPCMState(PCHFilename)); + EXPECT_TRUE(Compiler.getModuleCache().isPCMFinal(PCHFilename)); else - EXPECT_EQ(InMemoryModuleCache::Unknown, - Compiler.getModuleCache().getPCMState(PCHFilename)); + EXPECT_EQ(nullptr, Compiler.getModuleCache().lookupPCM(PCHFilename)); } } diff --git a/clang/unittests/Serialization/InMemoryModuleCacheTest.cpp b/clang/unittests/Serialization/InMemoryModuleCacheTest.cpp --- a/clang/unittests/Serialization/InMemoryModuleCacheTest.cpp +++ b/clang/unittests/Serialization/InMemoryModuleCacheTest.cpp @@ -24,12 +24,11 @@ TEST(InMemoryModuleCacheTest, initialState) { InMemoryModuleCache Cache; - EXPECT_EQ(InMemoryModuleCache::Unknown, Cache.getPCMState("B")); + EXPECT_EQ(nullptr, Cache.lookupPCM("B")); EXPECT_FALSE(Cache.isPCMFinal("B")); - EXPECT_FALSE(Cache.shouldBuildPCM("B")); #if !defined(NDEBUG) && GTEST_HAS_DEATH_TEST - EXPECT_DEATH(Cache.tryToDropPCM("B"), "PCM to remove is unknown"); + EXPECT_DEATH(Cache.tryToRemovePCM("B"), "PCM to remove is unknown"); EXPECT_DEATH(Cache.finalizePCM("B"), "PCM to finalize is unknown"); #endif } @@ -40,37 +39,33 @@ InMemoryModuleCache Cache; EXPECT_EQ(RawB, &Cache.addPCM("B", std::move(B))); - EXPECT_EQ(InMemoryModuleCache::Tentative, Cache.getPCMState("B")); EXPECT_EQ(RawB, Cache.lookupPCM("B")); EXPECT_FALSE(Cache.isPCMFinal("B")); - EXPECT_FALSE(Cache.shouldBuildPCM("B")); #if !defined(NDEBUG) && GTEST_HAS_DEATH_TEST EXPECT_DEATH(Cache.addPCM("B", getBuffer(2)), "Already has a PCM"); - EXPECT_DEATH(Cache.addBuiltPCM("B", getBuffer(2)), - "Trying to override tentative PCM"); + EXPECT_DEATH(Cache.addFinalPCM("B", getBuffer(2)), + "Already has a non-final PCM"); #endif } -TEST(InMemoryModuleCacheTest, addBuiltPCM) { +TEST(InMemoryModuleCacheTest, addFinalPCM) { auto B = getBuffer(1); auto *RawB = B.get(); InMemoryModuleCache Cache; - EXPECT_EQ(RawB, &Cache.addBuiltPCM("B", std::move(B))); - EXPECT_EQ(InMemoryModuleCache::Final, Cache.getPCMState("B")); + EXPECT_EQ(RawB, &Cache.addFinalPCM("B", std::move(B))); EXPECT_EQ(RawB, Cache.lookupPCM("B")); EXPECT_TRUE(Cache.isPCMFinal("B")); - EXPECT_FALSE(Cache.shouldBuildPCM("B")); #if !defined(NDEBUG) && GTEST_HAS_DEATH_TEST EXPECT_DEATH(Cache.addPCM("B", getBuffer(2)), "Already has a PCM"); - EXPECT_DEATH(Cache.addBuiltPCM("B", getBuffer(2)), + EXPECT_DEATH(Cache.addFinalPCM("B", getBuffer(2)), "Trying to override finalized PCM"); #endif } -TEST(InMemoryModuleCacheTest, tryToDropPCM) { +TEST(InMemoryModuleCacheTest, tryToRemovePCM) { auto B1 = getBuffer(1); auto B2 = getBuffer(2); auto *RawB1 = B1.get(); @@ -78,27 +73,22 @@ ASSERT_NE(RawB1, RawB2); InMemoryModuleCache Cache; - EXPECT_EQ(InMemoryModuleCache::Unknown, Cache.getPCMState("B")); EXPECT_EQ(RawB1, &Cache.addPCM("B", std::move(B1))); - EXPECT_FALSE(Cache.tryToDropPCM("B")); + EXPECT_FALSE(Cache.tryToRemovePCM("B")); EXPECT_EQ(nullptr, Cache.lookupPCM("B")); - EXPECT_EQ(InMemoryModuleCache::ToBuild, Cache.getPCMState("B")); EXPECT_FALSE(Cache.isPCMFinal("B")); - EXPECT_TRUE(Cache.shouldBuildPCM("B")); #if !defined(NDEBUG) && GTEST_HAS_DEATH_TEST - EXPECT_DEATH(Cache.addPCM("B", getBuffer(2)), "Already has a PCM"); - EXPECT_DEATH(Cache.tryToDropPCM("B"), - "PCM to remove is scheduled to be built"); - EXPECT_DEATH(Cache.finalizePCM("B"), "Trying to finalize a dropped PCM"); + EXPECT_DEATH(Cache.tryToRemovePCM("B"), "PCM to remove is unknown"); + EXPECT_DEATH(Cache.finalizePCM("B"), "PCM to finalize is unknown"); #endif // Add a new one. - EXPECT_EQ(RawB2, &Cache.addBuiltPCM("B", std::move(B2))); + EXPECT_EQ(RawB2, &Cache.addFinalPCM("B", std::move(B2))); EXPECT_TRUE(Cache.isPCMFinal("B")); // Can try to drop again, but this should error and do nothing. - EXPECT_TRUE(Cache.tryToDropPCM("B")); + EXPECT_TRUE(Cache.tryToRemovePCM("B")); EXPECT_EQ(RawB2, Cache.lookupPCM("B")); } @@ -107,12 +97,10 @@ auto *RawB = B.get(); InMemoryModuleCache Cache; - EXPECT_EQ(InMemoryModuleCache::Unknown, Cache.getPCMState("B")); EXPECT_EQ(RawB, &Cache.addPCM("B", std::move(B))); // Call finalize. Cache.finalizePCM("B"); - EXPECT_EQ(InMemoryModuleCache::Final, Cache.getPCMState("B")); EXPECT_TRUE(Cache.isPCMFinal("B")); }