llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Jan Svoboda (jansvoboda11) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/182063.diff 10 Files Affected: - (modified) clang/include/clang/DependencyScanning/DependencyScannerImpl.h (-12) - (modified) clang/include/clang/DependencyScanning/ModuleDepCollector.h (+3) - (modified) clang/include/clang/Tooling/DependencyScanningTool.h (+1) - (modified) clang/lib/DependencyScanning/DependencyScannerImpl.cpp (+14-52) - (modified) clang/lib/DependencyScanning/ModuleDepCollector.cpp (+8-4) - (modified) clang/lib/Tooling/DependencyScanningTool.cpp (+17-9) - (modified) clang/test/ClangScanDeps/modules-cc1.cpp (+1-1) - (modified) clang/test/ClangScanDeps/modules-has-include-umbrella-header.c (+2-1) - (modified) clang/tools/clang-scan-deps/ClangScanDeps.cpp (+2-2) - (modified) clang/unittests/Tooling/DependencyScannerTest.cpp (+6-4) ``````````diff diff --git a/clang/include/clang/DependencyScanning/DependencyScannerImpl.h b/clang/include/clang/DependencyScanning/DependencyScannerImpl.h index d50371512667d..3d55759d60d83 100644 --- a/clang/include/clang/DependencyScanning/DependencyScannerImpl.h +++ b/clang/include/clang/DependencyScanning/DependencyScannerImpl.h @@ -106,18 +106,6 @@ std::optional<PrebuiltModulesAttrsMap> computePrebuiltModulesASTMap(CompilerInstance &ScanInstance, SmallVector<StringRef> &StableDirs); -/// Create the dependency collector that will collect the produced -/// dependencies. May return the created ModuleDepCollector depending -/// on the scanning format. -std::shared_ptr<ModuleDepCollector> initializeScanInstanceDependencyCollector( - CompilerInstance &ScanInstance, - std::unique_ptr<DependencyOutputOptions> DepOutputOpts, - StringRef WorkingDirectory, DependencyConsumer &Consumer, - DependencyScanningService &Service, CompilerInvocation &Inv, - DependencyActionController &Controller, - PrebuiltModulesAttrsMap PrebuiltModulesASTMap, - llvm::SmallVector<StringRef> &StableDirs); - class CompilerInstanceWithContext { // Context DependencyScanningWorker &Worker; diff --git a/clang/include/clang/DependencyScanning/ModuleDepCollector.h b/clang/include/clang/DependencyScanning/ModuleDepCollector.h index 8f665daf03c69..2a581d72274f1 100644 --- a/clang/include/clang/DependencyScanning/ModuleDepCollector.h +++ b/clang/include/clang/DependencyScanning/ModuleDepCollector.h @@ -225,6 +225,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 96d21866dd7f9..5a94025191513 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 861e6e0042265..8d580012aae78 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 @@ -527,31 +503,18 @@ createDependencyOutputOptions(const CompilerInvocation &Invocation) { return Opts; } -std::shared_ptr<ModuleDepCollector> -dependencies::initializeScanInstanceDependencyCollector( +static 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) { - 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; } @@ -790,9 +753,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); std::unique_ptr<FrontendAction> Action; @@ -901,7 +863,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/lib/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/DependencyScanning/ModuleDepCollector.cpp index 46a0f79bfd38e..ef56c894657e6 100644 --- a/clang/lib/DependencyScanning/ModuleDepCollector.cpp +++ b/clang/lib/DependencyScanning/ModuleDepCollector.cpp @@ -549,6 +549,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, @@ -950,12 +957,9 @@ void ModuleDepCollector::addVisibleModules() { static StringRef makeAbsoluteAndPreferred(CompilerInstance &CI, StringRef Path, SmallVectorImpl<char> &Storage) { - if (llvm::sys::path::is_absolute(Path) && - !llvm::sys::path::is_style_windows(llvm::sys::path::Style::native)) - return Path; Storage.assign(Path.begin(), Path.end()); CI.getFileManager().makeAbsolutePath(Storage); - llvm::sys::path::make_preferred(Storage); + llvm::sys::path::remove_dots(Storage, /*remove_dot_dot=*/true); return StringRef(Storage.data(), Storage.size()); } diff --git a/clang/lib/Tooling/DependencyScanningTool.cpp b/clang/lib/Tooling/DependencyScanningTool.cpp index 2ae149e8fb2cf..56f4e56fd13a1 100644 --- a/clang/lib/Tooling/DependencyScanningTool.cpp +++ b/clang/lib/Tooling/DependencyScanningTool.cpp @@ -40,7 +40,11 @@ class MakeDependencyPrinterConsumer : public DependencyConsumer { // 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) { + ModuleDependencies.push_back(std::string(File)); + }); + } void handleDirectModuleDependency(ModuleID ID) override {} void handleVisibleModule(std::string ModuleName) override {} void handleContextHash(std::string Hash) override {} @@ -51,10 +55,13 @@ class MakeDependencyPrinterConsumer : public DependencyConsumer { class DependencyPrinter : public DependencyFileGenerator { public: DependencyPrinter(DependencyOutputOptions &Opts, - ArrayRef<std::string> Dependencies) - : DependencyFileGenerator(Opts) { + 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) { @@ -63,13 +70,14 @@ class MakeDependencyPrinterConsumer : public DependencyConsumer { } }; - DependencyPrinter Generator(*Opts, Dependencies); + DependencyPrinter Generator(*Opts, Dependencies, ModuleDependencies); Generator.printDependencies(S); } protected: std::unique_ptr<DependencyOutputOptions> Opts; std::vector<std::string> Dependencies; + std::vector<std::string> ModuleDependencies; }; } // anonymous namespace @@ -182,12 +190,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; 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 30fb8d47f8d5d..7f909ae34e595 100644 --- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp +++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp @@ -1009,8 +1009,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)); `````````` </details> https://github.com/llvm/llvm-project/pull/182063 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
