https://github.com/cyndyishida updated https://github.com/llvm/llvm-project/pull/147969
>From 87900ca81b552c9d7718c8a38a21f53260534c6e Mon Sep 17 00:00:00 2001 From: Cyndy Ishida <cyndy_ish...@apple.com> Date: Wed, 9 Jul 2025 21:50:44 -0700 Subject: [PATCH 1/2] [clang][scan-deps] Report a scanned TU's visible modules --- .../DependencyScanningTool.h | 12 +- .../DependencyScanningWorker.h | 2 + .../DependencyScanning/ModuleDepCollector.h | 8 ++ .../DependencyScanningTool.cpp | 22 +--- .../DependencyScanning/ModuleDepCollector.cpp | 32 ++++- clang/test/ClangScanDeps/visible-modules.c | 116 ++++++++++++++++++ clang/tools/clang-scan-deps/ClangScanDeps.cpp | 32 ++++- clang/tools/clang-scan-deps/Opts.td | 3 + 8 files changed, 201 insertions(+), 26 deletions(-) create mode 100644 clang/test/ClangScanDeps/visible-modules.c diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h index ee24e5d1543d3..c3601a4e73e1f 100644 --- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h +++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h @@ -57,6 +57,10 @@ struct TranslationUnitDeps { /// determined that the differences are benign for this compilation. std::vector<ModuleID> ClangModuleDeps; + /// A list of module names that are visible to this translation unit. This + /// includes both direct and transitive module dependencies. + std::vector<std::string> VisibleModules; + /// A list of the C++20 named modules this translation unit depends on. std::vector<std::string> NamedModuleDeps; @@ -150,7 +154,7 @@ class DependencyScanningTool { /// Given a compilation context specified via the Clang driver command-line, /// gather modular dependencies of module with the given name, and return the /// information needed for explicit build. - llvm::Expected<ModuleDepsGraph> getModuleDependencies( + llvm::Expected<TranslationUnitDeps> getModuleDependencies( StringRef ModuleName, const std::vector<std::string> &CommandLine, StringRef CWD, const llvm::DenseSet<ModuleID> &AlreadySeen, LookupModuleOutputCallback LookupModuleOutput); @@ -188,6 +192,10 @@ class FullDependencyConsumer : public DependencyConsumer { DirectModuleDeps.push_back(ID); } + void handleVisibleModule(std::string ModuleName) override { + VisibleModules.push_back(ModuleName); + } + void handleContextHash(std::string Hash) override { ContextHash = std::move(Hash); } @@ -201,7 +209,6 @@ class FullDependencyConsumer : public DependencyConsumer { } TranslationUnitDeps takeTranslationUnitDeps(); - ModuleDepsGraph takeModuleGraphDeps(); private: std::vector<std::string> Dependencies; @@ -210,6 +217,7 @@ class FullDependencyConsumer : public DependencyConsumer { std::string ModuleName; std::vector<std::string> NamedModuleDeps; std::vector<ModuleID> DirectModuleDeps; + std::vector<std::string> VisibleModules; std::vector<Command> Commands; std::string ContextHash; std::vector<std::string> OutputPaths; diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h index 3e232c79397ce..6060e4b43312e 100644 --- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h +++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h @@ -59,6 +59,8 @@ class DependencyConsumer { virtual void handleDirectModuleDependency(ModuleID MD) = 0; + virtual void handleVisibleModule(std::string ModuleName) = 0; + virtual void handleContextHash(std::string Hash) = 0; }; diff --git a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h index e96c49883d3c6..4136cb73f7043 100644 --- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h +++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h @@ -323,6 +323,11 @@ class ModuleDepCollector final : public DependencyCollector { llvm::MapVector<const Module *, PrebuiltModuleDep> DirectPrebuiltModularDeps; /// Working set of direct modular dependencies. llvm::SetVector<const Module *> DirectModularDeps; + /// Working set of direct modular dependencies, as they were imported. + llvm::SmallPtrSet<const Module *, 32> DirectImports; + /// All direct and transitive visible modules. + llvm::StringSet<> VisibleModules; + /// Options that control the dependency output generation. std::unique_ptr<DependencyOutputOptions> Opts; /// A Clang invocation that's based on the original TU invocation and that has @@ -337,6 +342,9 @@ class ModuleDepCollector final : public DependencyCollector { /// Checks whether the module is known as being prebuilt. bool isPrebuiltModule(const Module *M); + /// Computes all visible modules resolved from direct imports. + void addVisibleModules(); + /// Adds \p Path to \c FileDeps, making it absolute if necessary. void addFileDep(StringRef Path); /// Adds \p Path to \c MD.FileDeps, making it absolute if necessary. diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp index 515211d47b348..27734ffd0e20b 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp @@ -40,6 +40,7 @@ class MakeDependencyPrinterConsumer : public DependencyConsumer { void handlePrebuiltModuleDependency(PrebuiltModuleDep PMD) override {} void handleModuleDependency(ModuleDeps MD) override {} void handleDirectModuleDependency(ModuleID ID) override {} + void handleVisibleModule(std::string ModuleName) override {} void handleContextHash(std::string Hash) override {} void printDependencies(std::string &S) { @@ -154,7 +155,8 @@ DependencyScanningTool::getTranslationUnitDependencies( return Consumer.takeTranslationUnitDeps(); } -llvm::Expected<ModuleDepsGraph> DependencyScanningTool::getModuleDependencies( +llvm::Expected<TranslationUnitDeps> +DependencyScanningTool::getModuleDependencies( StringRef ModuleName, const std::vector<std::string> &CommandLine, StringRef CWD, const llvm::DenseSet<ModuleID> &AlreadySeen, LookupModuleOutputCallback LookupModuleOutput) { @@ -164,7 +166,7 @@ llvm::Expected<ModuleDepsGraph> DependencyScanningTool::getModuleDependencies( Controller, ModuleName); if (Result) return std::move(Result); - return Consumer.takeModuleGraphDeps(); + return Consumer.takeTranslationUnitDeps(); } TranslationUnitDeps FullDependencyConsumer::takeTranslationUnitDeps() { @@ -175,6 +177,7 @@ TranslationUnitDeps FullDependencyConsumer::takeTranslationUnitDeps() { TU.NamedModuleDeps = std::move(NamedModuleDeps); TU.FileDeps = std::move(Dependencies); TU.PrebuiltModuleDeps = std::move(PrebuiltModuleDeps); + TU.VisibleModules = std::move(VisibleModules); TU.Commands = std::move(Commands); for (auto &&M : ClangModuleDeps) { @@ -190,19 +193,4 @@ TranslationUnitDeps FullDependencyConsumer::takeTranslationUnitDeps() { return TU; } -ModuleDepsGraph FullDependencyConsumer::takeModuleGraphDeps() { - ModuleDepsGraph ModuleGraph; - - for (auto &&M : ClangModuleDeps) { - auto &MD = M.second; - // TODO: Avoid handleModuleDependency even being called for modules - // we've already seen. - if (AlreadySeen.count(M.first)) - continue; - ModuleGraph.push_back(std::move(MD)); - } - - return ModuleGraph; -} - CallbackActionController::~CallbackActionController() {} diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp index fa86d714ff69a..37f8b945d785e 100644 --- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp +++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp @@ -673,8 +673,10 @@ void ModuleDepCollectorPP::handleImport(const Module *Imported) { if (MDC.isPrebuiltModule(TopLevelModule)) MDC.DirectPrebuiltModularDeps.insert( {TopLevelModule, PrebuiltModuleDep{TopLevelModule}}); - else + else { MDC.DirectModularDeps.insert(TopLevelModule); + MDC.DirectImports.insert(Imported); + } } void ModuleDepCollectorPP::EndOfMainFile() { @@ -706,6 +708,8 @@ void ModuleDepCollectorPP::EndOfMainFile() { if (!MDC.isPrebuiltModule(M)) MDC.DirectModularDeps.insert(M); + MDC.addVisibleModules(); + for (const Module *M : MDC.DirectModularDeps) handleTopLevelModule(M); @@ -727,6 +731,9 @@ void ModuleDepCollectorPP::EndOfMainFile() { MDC.Consumer.handleDirectModuleDependency(It->second->ID); } + for (auto &&I : MDC.VisibleModules) + MDC.Consumer.handleVisibleModule(std::string(I.getKey())); + for (auto &&I : MDC.FileDeps) MDC.Consumer.handleFileDependency(I); @@ -993,6 +1000,29 @@ bool ModuleDepCollector::isPrebuiltModule(const Module *M) { return true; } +void ModuleDepCollector::addVisibleModules() { + llvm::DenseSet<const Module *> ImportedModules; + auto InsertVisibleModules = [&](const Module *M) { + if (ImportedModules.contains(M)) + return; + + VisibleModules.insert(M->getTopLevelModuleName()); + SmallVector<Module *> Stack; + M->getExportedModules(Stack); + while (!Stack.empty()) { + const Module *CurrModule = Stack.pop_back_val(); + if (ImportedModules.contains(CurrModule)) + continue; + ImportedModules.insert(CurrModule); + VisibleModules.insert(CurrModule->getTopLevelModuleName()); + CurrModule->getExportedModules(Stack); + } + }; + + for (const Module *Import : DirectImports) + InsertVisibleModules(Import); +} + static StringRef makeAbsoluteAndPreferred(CompilerInstance &CI, StringRef Path, SmallVectorImpl<char> &Storage) { if (llvm::sys::path::is_absolute(Path) && diff --git a/clang/test/ClangScanDeps/visible-modules.c b/clang/test/ClangScanDeps/visible-modules.c new file mode 100644 index 0000000000000..ac394a9925267 --- /dev/null +++ b/clang/test/ClangScanDeps/visible-modules.c @@ -0,0 +1,116 @@ +// This test verifies that the modules visible to the translation unit are computed in dependency scanning. +// "client" in the first scan represents the translation unit that imports an explicit submodule, +// that only exports one other module. +// In the second scan, the translation unit that imports an explicit submodule, +// that exports an additional module. +// Thus, the dependencies of the top level module for the submodule always differ from what is visible to the TU. + +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: sed -e "s|DIR|%/t|g" %t/compile-commands.json.in > %t/compile-commands.json +// RUN: clang-scan-deps -emit-visible-modules -compilation-database %t/compile-commands.json \ +// RUN: -j 1 -format experimental-full 2>&1 > %t/result-first-scan.json +// RUN: cat %t/result-first-scan.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t --check-prefix=SINGLE + +/// Re-run scan with different module map for direct dependency. +// RUN: mv %t/A_without_visible_export.modulemap %t/Sysroot/usr/include/A/module.modulemap +// RUN: clang-scan-deps -emit-visible-modules -compilation-database %t/compile-commands.json \ +// RUN: -j 1 -format experimental-full 2>&1 > %t/result.json +// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t --check-prefix=MULTIPLE + +// RUN: %deps-to-rsp %t/result.json --module-name=transitive > %t/transitive.rsp +// RUN: %deps-to-rsp %t/result.json --module-name=visible > %t/visible.rsp +// RUN: %deps-to-rsp %t/result.json --module-name=invisible > %t/invisible.rsp +// RUN: %deps-to-rsp %t/result.json --module-name=A > %t/A.rsp +// RUN: %deps-to-rsp %t/result.json --tu-index=0 > %t/tu.rsp + +// RUN: %clang @%t/transitive.rsp +// RUN: %clang @%t/visible.rsp +// RUN: %clang @%t/invisible.rsp +// RUN: %clang @%t/A.rsp + +/// Verify compilation & scan agree with each other. +// RUN: not %clang @%t/tu.rsp 2>&1 | FileCheck %s --check-prefix=COMPILE + +// SINGLE: "visible-clang-modules": [ +// SINGLE-NEXT: "A" +// SINGLE-NEXT: ] + +// MULTIPLE: "visible-clang-modules": [ +// MULTIPLE-NEXT: "A", +// MULTIPLE-NEXT: "visible" +// MULTIPLE-NEXT: ] + +// COMPILE-NOT: 'visible_t' must be declared before it is used +// COMPILE: 'transitive_t' must be declared before it is used +// COMPILE: 'invisible_t' must be declared before it is used + +//--- compile-commands.json.in +[ +{ + "directory": "DIR", + "command": "clang -c DIR/client.c -isysroot DIR/Sysroot -IDIR/Sysroot/usr/include -fmodules -fmodules-cache-path=DIR/module-cache -fimplicit-module-maps", + "file": "DIR/client.c" +} +] + +//--- Sysroot/usr/include/A/module.modulemap +module A { + explicit module visibleToTU { + header "visibleToTU.h" + } + explicit module invisibleToTU { + header "invisibleToTU.h" + } +} + +//--- A_without_visible_export.modulemap +module A { + explicit module visibleToTU { + header "visibleToTU.h" + export visible + } + explicit module invisibleToTU { + header "invisibleToTU.h" + } +} + +//--- Sysroot/usr/include/A/visibleToTU.h +#include <visible/visible.h> +typedef int A_visibleToTU; + +//--- Sysroot/usr/include/A/invisibleToTU.h +#include <invisible/invisible.h> +typedef int A_invisibleToTU; + +//--- Sysroot/usr/include/invisible/module.modulemap +module invisible { + umbrella "." +} + +//--- Sysroot/usr/include/invisible/invisible.h +typedef int invisible_t; + +//--- Sysroot/usr/include/visible/module.modulemap +module visible { + umbrella "." +} + +//--- Sysroot/usr/include/visible/visible.h +#include <transitive/transitive.h> +typedef int visible_t; + +//--- Sysroot/usr/include/transitive/module.modulemap +module transitive { + umbrella "." +} + +//--- Sysroot/usr/include/transitive/transitive.h +typedef int transitive_t; + +//--- client.c +#include <A/visibleToTU.h> +visible_t foo_v(void); +// Both decls are not visible, thus should fail to actually compile. +transitive_t foo_t(void); +invisible_t foo_i(void); diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp index 8b590bd57e1a3..f10b73278381b 100644 --- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp +++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp @@ -94,6 +94,7 @@ static bool DeprecatedDriverCommand; static ResourceDirRecipeKind ResourceDirRecipe; static bool Verbose; static bool PrintTiming; +static bool EmitVisibleModules; static llvm::BumpPtrAllocator Alloc; static llvm::StringSaver Saver{Alloc}; static std::vector<const char *> CommandLine; @@ -232,6 +233,8 @@ static void ParseArgs(int argc, char **argv) { PrintTiming = Args.hasArg(OPT_print_timing); + EmitVisibleModules = Args.hasArg(OPT_emit_visible_modules); + Verbose = Args.hasArg(OPT_verbose); RoundTripArgs = Args.hasArg(OPT_round_trip_args); @@ -380,6 +383,14 @@ static auto toJSONSorted(llvm::json::OStream &JOS, }; } +static auto toJSONSorted(llvm::json::OStream &JOS, std::vector<std::string> V) { + llvm::sort(V); + return [&JOS, V = std::move(V)] { + for (const StringRef Entry : V) + JOS.value(Entry); + }; +} + // Thread safe. class FullDeps { public: @@ -396,6 +407,7 @@ class FullDeps { ID.NamedModule = std::move(TUDeps.ID.ModuleName); ID.NamedModuleDeps = std::move(TUDeps.NamedModuleDeps); ID.ClangModuleDeps = std::move(TUDeps.ClangModuleDeps); + ID.VisibleModules = std::move(TUDeps.VisibleModules); ID.DriverCommandLine = std::move(TUDeps.DriverCommandLine); ID.Commands = std::move(TUDeps.Commands); @@ -525,6 +537,9 @@ class FullDeps { JOS.attributeArray("file-deps", toJSONStrings(JOS, I.FileDeps)); JOS.attribute("input-file", StringRef(I.FileName)); + if (EmitVisibleModules) + JOS.attributeArray("visible-clang-modules", + toJSONSorted(JOS, I.VisibleModules)); }); } } else { @@ -545,6 +560,9 @@ class FullDeps { JOS.attributeArray("file-deps", toJSONStrings(JOS, I.FileDeps)); JOS.attribute("input-file", StringRef(I.FileName)); + if (EmitVisibleModules) + JOS.attributeArray("visible-clang-modules", + toJSONSorted(JOS, I.VisibleModules)); }); } }); @@ -596,6 +614,7 @@ class FullDeps { std::string NamedModule; std::vector<std::string> NamedModuleDeps; std::vector<ModuleID> ClangModuleDeps; + std::vector<std::string> VisibleModules; std::vector<std::string> DriverCommandLine; std::vector<Command> Commands; }; @@ -623,11 +642,12 @@ static bool handleTranslationUnitResult( return false; } -static bool handleModuleResult( - StringRef ModuleName, llvm::Expected<ModuleDepsGraph> &MaybeModuleGraph, - FullDeps &FD, size_t InputIndex, SharedStream &OS, SharedStream &Errs) { - if (!MaybeModuleGraph) { - llvm::handleAllErrors(MaybeModuleGraph.takeError(), +static bool handleModuleResult(StringRef ModuleName, + llvm::Expected<TranslationUnitDeps> &MaybeTUDeps, + FullDeps &FD, size_t InputIndex, + SharedStream &OS, SharedStream &Errs) { + if (!MaybeTUDeps) { + llvm::handleAllErrors(MaybeTUDeps.takeError(), [&ModuleName, &Errs](llvm::StringError &Err) { Errs.applyLocked([&](raw_ostream &OS) { OS << "Error while scanning dependencies for " @@ -637,7 +657,7 @@ static bool handleModuleResult( }); return true; } - FD.mergeDeps(std::move(*MaybeModuleGraph), InputIndex); + FD.mergeDeps(std::move(MaybeTUDeps->ModuleGraph), InputIndex); return false; } diff --git a/clang/tools/clang-scan-deps/Opts.td b/clang/tools/clang-scan-deps/Opts.td index 9cccbb3aaf0c8..03011f9ae1f75 100644 --- a/clang/tools/clang-scan-deps/Opts.td +++ b/clang/tools/clang-scan-deps/Opts.td @@ -37,6 +37,9 @@ defm resource_dir_recipe : Eq<"resource-dir-recipe", "How to produce missing '-r def print_timing : F<"print-timing", "Print timing information">; +def emit_visible_modules + : F<"emit-visible-modules", "emit visible modules in primary output">; + def verbose : F<"v", "Use verbose output">; def round_trip_args : F<"round-trip-args", "verify that command-line arguments are canonical by parsing and re-serializing">; >From 525f4ef40cfeb0dd58ee43d9db0257315cce7d23 Mon Sep 17 00:00:00 2001 From: Cyndy Ishida <cyndy_ish...@apple.com> Date: Fri, 11 Jul 2025 08:00:35 -0700 Subject: [PATCH 2/2] Update clang/test/ClangScanDeps/visible-modules.c Co-authored-by: Jan Svoboda <j...@svoboda.ai> --- clang/test/ClangScanDeps/visible-modules.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/test/ClangScanDeps/visible-modules.c b/clang/test/ClangScanDeps/visible-modules.c index ac394a9925267..77716a4956f00 100644 --- a/clang/test/ClangScanDeps/visible-modules.c +++ b/clang/test/ClangScanDeps/visible-modules.c @@ -13,7 +13,7 @@ // RUN: cat %t/result-first-scan.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t --check-prefix=SINGLE /// Re-run scan with different module map for direct dependency. -// RUN: mv %t/A_without_visible_export.modulemap %t/Sysroot/usr/include/A/module.modulemap +// RUN: mv %t/A_with_visible_export.modulemap %t/Sysroot/usr/include/A/module.modulemap // RUN: clang-scan-deps -emit-visible-modules -compilation-database %t/compile-commands.json \ // RUN: -j 1 -format experimental-full 2>&1 > %t/result.json // RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t --check-prefix=MULTIPLE @@ -64,7 +64,7 @@ module A { } } -//--- A_without_visible_export.modulemap +//--- A_with_visible_export.modulemap module A { explicit module visibleToTU { header "visibleToTU.h" _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits