[clang][cas] IncludeTree filesystem merges from imported PCH/modules

Instead of looking through the PCH or modules for input files and
loading them into the current FileManager while scanning, merge them
directly from the include-tree that was used to build that PCH and/or
module. This makes it easier to understand where files come from in the
include-tree, and eliminates certain issues where the scanner module may
include more files than are necessary, or include them with different
paths in the case of VFS/symlinks.  It also avoids needing to re-stat
these files if they are not used during scanning the importing TU. As
part of this, we teach IncludeTreeFilesystem to handle ./ stripping in
paths to match what ASTWriter does.

(cherry picked from commit 4c3f73e)

Author: benlangmuir

clang/include/clang/CAS/IncludeTree.h

@@ -483,4 +483,26 @@ createIncludeTreeFileSystem(IncludeTreeRoot &Root);
 } // namespace cas
 } // namespace clang
 
+namespace llvm {
+template <> struct DenseMapInfo<clang::cas::IncludeTree::FileList::FileEntry> {
+  using FileEntry = clang::cas::IncludeTree::FileList::FileEntry;
+
+  static FileEntry getEmptyKey() {
+    return {cas::ObjectRef::getDenseMapEmptyKey(), 0};
+  }
+
+  static FileEntry getTombstoneKey() {
+    return {cas::ObjectRef::getDenseMapTombstoneKey(), 0};
+  }
+
+  static unsigned getHashValue(FileEntry F) {
+    return F.FileRef.getDenseMapHash();
+  }
+
+  static bool isEqual(FileEntry LHS, FileEntry RHS) {
+    return LHS.FileRef == RHS.FileRef && LHS.Size == RHS.Size;
+  }
+};
+} // namespace llvm
+
 #endif

clang/lib/CAS/IncludeTree.cpp

@@ -461,9 +461,8 @@ class IncludeTreeFileSystem : public llvm::vfs::FileSystem {
       Directories{Alloc};
 
   llvm::ErrorOr<llvm::vfs::Status> status(const Twine &Path) override {
-    SmallString<128> FilenameBuffer;
-    StringRef Filename = Path.toStringRef(FilenameBuffer);
-
+    SmallString<128> Filename;
+    getPath(Path, Filename);
     auto FileEntry = Files.find(Filename);
     if (FileEntry != Files.end()) {
       return makeStatus(Filename, FileEntry->second.Size,
@@ -483,8 +482,8 @@ class IncludeTreeFileSystem : public llvm::vfs::FileSystem {
 
   llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>>
   openFileForRead(const Twine &Path) override {
-    SmallString<128> FilenameBuffer;
-    StringRef Filename = Path.toStringRef(FilenameBuffer);
+    SmallString<128> Filename;
+    getPath(Path, Filename);
     auto MaterializedFile = materialize(Filename);
     if (!MaterializedFile)
       return llvm::errorToErrorCode(MaterializedFile.takeError());
@@ -507,6 +506,15 @@ class IncludeTreeFileSystem : public llvm::vfs::FileSystem {
     return MaterializedFile{ContentsBlob->getData(), Entry->second};
   }
 
+  /// Produce a filename compatible with our StringMaps. See comment in
+  /// \c createIncludeTreeFileSystem.
+  void getPath(const Twine &Path, SmallVectorImpl<char> &Out) {
+    Path.toVector(Out);
+    // Strip dots, but do not eliminate a path consisting only of '.'
+    if (Out.size() != 1)
+      llvm::sys::path::remove_dots(Out);
+  }
+
   static llvm::vfs::Status makeStatus(StringRef Filename, uint64_t Size,
                                       llvm::sys::fs::UniqueID UniqueID,
                                       llvm::sys::fs::file_type Type) {
@@ -552,7 +560,11 @@ cas::createIncludeTreeFileSystem(IncludeTreeRoot &Root) {
         auto FilenameBlob = File.getFilename();
         if (!FilenameBlob)
           return FilenameBlob.takeError();
-        StringRef Filename = FilenameBlob->getData();
+        SmallString<128> Filename(FilenameBlob->getData());
+        // Strip './' in the filename to match the behaviour of ASTWriter; we
+        // also strip './' in IncludeTreeFileSystem::getPath.
+        assert(Filename != ".");
+        llvm::sys::path::remove_dots(Filename);
 
         StringRef DirName = llvm::sys::path::parent_path(Filename);
         if (DirName.empty())

clang/lib/Tooling/DependencyScanning/IncludeTreeActionController.cpp

@@ -99,6 +99,7 @@ class IncludeTreeBuilder {
     llvm::SmallBitVector HasIncludeChecks;
   };
 
+  Error addModuleInputs(ASTReader &Reader);
   Expected<cas::ObjectRef> getObjectForFile(Preprocessor &PP, FileID FID);
   Expected<cas::ObjectRef>
   getObjectForFileNonCached(FileManager &FM, const SrcMgr::FileInfo &FI);
@@ -128,7 +129,7 @@ class IncludeTreeBuilder {
   // are recorded in the PCH, ordered by \p FileEntry::UID index.
   SmallVector<StringRef> PreIncludedFileNames;
   llvm::BitVector SeenIncludeFiles;
-  SmallVector<cas::IncludeTree::FileList::FileEntry> IncludedFiles;
+  llvm::SetVector<cas::IncludeTree::FileList::FileEntry> IncludedFiles;
   std::optional<cas::ObjectRef> PredefinesBufferRef;
   std::optional<cas::ObjectRef> ModuleIncludesBufferRef;
   std::optional<cas::ObjectRef> ModuleMapFileRef;
@@ -495,30 +496,8 @@ IncludeTreeBuilder::finishIncludeTree(CompilerInstance &ScanInstance,
       return Error::success(); // no need for additional work.
 
     // Go through all the recorded input files.
-    SmallVector<const FileEntry *, 32> NotSeenIncludes;
-    for (serialization::ModuleFile &MF : Reader->getModuleManager()) {
-      if (hasErrorOccurred())
-        break;
-      Reader->visitInputFiles(
-          MF, /*IncludeSystem=*/true, /*Complain=*/false,
-          [&](const serialization::InputFile &IF, bool isSystem) {
-            OptionalFileEntryRef FE = IF.getFile();
-            assert(FE);
-            if (FE->getUID() >= SeenIncludeFiles.size() ||
-                !SeenIncludeFiles[FE->getUID()])
-              NotSeenIncludes.push_back(*FE);
-          });
-    }
-    // Sort so we can visit the files in deterministic order.
-    llvm::sort(NotSeenIncludes, [](const FileEntry *LHS, const FileEntry *RHS) {
-      return LHS->getUID() < RHS->getUID();
-    });
-
-    for (const FileEntry *FE : NotSeenIncludes) {
-      auto FileNode = addToFileList(FM, FE);
-      if (!FileNode)
-        return FileNode.takeError();
-    }
+    if (Error E = addModuleInputs(*Reader))
+      return E;
 
     PreprocessorOptions &PPOpts = NewInvocation.getPreprocessorOpts();
     if (PPOpts.ImplicitPCHInclude.empty())
@@ -544,7 +523,8 @@ IncludeTreeBuilder::finishIncludeTree(CompilerInstance &ScanInstance,
       getCASTreeForFileIncludes(IncludeStack.pop_back_val());
   if (!MainIncludeTree)
     return MainIncludeTree.takeError();
-  auto FileList = cas::IncludeTree::FileList::create(DB, IncludedFiles);
+  auto FileList =
+      cas::IncludeTree::FileList::create(DB, IncludedFiles.getArrayRef());
   if (!FileList)
     return FileList.takeError();
 
@@ -553,6 +533,39 @@ IncludeTreeBuilder::finishIncludeTree(CompilerInstance &ScanInstance,
                                       ModuleMapFileRef);
 }
 
+Error IncludeTreeBuilder::addModuleInputs(ASTReader &Reader) {
+  for (serialization::ModuleFile &MF : Reader.getModuleManager()) {
+    // Only add direct imports to avoid duplication. Each include tree is a
+    // superset of its imported modules' include trees.
+    if (!MF.isDirectlyImported())
+      continue;
+
+    assert(!MF.IncludeTreeID.empty() && "missing include-tree for import");
+
+    std::optional<cas::CASID> ID;
+    if (Error E = DB.parseID(MF.IncludeTreeID).moveInto(ID))
+      return E;
+    std::optional<cas::ObjectRef> Ref = DB.getReference(*ID);
+    if (!Ref)
+      return DB.createUnknownObjectError(*ID);
+    std::optional<cas::IncludeTreeRoot> Root;
+    if (Error E = cas::IncludeTreeRoot::get(DB, *Ref).moveInto(Root))
+      return E;
+    std::optional<cas::IncludeTree::FileList> Files;
+    if (Error E = Root->getFileList().moveInto(Files))
+      return E;
+
+    Error E = Files->forEachFile([&](auto IF, auto Size) -> Error {
+      IncludedFiles.insert({IF.getRef(), Size});
+      return Error::success();
+    });
+    if (E)
+      return E;
+  }
+
+  return Error::success();
+}
+
 Expected<cas::ObjectRef> IncludeTreeBuilder::getObjectForFile(Preprocessor &PP,
                                                               FileID FID) {
   SourceManager &SM = PP.getSourceManager();
@@ -628,7 +641,7 @@ IncludeTreeBuilder::addToFileList(FileManager &FM, const FileEntry *FE) {
     auto FileNode = createIncludeFile(Filename, **CASContents);
     if (!FileNode)
       return FileNode.takeError();
-    IncludedFiles.push_back(
+    IncludedFiles.insert(
         {FileNode->getRef(),
          static_cast<cas::IncludeTree::FileList::FileSizeTy>(FE->getSize())});
     return FileNode->getRef();

clang/test/ClangScanDeps/modules-include-tree.c

@@ -77,8 +77,8 @@
 // fmodule-file, and fmodule-file-cache-key options.
 
 // CHECK: Files:
-// CHECK: [[PREFIX]]/Left.h llvmcas://{{[[:xdigit:]]+}}
 // CHECK: [[PREFIX]]/module.modulemap llvmcas://{{[[:xdigit:]]+}}
+// CHECK: [[PREFIX]]/Left.h llvmcas://{{[[:xdigit:]]+}}
 // CHECK: [[PREFIX]]/Top.h llvmcas://{{[[:xdigit:]]+}}
 // CHECK: [[PREFIX]]/Right.h llvmcas://{{[[:xdigit:]]+}}