From 33c8090a2f7f2e6d554f84a15a573a7ff7d3f59d Mon Sep 17 00:00:00 2001 From: Ben Langmuir Date: Mon, 30 Jun 2014 20:04:14 +0000 Subject: [PATCH] Consider module depedencies when checking a preamble in libclang MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Add module dependencies (header files, module map files) to the list of files to check when deciding whether to rebuild a preamble. That fixes using preambles with module imports so long as they are in non-overridden files. My intent is to use to unify the existing dependency collectors to the new “DependencyCollectory” interface from this commit, starting with the DependencyFileGenerator. llvm-svn: 212060 --- clang/include/clang/Frontend/CompilerInstance.h | 6 ++ clang/include/clang/Frontend/Utils.h | 33 +++++++++ clang/lib/Frontend/ASTUnit.cpp | 24 +++--- clang/lib/Frontend/CompilerInstance.cpp | 6 ++ clang/lib/Frontend/DependencyFile.cpp | 99 +++++++++++++++++++++++++ clang/unittests/libclang/LibclangTest.cpp | 35 ++++++++- 6 files changed, 187 insertions(+), 16 deletions(-) diff --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h index d72f904..e089726 100644 --- a/clang/include/clang/Frontend/CompilerInstance.h +++ b/clang/include/clang/Frontend/CompilerInstance.h @@ -111,6 +111,8 @@ class CompilerInstance : public ModuleLoader { /// \brief The dependency file generator. std::unique_ptr TheDependencyFileGenerator; + std::vector> DependencyCollectors; + /// \brief The set of top-level modules that has already been loaded, /// along with the module map llvm::DenseMap KnownModules; @@ -711,6 +713,10 @@ public: GlobalModuleIndex *loadGlobalModuleIndex(SourceLocation TriggerLoc) override; bool lookupMissingImports(StringRef Name, SourceLocation TriggerLoc) override; + + void addDependencyCollector(std::shared_ptr Listener) { + DependencyCollectors.push_back(std::move(Listener)); + } }; } // end namespace clang diff --git a/clang/include/clang/Frontend/Utils.h b/clang/include/clang/Frontend/Utils.h index a791cd5..c245b09 100644 --- a/clang/include/clang/Frontend/Utils.h +++ b/clang/include/clang/Frontend/Utils.h @@ -69,6 +69,39 @@ void InitializePreprocessor(Preprocessor &PP, void DoPrintPreprocessedInput(Preprocessor &PP, raw_ostream* OS, const PreprocessorOutputOptions &Opts); +/// An interface for collecting the dependencies of a compilation. Users should +/// use \c attachToPreprocessor and \c attachToASTReader to get all of the +/// dependencies. +// FIXME: Migrate DependencyFileGen, DependencyGraphGen, ModuleDepCollectory to +// use this interface. +class DependencyCollector { +public: + void attachToPreprocessor(Preprocessor &PP); + void attachToASTReader(ASTReader &R); + llvm::ArrayRef getDependencies() const { return Dependencies; } + + /// Called when a new file is seen. Return true if \p Filename should be added + /// to the list of dependencies. + /// + /// The default implementation ignores and system files. + virtual bool sawDependency(StringRef Filename, bool FromModule, + bool IsSystem, bool IsModuleFile, bool IsMissing); + /// Called when the end of the main file is reached. + virtual void finishedMainFile() { } + /// Return true if system files should be passed to sawDependency(). + virtual bool needSystemDependencies() { return false; } + virtual ~DependencyCollector(); + +public: // implementation detail + /// Add a dependency \p Filename if it has not been seen before and + /// sawDependency() returns true. + void maybeAddDependency(StringRef Filename, bool FromModule, bool IsSystem, + bool IsModuleFile, bool IsMissing); +private: + llvm::StringSet<> Seen; + std::vector Dependencies; +}; + /// Builds a depdenency file when attached to a Preprocessor (for includes) and /// ASTReader (for module imports), and writes it out at the end of processing /// a source file. Users should attach to the ast reader whenever a module is diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp index df50c40..3c2b423 100644 --- a/clang/lib/Frontend/ASTUnit.cpp +++ b/clang/lib/Frontend/ASTUnit.cpp @@ -1609,6 +1609,9 @@ llvm::MemoryBuffer *ASTUnit::getMainBufferWithPrecompiledPreamble( Clang->setSourceManager(new SourceManager(getDiagnostics(), Clang->getFileManager())); + auto PreambleDepCollector = std::make_shared(); + Clang->addDependencyCollector(PreambleDepCollector); + std::unique_ptr Act; Act.reset(new PrecompilePreambleAction(*this)); if (!Act->BeginSourceFile(*Clang.get(), Clang->getFrontendOpts().Inputs[0])) { @@ -1657,29 +1660,20 @@ llvm::MemoryBuffer *ASTUnit::getMainBufferWithPrecompiledPreamble( // so we can verify whether they have changed or not. FilesInPreamble.clear(); SourceManager &SourceMgr = Clang->getSourceManager(); - const llvm::MemoryBuffer *MainFileBuffer - = SourceMgr.getBuffer(SourceMgr.getMainFileID()); - for (SourceManager::fileinfo_iterator F = SourceMgr.fileinfo_begin(), - FEnd = SourceMgr.fileinfo_end(); - F != FEnd; - ++F) { - const FileEntry *File = F->second->OrigEntry; - if (!File) - continue; - const llvm::MemoryBuffer *Buffer = F->second->getRawBuffer(); - if (Buffer == MainFileBuffer) + for (auto &Filename : PreambleDepCollector->getDependencies()) { + const FileEntry *File = Clang->getFileManager().getFile(Filename); + if (!File || File == SourceMgr.getFileEntryForID(SourceMgr.getMainFileID())) continue; - if (time_t ModTime = File->getModificationTime()) { FilesInPreamble[File->getName()] = PreambleFileHash::createForFile( - F->second->getSize(), ModTime); + File->getSize(), ModTime); } else { - assert(F->second->getSize() == Buffer->getBufferSize()); + llvm::MemoryBuffer *Buffer = SourceMgr.getMemoryBufferForFile(File); FilesInPreamble[File->getName()] = PreambleFileHash::createForMemoryBuffer(Buffer); } } - + PreambleRebuildCounter = 1; PreprocessorOpts.eraseRemappedFile( PreprocessorOpts.remapped_file_buffer_end() - 1); diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index b5efb14..60f8ae8 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -288,6 +288,9 @@ void CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) { AttachDependencyGraphGen(*PP, DepOpts.DOTOutputFile, getHeaderSearchOpts().Sysroot); + for (auto &Listener : DependencyCollectors) + Listener->attachToPreprocessor(*PP); + // If we don't have a collector, but we are collecting module dependencies, // then we're the top level compiler instance and need to create one. if (!ModuleDepCollector && !DepOpts.ModuleDependencyOutputDir.empty()) @@ -1233,6 +1236,9 @@ CompilerInstance::loadModule(SourceLocation ImportLoc, if (ModuleDepCollector) ModuleDepCollector->attachToASTReader(*ModuleManager); + for (auto &Listener : DependencyCollectors) + Listener->attachToASTReader(*ModuleManager); + // Try to load the module file. unsigned ARRFlags = ASTReader::ARR_OutOfDate | ASTReader::ARR_Missing; switch (ModuleManager->ReadAST(ModuleFileName, serialization::MK_Module, diff --git a/clang/lib/Frontend/DependencyFile.cpp b/clang/lib/Frontend/DependencyFile.cpp index e72be89..0b9c0d4 100644 --- a/clang/lib/Frontend/DependencyFile.cpp +++ b/clang/lib/Frontend/DependencyFile.cpp @@ -29,6 +29,105 @@ using namespace clang; namespace { +struct DepCollectorPPCallbacks : public PPCallbacks { + DependencyCollector &DepCollector; + SourceManager &SM; + DepCollectorPPCallbacks(DependencyCollector &L, SourceManager &SM) + : DepCollector(L), SM(SM) { } + + void FileChanged(SourceLocation Loc, FileChangeReason Reason, + SrcMgr::CharacteristicKind FileType, + FileID PrevFID) override { + if (Reason != PPCallbacks::EnterFile) + return; + + // Dependency generation really does want to go all the way to the + // file entry for a source location to find out what is depended on. + // We do not want #line markers to affect dependency generation! + const FileEntry *FE = + SM.getFileEntryForID(SM.getFileID(SM.getExpansionLoc(Loc))); + if (!FE) + return; + + StringRef Filename = FE->getName(); + + // Remove leading "./" (or ".//" or "././" etc.) + while (Filename.size() > 2 && Filename[0] == '.' && + llvm::sys::path::is_separator(Filename[1])) { + Filename = Filename.substr(1); + while (llvm::sys::path::is_separator(Filename[0])) + Filename = Filename.substr(1); + } + + DepCollector.maybeAddDependency(Filename, /*FromModule*/false, + FileType != SrcMgr::C_User, + /*IsModuleFile*/false, /*IsMissing*/false); + } + + void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok, + StringRef FileName, bool IsAngled, + CharSourceRange FilenameRange, const FileEntry *File, + StringRef SearchPath, StringRef RelativePath, + const Module *Imported) override { + if (!File) + DepCollector.maybeAddDependency(FileName, /*FromModule*/false, + /*IsSystem*/false, /*IsModuleFile*/false, + /*IsMissing*/true); + // Files that actually exist are handled by FileChanged. + } + + void EndOfMainFile() override { + DepCollector.finishedMainFile(); + } +}; + +struct DepCollectorASTListener : public ASTReaderListener { + DependencyCollector &DepCollector; + DepCollectorASTListener(DependencyCollector &L) : DepCollector(L) { } + bool needsInputFileVisitation() override { return true; } + bool needsSystemInputFileVisitation() override { + return DepCollector.needSystemDependencies(); + } + void visitModuleFile(StringRef Filename) override { + DepCollector.maybeAddDependency(Filename, /*FromModule*/true, + /*IsSystem*/false, /*IsModuleFile*/true, + /*IsMissing*/false); + } + bool visitInputFile(StringRef Filename, bool IsSystem, + bool IsOverridden) override { + if (IsOverridden) + return true; + + DepCollector.maybeAddDependency(Filename, /*FromModule*/true, IsSystem, + /*IsModuleFile*/false, /*IsMissing*/false); + return true; + } +}; +} // end anonymous namespace + +void DependencyCollector::maybeAddDependency(StringRef Filename, bool FromModule, + bool IsSystem, bool IsModuleFile, + bool IsMissing) { + if (Seen.insert(Filename) && + sawDependency(Filename, FromModule, IsSystem, IsModuleFile, IsMissing)) + Dependencies.push_back(Filename); +} + +bool DependencyCollector::sawDependency(StringRef Filename, bool FromModule, + bool IsSystem, bool IsModuleFile, + bool IsMissing) { + return Filename != "" && (needSystemDependencies() || !IsSystem); +} + +DependencyCollector::~DependencyCollector() { } +void DependencyCollector::attachToPreprocessor(Preprocessor &PP) { + PP.addPPCallbacks(new DepCollectorPPCallbacks(*this, PP.getSourceManager())); +} +void DependencyCollector::attachToASTReader(ASTReader &R) { + R.addListener(new DepCollectorASTListener(*this)); +} + +namespace { /// Private implementation for DependencyFileGenerator class DFGImpl : public PPCallbacks { std::vector Files; diff --git a/clang/unittests/libclang/LibclangTest.cpp b/clang/unittests/libclang/LibclangTest.cpp index f0bfb27..8f2694f 100644 --- a/clang/unittests/libclang/LibclangTest.cpp +++ b/clang/unittests/libclang/LibclangTest.cpp @@ -340,9 +340,9 @@ TEST(libclang, ModuleMapDescriptor) { } class LibclangReparseTest : public ::testing::Test { - std::string TestDir; std::set Files; public: + std::string TestDir; CXIndex Index; CXTranslationUnit ClangTU; unsigned TUFlags; @@ -420,3 +420,36 @@ TEST_F(LibclangReparseTest, Reparse) { ASSERT_TRUE(ReparseTU(0, nullptr /* No unsaved files. */)); EXPECT_EQ(0U, clang_getNumDiagnostics(ClangTU)); } + +TEST_F(LibclangReparseTest, ReparseWithModule) { + const char *HeaderTop = "#ifndef H\n#define H\nstruct Foo { int bar;"; + const char *HeaderBottom = "\n};\n#endif\n"; + const char *MFile = "#include \"HeaderFile.h\"\nint main() {" + " struct Foo foo; foo.bar = 7; foo.baz = 8; }\n"; + const char *ModFile = "module A { header \"HeaderFile.h\" }\n"; + std::string HeaderName = "HeaderFile.h"; + std::string MName = "MFile.m"; + std::string ModName = "module.modulemap"; + WriteFile(MName, MFile); + WriteFile(HeaderName, std::string(HeaderTop) + HeaderBottom); + WriteFile(ModName, ModFile); + + const char *Args[] = { "-fmodules", "-I", TestDir.c_str() }; + int NumArgs = sizeof(Args) / sizeof(Args[0]); + ClangTU = clang_parseTranslationUnit(Index, MName.c_str(), Args, NumArgs, + nullptr, 0, TUFlags); + EXPECT_EQ(1U, clang_getNumDiagnostics(ClangTU)); + DisplayDiagnostics(); + + // Immedaitely reparse. + ASSERT_TRUE(ReparseTU(0, nullptr /* No unsaved files. */)); + EXPECT_EQ(1U, clang_getNumDiagnostics(ClangTU)); + + std::string NewHeaderContents = + std::string(HeaderTop) + "int baz;" + HeaderBottom; + WriteFile(HeaderName, NewHeaderContents); + + // Reparse after fix. + ASSERT_TRUE(ReparseTU(0, nullptr /* No unsaved files. */)); + EXPECT_EQ(0U, clang_getNumDiagnostics(ClangTU)); +} -- 2.7.4