r212060 - Consider module depedencies when checking a preamble in libclang

Nico Weber thakis at chromium.org
Wed Jul 16 08:59:04 PDT 2014


LibclangReparseTest.ReparseWithModule fails for me. (With `./configure
--enable-optimized --enable-targets=host-only --enable-libedit=no
--disable-threads --disable-pthreads --without-llvmgcc --without-llvmgxx`)



On Mon, Jun 30, 2014 at 1:04 PM, Ben Langmuir <blangmuir at apple.com> wrote:

> Author: benlangmuir
> Date: Mon Jun 30 15:04:14 2014
> New Revision: 212060
>
> URL: http://llvm.org/viewvc/llvm-project?rev=212060&view=rev
> Log:
> 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.
>
> Modified:
>     cfe/trunk/include/clang/Frontend/CompilerInstance.h
>     cfe/trunk/include/clang/Frontend/Utils.h
>     cfe/trunk/lib/Frontend/ASTUnit.cpp
>     cfe/trunk/lib/Frontend/CompilerInstance.cpp
>     cfe/trunk/lib/Frontend/DependencyFile.cpp
>     cfe/trunk/unittests/libclang/LibclangTest.cpp
>
> Modified: cfe/trunk/include/clang/Frontend/CompilerInstance.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CompilerInstance.h?rev=212060&r1=212059&r2=212060&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Frontend/CompilerInstance.h (original)
> +++ cfe/trunk/include/clang/Frontend/CompilerInstance.h Mon Jun 30
> 15:04:14 2014
> @@ -111,6 +111,8 @@ class CompilerInstance : public ModuleLo
>    /// \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
>
> Modified: cfe/trunk/include/clang/Frontend/Utils.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/Utils.h?rev=212060&r1=212059&r2=212060&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Frontend/Utils.h (original)
> +++ cfe/trunk/include/clang/Frontend/Utils.h Mon Jun 30 15:04:14 2014
> @@ -69,6 +69,39 @@ void InitializePreprocessor(Preprocessor
>  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
>
> Modified: cfe/trunk/lib/Frontend/ASTUnit.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTUnit.cpp?rev=212060&r1=212059&r2=212060&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Frontend/ASTUnit.cpp (original)
> +++ cfe/trunk/lib/Frontend/ASTUnit.cpp Mon Jun 30 15:04:14 2014
> @@ -1609,6 +1609,9 @@ llvm::MemoryBuffer *ASTUnit::getMainBuff
>    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::getMainBuff
>    // 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);
>
> Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=212060&r1=212059&r2=212060&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
> +++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Mon Jun 30 15:04:14 2014
> @@ -288,6 +288,9 @@ void CompilerInstance::createPreprocesso
>      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(SourceLocat
>      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,
>
> Modified: cfe/trunk/lib/Frontend/DependencyFile.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/DependencyFile.cpp?rev=212060&r1=212059&r2=212060&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Frontend/DependencyFile.cpp (original)
> +++ cfe/trunk/lib/Frontend/DependencyFile.cpp Mon Jun 30 15:04:14 2014
> @@ -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 != "<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 {
>    std::vector<std::string> Files;
>
> Modified: cfe/trunk/unittests/libclang/LibclangTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/libclang/LibclangTest.cpp?rev=212060&r1=212059&r2=212060&view=diff
>
> ==============================================================================
> --- cfe/trunk/unittests/libclang/LibclangTest.cpp (original)
> +++ cfe/trunk/unittests/libclang/LibclangTest.cpp Mon Jun 30 15:04:14 2014
> @@ -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;
> @@ -407,6 +407,39 @@ TEST_F(LibclangReparseTest, Reparse) {
>                                         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));
> +}
> +
> +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. */));
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140716/5e573407/attachment.html>


More information about the cfe-commits mailing list