Author: Jan Svoboda Date: 2026-05-08T12:54:52-07:00 New Revision: 1ae2255db9d0098a1ed30a184fcdfcc787247cce
URL: https://github.com/llvm/llvm-project/commit/1ae2255db9d0098a1ed30a184fcdfcc787247cce DIFF: https://github.com/llvm/llvm-project/commit/1ae2255db9d0098a1ed30a184fcdfcc787247cce.diff LOG: [clang][deps] Use `ModuleDepCollector` for Make output (#182063) The dependency scanner works significantly differently depending on what kind of output it's asked to produce. The Make output format has been using the regular Clang dependency collection mechanism since it was first implemented. This means the implementation works very differently to the rest of the scanner and isn't able to turn implicit module command lines into Makefiles using explicit modules. This PR unifies the two implementations, using `ModuleDepCollector` even for Make output. Emitting explicit module builds into Makefiles will come in a later PR. Added: Modified: clang/include/clang/DependencyScanning/DependencyScannerImpl.h clang/include/clang/DependencyScanning/ModuleDepCollector.h clang/include/clang/Tooling/DependencyScanningTool.h clang/lib/DependencyScanning/DependencyScannerImpl.cpp clang/lib/DependencyScanning/ModuleDepCollector.cpp clang/lib/Tooling/DependencyScanningTool.cpp clang/test/ClangScanDeps/modules-cc1.cpp clang/test/ClangScanDeps/modules-has-include-umbrella-header.c clang/tools/clang-scan-deps/ClangScanDeps.cpp clang/unittests/Tooling/DependencyScannerTest.cpp Removed: ################################################################################ diff --git a/clang/include/clang/DependencyScanning/DependencyScannerImpl.h b/clang/include/clang/DependencyScanning/DependencyScannerImpl.h index 55dcbd6fe0e9f..31dcffb2f01dd 100644 --- a/clang/include/clang/DependencyScanning/DependencyScannerImpl.h +++ b/clang/include/clang/DependencyScanning/DependencyScannerImpl.h @@ -121,11 +121,10 @@ computePrebuiltModulesASTMap(CompilerInstance &ScanInstance, std::shared_ptr<ModuleDepCollector> initializeScanInstanceDependencyCollector( CompilerInstance &ScanInstance, std::unique_ptr<DependencyOutputOptions> DepOutputOpts, - StringRef WorkingDirectory, DependencyConsumer &Consumer, - DependencyScanningService &Service, CompilerInvocation &Inv, - DependencyActionController &Controller, + DependencyConsumer &Consumer, DependencyScanningService &Service, + CompilerInvocation &Inv, DependencyActionController &Controller, PrebuiltModulesAttrsMap PrebuiltModulesASTMap, - llvm::SmallVector<StringRef> &StableDirs); + SmallVector<StringRef> &StableDirs); } // namespace dependencies } // namespace clang diff --git a/clang/include/clang/DependencyScanning/ModuleDepCollector.h b/clang/include/clang/DependencyScanning/ModuleDepCollector.h index e7dd907a00381..108127fbbe523 100644 --- a/clang/include/clang/DependencyScanning/ModuleDepCollector.h +++ b/clang/include/clang/DependencyScanning/ModuleDepCollector.h @@ -228,6 +228,9 @@ class ModuleDepCollectorPP final : public PPCallbacks { void LexedFileChanged(FileID FID, LexedFileChangeReason Reason, SrcMgr::CharacteristicKind FileType, FileID PrevFID, SourceLocation Loc) override; + void HasInclude(SourceLocation Loc, StringRef FileName, bool IsAngled, + OptionalFileEntryRef File, + SrcMgr::CharacteristicKind FileType) override; void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName, bool IsAngled, CharSourceRange FilenameRange, diff --git a/clang/include/clang/Tooling/DependencyScanningTool.h b/clang/include/clang/Tooling/DependencyScanningTool.h index c845e212ce153..c368e93fa6286 100644 --- a/clang/include/clang/Tooling/DependencyScanningTool.h +++ b/clang/include/clang/Tooling/DependencyScanningTool.h @@ -47,6 +47,7 @@ class DependencyScanningTool { /// dependency file contents otherwise. std::optional<std::string> getDependencyFile(ArrayRef<std::string> CommandLine, StringRef CWD, + dependencies::LookupModuleOutputCallback LookupModuleOutput, DiagnosticConsumer &DiagConsumer); /// Collect the module dependency in P1689 format for C++20 named modules. diff --git a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp index 40a7d1b908a6c..224413bb99cbc 100644 --- a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp +++ b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp @@ -27,32 +27,6 @@ using namespace clang; using namespace dependencies; -namespace { -/// Forwards the gatherered dependencies to the consumer. -class DependencyConsumerForwarder : public DependencyFileGenerator { -public: - DependencyConsumerForwarder(std::unique_ptr<DependencyOutputOptions> Opts, - StringRef WorkingDirectory, DependencyConsumer &C) - : DependencyFileGenerator(*Opts), WorkingDirectory(WorkingDirectory), - Opts(std::move(Opts)), C(C) {} - - void finishedMainFile(DiagnosticsEngine &Diags) override { - C.handleDependencyOutputOpts(*Opts); - llvm::SmallString<256> CanonPath; - for (const auto &File : getDependencies()) { - CanonPath = File; - llvm::sys::path::remove_dots(CanonPath, /*remove_dot_dot=*/true); - llvm::sys::path::make_absolute(WorkingDirectory, CanonPath); - C.handleFileDependency(CanonPath); - } - } - -private: - StringRef WorkingDirectory; - std::unique_ptr<DependencyOutputOptions> Opts; - DependencyConsumer &C; -}; - static bool checkHeaderSearchPaths(const HeaderSearchOptions &HSOpts, const HeaderSearchOptions &ExistingHSOpts, DiagnosticsEngine *Diags, @@ -77,6 +51,8 @@ static bool checkHeaderSearchPaths(const HeaderSearchOptions &HSOpts, return false; } +namespace { + using PrebuiltModuleFilesT = decltype(HeaderSearchOptions::PrebuiltModuleFiles); /// A listener that collects the imported modules and the input @@ -531,27 +507,14 @@ std::shared_ptr<ModuleDepCollector> dependencies::initializeScanInstanceDependencyCollector( CompilerInstance &ScanInstance, std::unique_ptr<DependencyOutputOptions> DepOutputOpts, - StringRef WorkingDirectory, DependencyConsumer &Consumer, - DependencyScanningService &Service, CompilerInvocation &Inv, - DependencyActionController &Controller, + DependencyConsumer &Consumer, DependencyScanningService &Service, + CompilerInvocation &Inv, DependencyActionController &Controller, PrebuiltModulesAttrsMap PrebuiltModulesASTMap, - llvm::SmallVector<StringRef> &StableDirs) { - std::shared_ptr<ModuleDepCollector> MDC; - switch (Service.getOpts().Format) { - case ScanningOutputFormat::Make: - ScanInstance.addDependencyCollector( - std::make_shared<DependencyConsumerForwarder>( - std::move(DepOutputOpts), WorkingDirectory, Consumer)); - break; - case ScanningOutputFormat::P1689: - case ScanningOutputFormat::Full: - MDC = std::make_shared<ModuleDepCollector>( - Service, std::move(DepOutputOpts), ScanInstance, Consumer, Controller, - Inv, std::move(PrebuiltModulesASTMap), StableDirs); - ScanInstance.addDependencyCollector(MDC); - break; - } - + SmallVector<StringRef> &StableDirs) { + auto MDC = std::make_shared<ModuleDepCollector>( + Service, std::move(DepOutputOpts), ScanInstance, Consumer, Controller, + Inv, std::move(PrebuiltModulesASTMap), StableDirs); + ScanInstance.addDependencyCollector(MDC); return MDC; } @@ -804,9 +767,8 @@ bool DependencyScanningAction::runInvocation( auto DepOutputOpts = createDependencyOutputOptions(*OriginalInvocation); MDC = initializeScanInstanceDependencyCollector( - ScanInstance, std::move(DepOutputOpts), WorkingDirectory, Consumer, - Service, *OriginalInvocation, Controller, *MaybePrebuiltModulesASTMap, - StableDirs); + ScanInstance, std::move(DepOutputOpts), Consumer, Service, + *OriginalInvocation, Controller, *MaybePrebuiltModulesASTMap, StableDirs); if (ScanInstance.getDiagnostics().hasErrorOccurred()) return false; diff --git a/clang/lib/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/DependencyScanning/ModuleDepCollector.cpp index f9bc4cc3098ef..127b26bf2e0f7 100644 --- a/clang/lib/DependencyScanning/ModuleDepCollector.cpp +++ b/clang/lib/DependencyScanning/ModuleDepCollector.cpp @@ -551,6 +551,13 @@ void ModuleDepCollectorPP::LexedFileChanged(FileID FID, MDC.addFileDep(llvm::sys::path::remove_leading_dotslash(*Filename)); } +void ModuleDepCollectorPP::HasInclude(SourceLocation Loc, StringRef FileName, + bool IsAngled, OptionalFileEntryRef File, + SrcMgr::CharacteristicKind FileType) { + if (File) + MDC.addFileDep(File->getName()); +} + void ModuleDepCollectorPP::InclusionDirective( SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName, bool IsAngled, CharSourceRange FilenameRange, OptionalFileEntryRef File, diff --git a/clang/lib/Tooling/DependencyScanningTool.cpp b/clang/lib/Tooling/DependencyScanningTool.cpp index 1d4ab579f5827..d9d368c6ffde3 100644 --- a/clang/lib/Tooling/DependencyScanningTool.cpp +++ b/clang/lib/Tooling/DependencyScanningTool.cpp @@ -38,14 +38,20 @@ class MakeDependencyPrinterConsumer : public DependencyConsumer { } void handleFileDependency(StringRef File) override { - Dependencies.push_back(std::string(File)); + SmallString<128> NormalizedFile = File; + llvm::sys::path::remove_dots(NormalizedFile, /*remove_dot_dot=*/true); + Dependencies.emplace_back(NormalizedFile.str()); } // These are ignored for the make format as it can't support the full // set of deps, and handleFileDependency handles enough for implicitly // built modules to work. void handlePrebuiltModuleDependency(PrebuiltModuleDep PMD) override {} - void handleModuleDependency(ModuleDeps MD) override {} + void handleModuleDependency(ModuleDeps MD) override { + MD.forEachFileDep([this](StringRef File) { + DependenciesFromModules.push_back(std::string(File)); + }); + } void handleDirectModuleDependency(ModuleID ID) override {} void handleVisibleModule(std::string ModuleName) override {} void handleContextHash(std::string Hash) override {} @@ -56,10 +62,13 @@ class MakeDependencyPrinterConsumer : public DependencyConsumer { class DependencyPrinter : public DependencyFileGenerator { public: DependencyPrinter(DependencyOutputOptions &Opts, - ArrayRef<std::string> Dependencies) + ArrayRef<std::string> Dependencies, + ArrayRef<std::string> ModuleDependencies) : DependencyFileGenerator(Opts) { for (const auto &Dep : Dependencies) addDependency(Dep); + for (const auto &Dep : ModuleDependencies) + addDependency(Dep); } void printDependencies(std::string &S) { @@ -68,13 +77,14 @@ class MakeDependencyPrinterConsumer : public DependencyConsumer { } }; - DependencyPrinter Generator(*Opts, Dependencies); + DependencyPrinter Generator(*Opts, Dependencies, DependenciesFromModules); Generator.printDependencies(S); } protected: std::unique_ptr<DependencyOutputOptions> Opts; std::vector<std::string> Dependencies; + std::vector<std::string> DependenciesFromModules; }; } // anonymous namespace @@ -187,12 +197,12 @@ bool tooling::computeDependencies( Controller, DiagConsumer, OverlayFS); } -std::optional<std::string> -DependencyScanningTool::getDependencyFile(ArrayRef<std::string> CommandLine, - StringRef CWD, - DiagnosticConsumer &DiagConsumer) { +std::optional<std::string> DependencyScanningTool::getDependencyFile( + ArrayRef<std::string> CommandLine, StringRef CWD, + LookupModuleOutputCallback LookupModuleOutput, + DiagnosticConsumer &DiagConsumer) { MakeDependencyPrinterConsumer DepConsumer; - CallbackActionController Controller(nullptr); + CallbackActionController Controller(LookupModuleOutput); if (!computeDependencies(Worker, CWD, CommandLine, DepConsumer, Controller, DiagConsumer)) return std::nullopt; @@ -540,7 +550,7 @@ bool CompilerInstanceWithContext::computeDependencies( }); auto MDC = initializeScanInstanceDependencyCollector( - CI, std::make_unique<DependencyOutputOptions>(*OutputOpts), CWD, Consumer, + CI, std::make_unique<DependencyOutputOptions>(*OutputOpts), Consumer, Worker.Service, /* The MDC's constructor makes a copy of the OriginalInvocation, so we can pass it in without worrying that it might be changed across diff --git a/clang/test/ClangScanDeps/modules-cc1.cpp b/clang/test/ClangScanDeps/modules-cc1.cpp index 04a365249f379..28fc020847d56 100644 --- a/clang/test/ClangScanDeps/modules-cc1.cpp +++ b/clang/test/ClangScanDeps/modules-cc1.cpp @@ -16,7 +16,7 @@ module header1 { header "header.h" } [{ "file": "DIR/modules_cc1.cpp", "directory": "DIR", - "command": "clang -cc1 DIR/modules_cc1.cpp -fimplicit-module-maps -o modules_cc1.o" + "command": "clang -cc1 DIR/modules_cc1.cpp -fmodules -fmodules-cache-path=DIR/cache -fimplicit-module-maps -o modules_cc1.o" }] // RUN: sed "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json diff --git a/clang/test/ClangScanDeps/modules-has-include-umbrella-header.c b/clang/test/ClangScanDeps/modules-has-include-umbrella-header.c index 022c59ca65db2..f7f804794ab41 100644 --- a/clang/test/ClangScanDeps/modules-has-include-umbrella-header.c +++ b/clang/test/ClangScanDeps/modules-has-include-umbrella-header.c @@ -65,7 +65,8 @@ module Dependency { header "dependency.h" } // CHECK-NEXT: "command-line": [ // CHECK: ], // CHECK: "file-deps": [ -// CHECK-NEXT: "[[PREFIX]]/tu.c" +// CHECK-NEXT: "[[PREFIX]]/tu.c", +// CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/PrivateHeaders/B.h" // CHECK-NEXT: ], // CHECK-NEXT: "input-file": "[[PREFIX]]/tu.c" // CHECK-NEXT: } diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp index 1d80ac519bb20..8944c5fc48e30 100644 --- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp +++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp @@ -1010,8 +1010,8 @@ int clang_scan_deps_main(int argc, char **argv, const llvm::ToolContext &) { // Run the tool on it. if (Format == ScanningOutputFormat::Make) { - auto MaybeFile = - WorkerTool.getDependencyFile(Input->CommandLine, CWD, DiagConsumer); + auto MaybeFile = WorkerTool.getDependencyFile( + Input->CommandLine, CWD, LookupOutput, DiagConsumer); handleDiagnostics(Filename, S, Errs); if (MaybeFile) DependencyOS.applyLocked([&](raw_ostream &OS) { OS << *MaybeFile; }); diff --git a/clang/unittests/Tooling/DependencyScannerTest.cpp b/clang/unittests/Tooling/DependencyScannerTest.cpp index 79fd5a312d2b9..86d4e0ee1b8c5 100644 --- a/clang/unittests/Tooling/DependencyScannerTest.cpp +++ b/clang/unittests/Tooling/DependencyScannerTest.cpp @@ -235,8 +235,9 @@ TEST(DependencyScanner, ScanDepsWithFS) { DependencyScanningTool ScanTool(Service); TextDiagnosticBuffer DiagConsumer; - std::optional<std::string> DepFile = - ScanTool.getDependencyFile(CommandLine, CWD, DiagConsumer); + std::optional<std::string> DepFile = ScanTool.getDependencyFile( + CommandLine, CWD, CallbackActionController::lookupUnreachableModuleOutput, + DiagConsumer); ASSERT_TRUE(DepFile.has_value()); EXPECT_EQ(llvm::sys::path::convert_to_slash(*DepFile), "test.cpp.o: /root/test.cpp /root/header.h\n"); @@ -297,8 +298,9 @@ TEST(DependencyScanner, ScanDepsWithModuleLookup) { // matter, the point of the test is to check that files are not read // unnecessarily. TextDiagnosticBuffer DiagConsumer; - std::optional<std::string> DepFile = - ScanTool.getDependencyFile(CommandLine, CWD, DiagConsumer); + std::optional<std::string> DepFile = ScanTool.getDependencyFile( + CommandLine, CWD, CallbackActionController::lookupUnreachableModuleOutput, + DiagConsumer); ASSERT_FALSE(DepFile.has_value()); EXPECT_TRUE(!llvm::is_contained(InterceptFS->StatPaths, OtherPath)); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
