[PATCH] D153670: [clang/HeaderSearch] Make sure `loadSubdirectoryModuleMaps` doesn't cause loading of regular files

[Argyrios Kyrtzidis via Phabricator via cfe-commits]

akyrtzi created this revision.
Herald added a project: All.
akyrtzi requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

`HeaderSearch::loadSubdirectoryModuleMaps` `stat`s all the files in a directory which causes the dependency scanning
service to load and cache their contents. This is problematic because a file may be in the process of being generated
and could be cached by the dep-scan service while it is still incomplete.

To address this change `loadSubdirectoryModuleMaps` to ignore regular files.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153670

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/unittests/Tooling/DependencyScannerTest.cpp


Index: clang/unittests/Tooling/DependencyScannerTest.cpp
===================================================================
--- clang/unittests/Tooling/DependencyScannerTest.cpp
+++ clang/unittests/Tooling/DependencyScannerTest.cpp
@@ -239,3 +239,57 @@
   EXPECT_EQ(convert_to_slash(DepFile),
             "test.cpp.o: /root/test.cpp /root/header.h\n");
 }
+
+TEST(DependencyScanner, ScanDepsWithModuleLookup) {
+  std::vector<std::string> CommandLine = {
+      "clang",
+      "-target",
+      "x86_64-apple-macosx10.7",
+      "-c",
+      "test.m",
+      "-o"
+      "test.m.o",
+      "-fmodules",
+      "-I/root/SomeSources",
+  };
+  StringRef CWD = "/root";
+
+  auto VFS = new llvm::vfs::InMemoryFileSystem();
+  VFS->setCurrentWorkingDirectory(CWD);
+  auto Sept = llvm::sys::path::get_separator();
+  std::string OtherPath =
+      std::string(llvm::formatv("{0}root{0}SomeSources{0}other.h", Sept));
+  std::string TestPath = std::string(llvm::formatv("{0}root{0}test.m", Sept));
+
+  VFS->addFile(OtherPath, 0, llvm::MemoryBuffer::getMemBuffer("\n"));
+  VFS->addFile(TestPath, 0, llvm::MemoryBuffer::getMemBuffer("@import Foo;\n"));
+
+  struct InterceptorFS : llvm::vfs::ProxyFileSystem {
+    std::vector<std::string> ReadFiles;
+
+    InterceptorFS(IntrusiveRefCntPtr<FileSystem> UnderlyingFS)
+        : ProxyFileSystem(UnderlyingFS) {}
+
+    llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>>
+    openFileForRead(const Twine &Path) override {
+      ReadFiles.push_back(Path.str());
+      return ProxyFileSystem::openFileForRead(Path);
+    }
+  };
+
+  auto InterceptFS = llvm::makeIntrusiveRefCnt<InterceptorFS>(VFS);
+
+  DependencyScanningService Service(ScanningMode::DependencyDirectivesScan,
+                                    ScanningOutputFormat::Make);
+  DependencyScanningTool ScanTool(Service, InterceptFS);
+
+  // This will fail with "fatal error: module 'Foo' not found" but it doesn't
+  // matter, the point of the test is to check that files are not read
+  // unnecessarily.
+  std::string DepFile;
+  ASSERT_THAT_ERROR(
+      ScanTool.getDependencyFile(CommandLine, CWD).moveInto(DepFile),
+      llvm::Failed());
+
+  EXPECT_EQ(InterceptFS->ReadFiles, std::vector<std::string>{"test.m"});
+}
Index: clang/lib/Lex/HeaderSearch.cpp
===================================================================
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1917,6 +1917,8 @@
   llvm::vfs::FileSystem &FS = FileMgr.getVirtualFileSystem();
   for (llvm::vfs::directory_iterator Dir = FS.dir_begin(DirNative, EC), DirEnd;
        Dir != DirEnd && !EC; Dir.increment(EC)) {
+    if (Dir->type() == llvm::sys::fs::file_type::regular_file)
+      continue;
     bool IsFramework = llvm::sys::path::extension(Dir->path()) == ".framework";
     if (IsFramework == SearchDir.isFramework())
       loadModuleMapFile(Dir->path(), SearchDir.isSystemHeaderDirectory(),


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D153670.534099.patch
Type: text/x-patch
Size: 2919 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230623/a371c980/attachment.bin>