From 0a2be46cfdb698fefcc860a56b47dde0884d5335 Mon Sep 17 00:00:00 2001 From: "Duncan P. N. Exon Smith" Date: Sat, 9 Mar 2019 17:44:01 +0000 Subject: [PATCH] Modules: Invalidate out-of-date PCMs as they're discovered Leverage the InMemoryModuleCache to invalidate a module the first time it fails to import (and to lock a module as soon as it's built or imported successfully). For implicit module builds, this optimizes importing deep graphs where the leaf module is out-of-date; see example near the end of the commit message. Previously the cache finalized ("locked in") all modules imported so far when starting a new module build. This was sufficient to prevent loading two versions of the same module, but was somewhat arbitrary and hard to reason about. Now the cache explicitly tracks module state, where each module must be one of: - Unknown: module not in the cache (yet). - Tentative: module in the cache, but not yet fully imported. - ToBuild: module found on disk could not be imported; need to build. - Final: module in the cache has been successfully built or imported. Preventing repeated failed imports avoids variation in builds based on shifting filesystem state. Now it's guaranteed that a module is loaded from disk exactly once. It now seems safe to remove FileManager::invalidateCache, but I'm leaving that for a later commit. The new, precise logic uncovered a pre-existing problem in the cache: the map key is the module filename, and different contexts use different filenames for the same PCM file. (In particular, the test Modules/relative-import-path.c does not build without this commit. r223577 started using a relative path to describe a module's base directory when importing it within another module. As a result, the module cache sees an absolute path when (a) building the module or importing it at the top-level, and a relative path when (b) importing the module underneath another one.) The "obvious" fix is to resolve paths using FileManager::getVirtualFile and change the map key for the cache to a FileEntry, but some contexts (particularly related to ASTUnit) have a shorter lifetime for their FileManager than the InMemoryModuleCache. This is worth pursuing further in a later commit; perhaps by tying together the FileManager and InMemoryModuleCache lifetime, or moving the in-memory PCM storage into a VFS layer. For now, use the PCM's base directory as-written for constructing the filename to check the ModuleCache. Example ======= To understand the build optimization, first consider the build of a module graph TU -> A -> B -> C -> D with an empty cache: TU builds A' A' builds B' B' builds C' C' builds D' imports D' B' imports C' imports D' A' imports B' imports C' imports D' TU imports A' imports B' imports C' imports D' If we build TU again, where A, B, C, and D are in the cache and D is out-of-date, we would previously get this build: TU imports A imports B imports C imports D (out-of-date) TU builds A' A' imports B imports C imports D (out-of-date) builds B' B' imports C imports D (out-of-date) builds C' C' imports D (out-of-date) builds D' imports D' B' imports C' imports D' A' imports B' imports C' imports D' TU imports A' imports B' imports C' imports D' After this commit, we'll immediateley invalidate A, B, C, and D when we first observe that D is out-of-date, giving this build: TU imports A imports B imports C imports D (out-of-date) TU builds A' // The same graph as an empty cache. A' builds B' B' builds C' C' builds D' imports D' B' imports C' imports D' A' imports B' imports C' imports D' TU imports A' imports B' imports C' imports D' The new build matches what we'd naively expect, pretty closely matching the original build with the empty cache. rdar://problem/48545366 llvm-svn: 355778 --- clang/include/clang/Serialization/ASTReader.h | 4 + .../clang/Serialization/InMemoryModuleCache.h | 83 +++++++---- clang/lib/Frontend/CompilerInstance.cpp | 6 +- clang/lib/Serialization/ASTReader.cpp | 28 +++- clang/lib/Serialization/ASTWriter.cpp | 6 +- clang/lib/Serialization/InMemoryModuleCache.cpp | 63 ++++++--- clang/lib/Serialization/ModuleManager.cpp | 26 ++-- .../Modules/Inputs/implicit-invalidate-chain/A.h | 2 + .../Modules/Inputs/implicit-invalidate-chain/B.h | 2 + .../Modules/Inputs/implicit-invalidate-chain/C.h | 2 + .../implicit-invalidate-chain/module.modulemap | 3 + clang/test/Modules/Inputs/relative-import-path/A.h | 2 + clang/test/Modules/Inputs/relative-import-path/B.h | 2 + clang/test/Modules/Inputs/relative-import-path/C.h | 1 + .../Inputs/relative-import-path/module.modulemap | 3 + clang/test/Modules/implicit-invalidate-chain.c | 67 +++++++++ clang/test/Modules/relative-import-path.c | 26 ++++ .../Serialization/InMemoryModuleCacheTest.cpp | 151 ++++++++++++--------- 18 files changed, 345 insertions(+), 132 deletions(-) create mode 100644 clang/test/Modules/Inputs/implicit-invalidate-chain/A.h create mode 100644 clang/test/Modules/Inputs/implicit-invalidate-chain/B.h create mode 100644 clang/test/Modules/Inputs/implicit-invalidate-chain/C.h create mode 100644 clang/test/Modules/Inputs/implicit-invalidate-chain/module.modulemap create mode 100644 clang/test/Modules/Inputs/relative-import-path/A.h create mode 100644 clang/test/Modules/Inputs/relative-import-path/B.h create mode 100644 clang/test/Modules/Inputs/relative-import-path/C.h create mode 100644 clang/test/Modules/Inputs/relative-import-path/module.modulemap create mode 100644 clang/test/Modules/implicit-invalidate-chain.c create mode 100644 clang/test/Modules/relative-import-path.c diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index 58a3b65..423313e 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -2231,6 +2231,10 @@ public: // Read a path std::string ReadPath(ModuleFile &F, const RecordData &Record, unsigned &Idx); + // Read a path + std::string ReadPath(StringRef BaseDirectory, const RecordData &Record, + unsigned &Idx); + // Skip a path static void SkipPath(const RecordData &Record, unsigned &Idx) { SkipString(Record, Idx); diff --git a/clang/include/clang/Serialization/InMemoryModuleCache.h b/clang/include/clang/Serialization/InMemoryModuleCache.h index aea4a8d..6d31e46b 100644 --- a/clang/include/clang/Serialization/InMemoryModuleCache.h +++ b/clang/include/clang/Serialization/InMemoryModuleCache.h @@ -30,54 +30,79 @@ namespace clang { /// Critically, it ensures that a single process has a consistent view of each /// PCM. This is used by \a CompilerInstance when building PCMs to ensure that /// each \a ModuleManager sees the same files. -/// -/// \a finalizeCurrentBuffers() should be called before creating a new user. -/// This locks in the current PCMs, ensuring that no PCM that has already been -/// accessed can be purged, preventing use-after-frees. class InMemoryModuleCache : public llvm::RefCountedBase { struct PCM { std::unique_ptr Buffer; - /// Track the timeline of when this was added to the cache. - unsigned Index; + /// Track whether this PCM is known to be good (either built or + /// successfully imported by a CompilerInstance/ASTReader using this + /// cache). + bool IsFinal = false; + + PCM() = default; + PCM(std::unique_ptr Buffer) + : Buffer(std::move(Buffer)) {} }; /// Cache of buffers. llvm::StringMap PCMs; - /// Monotonically increasing index. - unsigned NextIndex = 0; - - /// Bumped to prevent "older" buffers from being removed. - unsigned FirstRemovableIndex = 0; - public: - /// Store the Buffer under the Filename. + /// 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 There is not already buffer is not already in the cache. + /// \pre state is Unknown + /// \post state is Tentative /// \return a reference to the buffer as a convenience. - llvm::MemoryBuffer &addBuffer(llvm::StringRef Filename, - std::unique_ptr Buffer); + llvm::MemoryBuffer &addPCM(llvm::StringRef Filename, + std::unique_ptr Buffer); + + /// Store a just-built PCM under the Filename. + /// + /// \pre state is Unknown or ToBuild. + /// \pre state is not Tentative. + /// \return a reference to the buffer as a convenience. + llvm::MemoryBuffer &addBuiltPCM(llvm::StringRef Filename, + std::unique_ptr Buffer); + + /// Try to remove a buffer from the cache. No effect if state 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); - /// Try to remove a buffer from the cache. + /// Mark a PCM as final. /// - /// \return false on success, iff \c !isBufferFinal(). - bool tryToRemoveBuffer(llvm::StringRef Filename); + /// \pre state is Tentative or Final. + /// \post state is Final. + void finalizePCM(llvm::StringRef Filename); - /// Get a pointer to the buffer if it exists; else nullptr. - llvm::MemoryBuffer *lookupBuffer(llvm::StringRef Filename); + /// Get a pointer to the pCM if it exists; else nullptr. + llvm::MemoryBuffer *lookupPCM(llvm::StringRef Filename) const; - /// Check whether the buffer is final. + /// Check whether the PCM is final and has been shown to work. /// - /// \return true iff \a finalizeCurrentBuffers() has been called since the - /// buffer was added. This prevents buffers from being removed. - bool isBufferFinal(llvm::StringRef Filename); + /// \return true iff state is Final. + bool isPCMFinal(llvm::StringRef Filename) const; - /// Finalize the current buffers in the cache. + /// Check whether the PCM is waiting to be built. /// - /// Should be called when creating a new user to ensure previous uses aren't - /// invalidated. - void finalizeCurrentBuffers(); + /// \return true iff state is ToBuild. + bool shouldBuildPCM(llvm::StringRef Filename) const; }; } // end namespace clang diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 8fa1393..e738596 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -62,11 +62,7 @@ CompilerInstance::CompilerInstance( Invocation(new CompilerInvocation()), ModuleCache(SharedModuleCache ? SharedModuleCache : new InMemoryModuleCache), - ThePCHContainerOperations(std::move(PCHContainerOps)) { - // Don't allow this to invalidate buffers in use by others. - if (SharedModuleCache) - getModuleCache().finalizeCurrentBuffers(); -} + ThePCHContainerOperations(std::move(PCHContainerOps)) {} CompilerInstance::~CompilerInstance() { assert(OutputFiles.empty() && "Still output files in flight?"); diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index b8b390a..1ab6a75 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -92,6 +92,7 @@ #include "llvm/ADT/None.h" #include "llvm/ADT/Optional.h" #include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/SmallVector.h" @@ -2359,6 +2360,7 @@ ASTReader::ReadControlBlock(ModuleFile &F, RecordData Record; unsigned NumInputs = 0; unsigned NumUserInputs = 0; + StringRef BaseDirectoryAsWritten; while (true) { llvm::BitstreamEntry Entry = Stream.advance(); @@ -2559,7 +2561,9 @@ ASTReader::ReadControlBlock(ModuleFile &F, ImportedName, /*FileMapOnly*/ true); if (ImportedFile.empty()) - ImportedFile = ReadPath(F, Record, Idx); + // Use BaseDirectoryAsWritten to ensure we use the same path in the + // ModuleCache as when writing. + ImportedFile = ReadPath(BaseDirectoryAsWritten, Record, Idx); else SkipPath(Record, Idx); @@ -2624,6 +2628,9 @@ ASTReader::ReadControlBlock(ModuleFile &F, break; case MODULE_DIRECTORY: { + // Save the BaseDirectory as written in the PCM for computing the module + // filename for the ModuleCache. + BaseDirectoryAsWritten = Blob; assert(!F.ModuleName.empty() && "MODULE_DIRECTORY found before MODULE_NAME"); // If we've already loaded a module map file covering this module, we may @@ -4180,6 +4187,14 @@ ASTReader::ReadASTCore(StringRef FileName, assert(M && "Missing module file"); + bool ShouldFinalizePCM = false; + auto FinalizeOrDropPCM = llvm::make_scope_exit([&]() { + auto &MC = getModuleManager().getModuleCache(); + if (ShouldFinalizePCM) + MC.finalizePCM(FileName); + else + MC.tryToDropPCM(FileName); + }); ModuleFile &F = *M; BitstreamCursor &Stream = F.Stream; Stream = BitstreamCursor(PCHContainerRdr.ExtractPCH(*F.Buffer)); @@ -4246,6 +4261,7 @@ ASTReader::ReadASTCore(StringRef FileName, // Record that we've loaded this module. Loaded.push_back(ImportedModule(M, ImportedBy, ImportLoc)); + ShouldFinalizePCM = true; return Success; case UNHASHED_CONTROL_BLOCK_ID: @@ -4309,7 +4325,7 @@ ASTReader::readUnhashedControlBlock(ModuleFile &F, bool WasImportedBy, // validation will fail during the as-system import since the PCM on disk // doesn't guarantee that -Werror was respected. However, the -Werror // flags were checked during the initial as-user import. - if (getModuleManager().getModuleCache().isBufferFinal(F.FileName)) { + if (getModuleManager().getModuleCache().isPCMFinal(F.FileName)) { Diag(diag::warn_module_system_bit_conflict) << F.FileName; return Success; } @@ -9099,6 +9115,14 @@ std::string ASTReader::ReadPath(ModuleFile &F, const RecordData &Record, return Filename; } +std::string ASTReader::ReadPath(StringRef BaseDirectory, + const RecordData &Record, unsigned &Idx) { + std::string Filename = ReadString(Record, Idx); + if (!BaseDirectory.empty()) + ResolveImportedPath(Filename, BaseDirectory); + return Filename; +} + VersionTuple ASTReader::ReadVersionTuple(const RecordData &Record, unsigned &Idx) { unsigned Major = Record[Idx++]; diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index a385eec..85bc281 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -4622,9 +4622,9 @@ ASTFileSignature ASTWriter::WriteAST(Sema &SemaRef, WritingAST = false; if (SemaRef.Context.getLangOpts().ImplicitModules && WritingModule) { // Construct MemoryBuffer and update buffer manager. - ModuleCache.addBuffer(OutputFile, - llvm::MemoryBuffer::getMemBufferCopy( - StringRef(Buffer.begin(), Buffer.size()))); + ModuleCache.addBuiltPCM(OutputFile, + llvm::MemoryBuffer::getMemBufferCopy( + StringRef(Buffer.begin(), Buffer.size()))); } return Signature; } diff --git a/clang/lib/Serialization/InMemoryModuleCache.cpp b/clang/lib/Serialization/InMemoryModuleCache.cpp index 130ece0..d35fa2a 100644 --- a/clang/lib/Serialization/InMemoryModuleCache.cpp +++ b/clang/lib/Serialization/InMemoryModuleCache.cpp @@ -11,39 +11,70 @@ 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::addBuffer(llvm::StringRef Filename, - std::unique_ptr Buffer) { - auto Insertion = PCMs.insert({Filename, PCM{std::move(Buffer), NextIndex++}}); - assert(Insertion.second && "Already has a buffer"); +InMemoryModuleCache::addPCM(llvm::StringRef Filename, + std::unique_ptr Buffer) { + auto Insertion = PCMs.insert(std::make_pair(Filename, std::move(Buffer))); + assert(Insertion.second && "Already has a PCM"); return *Insertion.first->second.Buffer; } +llvm::MemoryBuffer & +InMemoryModuleCache::addBuiltPCM(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?"); + PCM.Buffer = std::move(Buffer); + PCM.IsFinal = true; + return *PCM.Buffer; +} + llvm::MemoryBuffer * -InMemoryModuleCache::lookupBuffer(llvm::StringRef Filename) { +InMemoryModuleCache::lookupPCM(llvm::StringRef Filename) const { auto I = PCMs.find(Filename); if (I == PCMs.end()) return nullptr; return I->second.Buffer.get(); } -bool InMemoryModuleCache::isBufferFinal(llvm::StringRef Filename) { - auto I = PCMs.find(Filename); - if (I == PCMs.end()) - return false; - return I->second.Index < FirstRemovableIndex; +bool InMemoryModuleCache::isPCMFinal(llvm::StringRef Filename) const { + return getPCMState(Filename) == Final; +} + +bool InMemoryModuleCache::shouldBuildPCM(llvm::StringRef Filename) const { + return getPCMState(Filename) == ToBuild; } -bool InMemoryModuleCache::tryToRemoveBuffer(llvm::StringRef Filename) { +bool InMemoryModuleCache::tryToDropPCM(llvm::StringRef Filename) { auto I = PCMs.find(Filename); - assert(I != PCMs.end() && "No buffer to remove..."); - if (I->second.Index < FirstRemovableIndex) + 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; } -void InMemoryModuleCache::finalizeCurrentBuffers() { - FirstRemovableIndex = NextIndex; +void InMemoryModuleCache::finalizePCM(llvm::StringRef Filename) { + auto I = PCMs.find(Filename); + assert(I != PCMs.end() && "PCM to finalize is unknown..."); + + auto &PCM = I->second; + assert(PCM.Buffer && "Trying to finalize a dropped PCM..."); + PCM.IsFinal = true; } diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp index 173fadc..eb8e10a 100644 --- a/clang/lib/Serialization/ModuleManager.cpp +++ b/clang/lib/Serialization/ModuleManager.cpp @@ -118,6 +118,8 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, // contents, but we can't check that.) ExpectedModTime = 0; } + // Note: ExpectedSize and ExpectedModTime will be 0 for MK_ImplicitModule + // when using an ASTFileSignature. if (lookupModuleFile(FileName, ExpectedSize, ExpectedModTime, Entry)) { ErrorStr = "module file out of date"; return OutOfDate; @@ -159,16 +161,21 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, // Load the contents of the module if (std::unique_ptr Buffer = lookupBuffer(FileName)) { // The buffer was already provided for us. - NewModule->Buffer = &ModuleCache->addBuffer(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. Entry->closeFile(); } else if (llvm::MemoryBuffer *Buffer = - getModuleCache().lookupBuffer(FileName)) { + getModuleCache().lookupPCM(FileName)) { 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())); @@ -186,7 +193,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, return Missing; } - NewModule->Buffer = &getModuleCache().addBuffer(FileName, std::move(*Buf)); + NewModule->Buffer = &getModuleCache().addPCM(FileName, std::move(*Buf)); } // Initialize the stream. @@ -198,7 +205,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, ExpectedSignature, ErrorStr)) { // Try to remove the buffer. If it can't be removed, then it was already // validated by this process. - if (!getModuleCache().tryToRemoveBuffer(NewModule->FileName)) + if (!getModuleCache().tryToDropPCM(NewModule->FileName)) FileMgr.invalidateCache(NewModule->File); return OutOfDate; } @@ -263,17 +270,6 @@ void ModuleManager::removeModules( mod->setASTFile(nullptr); } } - - // 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. - // - // The ModuleCache tracks whether the module was successfully loaded in - // another thread/context; in that case, it won't need to be rebuilt (and - // we can't safely invalidate it anyway). - if (LoadedSuccessfully.count(&*victim) == 0 && - !getModuleCache().tryToRemoveBuffer(victim->FileName)) - FileMgr.invalidateCache(victim->File); } // Delete the modules. diff --git a/clang/test/Modules/Inputs/implicit-invalidate-chain/A.h b/clang/test/Modules/Inputs/implicit-invalidate-chain/A.h new file mode 100644 index 0000000..2b9dab8 --- /dev/null +++ b/clang/test/Modules/Inputs/implicit-invalidate-chain/A.h @@ -0,0 +1,2 @@ +// 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 new file mode 100644 index 0000000..a2711d4 --- /dev/null +++ b/clang/test/Modules/Inputs/implicit-invalidate-chain/B.h @@ -0,0 +1,2 @@ +// 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 new file mode 100644 index 0000000..231e35c --- /dev/null +++ b/clang/test/Modules/Inputs/implicit-invalidate-chain/C.h @@ -0,0 +1,2 @@ +// 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 new file mode 100644 index 0000000..f5b6360 --- /dev/null +++ b/clang/test/Modules/Inputs/implicit-invalidate-chain/module.modulemap @@ -0,0 +1,3 @@ +module A { header "A.h" } +module B { header "B.h" } +module C { header "C.h" } diff --git a/clang/test/Modules/Inputs/relative-import-path/A.h b/clang/test/Modules/Inputs/relative-import-path/A.h new file mode 100644 index 0000000..2b9dab8 --- /dev/null +++ b/clang/test/Modules/Inputs/relative-import-path/A.h @@ -0,0 +1,2 @@ +// A +#include "B.h" diff --git a/clang/test/Modules/Inputs/relative-import-path/B.h b/clang/test/Modules/Inputs/relative-import-path/B.h new file mode 100644 index 0000000..a2711d4 --- /dev/null +++ b/clang/test/Modules/Inputs/relative-import-path/B.h @@ -0,0 +1,2 @@ +// B +#include "C.h" diff --git a/clang/test/Modules/Inputs/relative-import-path/C.h b/clang/test/Modules/Inputs/relative-import-path/C.h new file mode 100644 index 0000000..6f30d47 --- /dev/null +++ b/clang/test/Modules/Inputs/relative-import-path/C.h @@ -0,0 +1 @@ +// C diff --git a/clang/test/Modules/Inputs/relative-import-path/module.modulemap b/clang/test/Modules/Inputs/relative-import-path/module.modulemap new file mode 100644 index 0000000..f5b6360 --- /dev/null +++ b/clang/test/Modules/Inputs/relative-import-path/module.modulemap @@ -0,0 +1,3 @@ +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 new file mode 100644 index 0000000..458f24a --- /dev/null +++ b/clang/test/Modules/implicit-invalidate-chain.c @@ -0,0 +1,67 @@ +// 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/test/Modules/relative-import-path.c b/clang/test/Modules/relative-import-path.c new file mode 100644 index 0000000..3bcb343 --- /dev/null +++ b/clang/test/Modules/relative-import-path.c @@ -0,0 +1,26 @@ +// RUN: rm -rf %t +// RUN: cp -rf %S/Inputs/relative-import-path %t +// RUN: cp %s %t/t.c + +// Use FileCheck, which is more flexible. +// +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache \ +// RUN: -fdisable-module-hash -fsyntax-only \ +// RUN: -I%S/Inputs/relative-import-path \ +// RUN: -working-directory=%t \ +// RUN: -Rmodule-build -Rmodule-import t.c 2>&1 |\ +// RUN: FileCheck %s -implicit-check-not "remark:" -DWORKDIR=%t + +#include "A.h" // \ +// CHECK: remark: building module 'A' +// CHECK: remark: building module 'B' +// CHECK: remark: building module 'C' +// CHECK: remark: finished building module 'C' +// CHECK: remark: importing module 'C' from '[[WORKDIR]]/cache/C.pcm' +// CHECK: remark: finished building module 'B' +// CHECK: remark: importing module 'B' from '[[WORKDIR]]/cache/B.pcm' +// CHECK: remark: importing module 'C' into 'B' from '[[WORKDIR]]/cache/C.pcm' +// CHECK: remark: finished building module 'A' +// CHECK: remark: importing module 'A' from '[[WORKDIR]]/cache/A.pcm' +// CHECK: remark: importing module 'B' into 'A' from '[[WORKDIR]]/cache/B.pcm' +// CHECK: remark: importing module 'C' into 'B' from '[[WORKDIR]]/cache/C.pcm' diff --git a/clang/unittests/Serialization/InMemoryModuleCacheTest.cpp b/clang/unittests/Serialization/InMemoryModuleCacheTest.cpp index 356c9ed..3cba5f5 100644 --- a/clang/unittests/Serialization/InMemoryModuleCacheTest.cpp +++ b/clang/unittests/Serialization/InMemoryModuleCacheTest.cpp @@ -22,72 +22,99 @@ std::unique_ptr getBuffer(int I) { /* RequiresNullTerminator = */ false); } -TEST(InMemoryModuleCacheTest, addBuffer) { - auto B1 = getBuffer(1); - auto B2 = getBuffer(2); - auto B3 = getBuffer(3); - auto *RawB1 = B1.get(); - auto *RawB2 = B2.get(); - auto *RawB3 = B3.get(); - - // Add a few buffers. +TEST(InMemoryModuleCacheTest, initialState) { InMemoryModuleCache Cache; - EXPECT_EQ(RawB1, &Cache.addBuffer("1", std::move(B1))); - EXPECT_EQ(RawB2, &Cache.addBuffer("2", std::move(B2))); - EXPECT_EQ(RawB3, &Cache.addBuffer("3", std::move(B3))); - EXPECT_EQ(RawB1, Cache.lookupBuffer("1")); - EXPECT_EQ(RawB2, Cache.lookupBuffer("2")); - EXPECT_EQ(RawB3, Cache.lookupBuffer("3")); - EXPECT_FALSE(Cache.isBufferFinal("1")); - EXPECT_FALSE(Cache.isBufferFinal("2")); - EXPECT_FALSE(Cache.isBufferFinal("3")); - - // Remove the middle buffer. - EXPECT_FALSE(Cache.tryToRemoveBuffer("2")); - EXPECT_EQ(nullptr, Cache.lookupBuffer("2")); - EXPECT_FALSE(Cache.isBufferFinal("2")); - - // Replace the middle buffer. - B2 = getBuffer(2); - RawB2 = B2.get(); - EXPECT_EQ(RawB2, &Cache.addBuffer("2", std::move(B2))); - - // Check that nothing is final. - EXPECT_FALSE(Cache.isBufferFinal("1")); - EXPECT_FALSE(Cache.isBufferFinal("2")); - EXPECT_FALSE(Cache.isBufferFinal("3")); + EXPECT_EQ(InMemoryModuleCache::Unknown, Cache.getPCMState("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.finalizePCM("B"), "PCM to finalize is unknown"); +#endif } -TEST(InMemoryModuleCacheTest, finalizeCurrentBuffers) { - // Add a buffer. +TEST(InMemoryModuleCacheTest, addPCM) { + auto B = getBuffer(1); + auto *RawB = B.get(); + InMemoryModuleCache Cache; - auto B1 = getBuffer(1); - auto *RawB1 = B1.get(); - Cache.addBuffer("1", std::move(B1)); - ASSERT_FALSE(Cache.isBufferFinal("1")); - - // Finalize it. - Cache.finalizeCurrentBuffers(); - EXPECT_TRUE(Cache.isBufferFinal("1")); - EXPECT_TRUE(Cache.tryToRemoveBuffer("1")); - EXPECT_EQ(RawB1, Cache.lookupBuffer("1")); - EXPECT_TRUE(Cache.isBufferFinal("1")); - - // Repeat. - auto B2 = getBuffer(2); - auto *RawB2 = B2.get(); - Cache.addBuffer("2", std::move(B2)); - EXPECT_FALSE(Cache.isBufferFinal("2")); - - Cache.finalizeCurrentBuffers(); - EXPECT_TRUE(Cache.isBufferFinal("1")); - EXPECT_TRUE(Cache.isBufferFinal("2")); - EXPECT_TRUE(Cache.tryToRemoveBuffer("1")); - EXPECT_TRUE(Cache.tryToRemoveBuffer("2")); - EXPECT_EQ(RawB1, Cache.lookupBuffer("1")); - EXPECT_EQ(RawB2, Cache.lookupBuffer("2")); - EXPECT_TRUE(Cache.isBufferFinal("1")); - EXPECT_TRUE(Cache.isBufferFinal("2")); + 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"); +#endif +} + +TEST(InMemoryModuleCacheTest, addBuiltPCM) { + 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.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)), + "Trying to override finalized PCM"); +#endif +} + +TEST(InMemoryModuleCacheTest, tryToDropPCM) { + auto B = getBuffer(1); + auto *RawB = B.get(); + + InMemoryModuleCache Cache; + EXPECT_EQ(InMemoryModuleCache::Unknown, Cache.getPCMState("B")); + EXPECT_EQ(RawB, &Cache.addPCM("B", std::move(B))); + EXPECT_FALSE(Cache.tryToDropPCM("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"); +#endif + + B = getBuffer(2); + ASSERT_NE(RawB, B.get()); + RawB = B.get(); + + // Add a new one. + EXPECT_EQ(RawB, &Cache.addBuiltPCM("B", std::move(B))); + EXPECT_TRUE(Cache.isPCMFinal("B")); + + // Can try to drop again, but this should error and do nothing. + EXPECT_TRUE(Cache.tryToDropPCM("B")); + EXPECT_EQ(RawB, Cache.lookupPCM("B")); +} + +TEST(InMemoryModuleCacheTest, finalizePCM) { + auto B = getBuffer(1); + 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")); } } // namespace -- 2.7.4