Consider module depedencies when checking a preamble in libclang

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

@@ -111,6 +111,8 @@ class CompilerInstance : public ModuleLoader {
   /// \brief The dependency file generator.
   std::unique_ptr<DependencyFileGenerator> TheDependencyFileGenerator;
 
+  std::vector<std::shared_ptr<DependencyCollector>> DependencyCollectors;
+
   /// \brief The set of top-level modules that has already been loaded,
   /// along with the module map
   llvm::DenseMap<const IdentifierInfo *, Module *> KnownModules;
@@ -711,6 +713,10 @@ public:
   GlobalModuleIndex *loadGlobalModuleIndex(SourceLocation TriggerLoc) override;
 
   bool lookupMissingImports(StringRef Name, SourceLocation TriggerLoc) override;
+
+  void addDependencyCollector(std::shared_ptr<DependencyCollector> Listener) {
+    DependencyCollectors.push_back(std::move(Listener));
+  }
 };
 
 } // end namespace clang

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<std::string> 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 <built-in> 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<std::string> 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

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<DependencyCollector>();
+  Clang->addDependencyCollector(PreambleDepCollector);
+
   std::unique_ptr<PrecompilePreambleAction> 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)
+  for (auto &Filename : PreambleDepCollector->getDependencies()) {
+    const FileEntry *File = Clang->getFileManager().getFile(Filename);
+    if (!File || File == SourceMgr.getFileEntryForID(SourceMgr.getMainFileID()))
       continue;
-    const llvm::MemoryBuffer *Buffer = F->second->getRawBuffer();
-    if (Buffer == MainFileBuffer)
-      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);

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,

clang/lib/Frontend/DependencyFile.cpp

@@ -28,6 +28,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 != "<built-in>" && (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 {

clang/unittests/libclang/LibclangTest.cpp

@@ -340,9 +340,9 @@ TEST(libclang, ModuleMapDescriptor) {
 }
 
 class LibclangReparseTest : public ::testing::Test {
-  std::string TestDir;
   std::set<std::string> 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));
+}