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(),
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(),
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
