From 46288ffafa2ec3e9b85d567fae1568448e3f3863 Mon Sep 17 00:00:00 2001 From: Volodymyr Sapsai Date: Mon, 22 Sep 2025 21:01:00 -0700 Subject: [PATCH 1/7] [clang][deps] Add module map describing compiled module to file dependencies. When we add the module map describing the compiled module to the command line, add it to the file dependencies as well. Discovered while working on reproducers where a command line input was missing in the captured files as it wasn't considered a dependency. --- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp | 4 +++- .../test/ClangScanDeps/modules-fmodule-name-no-module-built.m | 3 ++- clang/test/ClangScanDeps/modules-header-sharing.m | 3 ++- clang/test/ClangScanDeps/modules-implementation-module-map.c | 3 ++- clang/test/ClangScanDeps/modules-implementation-private.m | 3 ++- 5 files changed, 11 insertions(+), 5 deletions(-) diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp index d67178c153e88..9109ce789ee4c 100644 --- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp +++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp @@ -444,9 +444,11 @@ void ModuleDepCollector::applyDiscoveredDependencies(CompilerInvocation &CI) { if (OptionalFileEntryRef CurrentModuleMap = PP.getHeaderSearchInfo() .getModuleMap() - .getModuleMapFileForUniquing(CurrentModule)) + .getModuleMapFileForUniquing(CurrentModule)) { CI.getFrontendOpts().ModuleMapFiles.emplace_back( CurrentModuleMap->getNameAsRequested()); + Consumer.handleFileDependency(CurrentModuleMap->getNameAsRequested()); + } SmallVector DirectDeps; for (const auto &KV : ModularDeps) diff --git a/clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m b/clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m index cfe29c2bf7cdb..1fdcbc51c8e1f 100644 --- a/clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m +++ b/clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m @@ -46,7 +46,8 @@ // CHECK: "file-deps": [ // CHECK-NEXT: "[[PREFIX]]/modules-fmodule-name-no-module-built.m", // CHECK-NEXT: "[[PREFIX]]/Inputs/header3.h", -// CHECK-NEXT: "[[PREFIX]]/Inputs/header.h" +// CHECK-NEXT: "[[PREFIX]]/Inputs/header.h", +// CHECK-NEXT: "Inputs/module.modulemap" // CHECK-NEXT: ], // CHECK-NEXT: "input-file": "[[PREFIX]]/modules-fmodule-name-no-module-built.m" // CHECK-NEXT: } diff --git a/clang/test/ClangScanDeps/modules-header-sharing.m b/clang/test/ClangScanDeps/modules-header-sharing.m index 31ef351ec38b7..005b22dc6902a 100644 --- a/clang/test/ClangScanDeps/modules-header-sharing.m +++ b/clang/test/ClangScanDeps/modules-header-sharing.m @@ -79,7 +79,8 @@ // CHECK: ], // CHECK: "file-deps": [ // CHECK-NEXT: "[[PREFIX]]/tu.m", -// CHECK-NEXT: "[[PREFIX]]/shared/H.h" +// CHECK-NEXT: "[[PREFIX]]/shared/H.h", +// CHECK-NEXT: "[[PREFIX]]/frameworks/A.framework/Modules/module.modulemap" // CHECK-NEXT: ], // CHECK-NEXT: "input-file": "[[PREFIX]]/tu.m" // CHECK-NEXT: } diff --git a/clang/test/ClangScanDeps/modules-implementation-module-map.c b/clang/test/ClangScanDeps/modules-implementation-module-map.c index b7637d0c9143a..a7170aab2448c 100644 --- a/clang/test/ClangScanDeps/modules-implementation-module-map.c +++ b/clang/test/ClangScanDeps/modules-implementation-module-map.c @@ -28,7 +28,8 @@ framework module FWPrivate { header "private.h" } // CHECK: "-fmodule-name=FWPrivate", // CHECK: ], // CHECK: "file-deps": [ -// CHECK-NEXT: "[[PREFIX]]/tu.m" +// CHECK-NEXT: "[[PREFIX]]/tu.m", +// CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap" // CHECK-NEXT: ], // CHECK-NEXT: "input-file": "[[PREFIX]]/tu.m" // CHECK-NEXT: } diff --git a/clang/test/ClangScanDeps/modules-implementation-private.m b/clang/test/ClangScanDeps/modules-implementation-private.m index b376073f4b9ee..210fbfb424aca 100644 --- a/clang/test/ClangScanDeps/modules-implementation-private.m +++ b/clang/test/ClangScanDeps/modules-implementation-private.m @@ -65,7 +65,8 @@ // CHECK: "file-deps": [ // CHECK-NEXT: "[[PREFIX]]/tu.m", // CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/PrivateHeaders/Missed.h", -// CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/Headers/FW.h" +// CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/Headers/FW.h", +// CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap" // CHECK-NEXT: ], // CHECK-NEXT: "input-file": "[[PREFIX]]/tu.m" // CHECK-NEXT: } From 39022447f7e73ec7e25feda1db7540c2df33824d Mon Sep 17 00:00:00 2001 From: Volodymyr Sapsai Date: Mon, 6 Oct 2025 16:16:01 -0700 Subject: [PATCH 2/7] Address PR comments: move module map dependency reporting to `ModuleDepCollectorPP::EndOfMainFile`. --- .../Tooling/DependencyScanning/ModuleDepCollector.cpp | 11 ++++++++--- .../modules-fmodule-name-no-module-built.m | 2 +- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp index 0612ca21c7130..8c63a014da31f 100644 --- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp +++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp @@ -448,11 +448,9 @@ void ModuleDepCollector::applyDiscoveredDependencies(CompilerInvocation &CI) { if (OptionalFileEntryRef CurrentModuleMap = PP.getHeaderSearchInfo() .getModuleMap() - .getModuleMapFileForUniquing(CurrentModule)) { + .getModuleMapFileForUniquing(CurrentModule)) CI.getFrontendOpts().ModuleMapFiles.emplace_back( CurrentModuleMap->getNameAsRequested()); - Consumer.handleFileDependency(CurrentModuleMap->getNameAsRequested()); - } SmallVector DirectDeps; for (const auto &KV : ModularDeps) @@ -687,6 +685,13 @@ void ModuleDepCollectorPP::EndOfMainFile() { if (!MDC.ScanInstance.getPreprocessorOpts().ImplicitPCHInclude.empty()) MDC.addFileDep(MDC.ScanInstance.getPreprocessorOpts().ImplicitPCHInclude); + if (Module *CurrentModule = PP.getCurrentModuleImplementation()) { + if (OptionalFileEntryRef CurrentModuleMap = + PP.getHeaderSearchInfo().getModuleMap().getModuleMapFileForUniquing( + CurrentModule)) + MDC.addFileDep(CurrentModuleMap->getNameAsRequested()); + } + for (const Module *M : MDC.ScanInstance.getPreprocessor().getAffectingClangModules()) if (!MDC.isPrebuiltModule(M)) diff --git a/clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m b/clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m index 1fdcbc51c8e1f..f7edb011ad32a 100644 --- a/clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m +++ b/clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m @@ -47,7 +47,7 @@ // CHECK-NEXT: "[[PREFIX]]/modules-fmodule-name-no-module-built.m", // CHECK-NEXT: "[[PREFIX]]/Inputs/header3.h", // CHECK-NEXT: "[[PREFIX]]/Inputs/header.h", -// CHECK-NEXT: "Inputs/module.modulemap" +// CHECK-NEXT: "[[PREFIX]]/Inputs/module.modulemap" // CHECK-NEXT: ], // CHECK-NEXT: "input-file": "[[PREFIX]]/modules-fmodule-name-no-module-built.m" // CHECK-NEXT: } From f2e02155bdb076a9078ef474cb410f55acbd97c5 Mon Sep 17 00:00:00 2001 From: Volodymyr Sapsai Date: Tue, 7 Oct 2025 21:52:20 -0700 Subject: [PATCH 3/7] Add a test case showing that `FileEntryRef::getName` doesn't work sometimes. --- .../DependencyScanning/ModuleDepCollector.cpp | 2 +- .../modules-current-modulemap-file-dep.c | 55 +++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 clang/test/ClangScanDeps/modules-current-modulemap-file-dep.c diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp index 8c63a014da31f..0c3d4dc349b05 100644 --- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp +++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp @@ -689,7 +689,7 @@ void ModuleDepCollectorPP::EndOfMainFile() { if (OptionalFileEntryRef CurrentModuleMap = PP.getHeaderSearchInfo().getModuleMap().getModuleMapFileForUniquing( CurrentModule)) - MDC.addFileDep(CurrentModuleMap->getNameAsRequested()); + MDC.addFileDep(CurrentModuleMap->getName()); } for (const Module *M : diff --git a/clang/test/ClangScanDeps/modules-current-modulemap-file-dep.c b/clang/test/ClangScanDeps/modules-current-modulemap-file-dep.c new file mode 100644 index 0000000000000..dee568c3c8b77 --- /dev/null +++ b/clang/test/ClangScanDeps/modules-current-modulemap-file-dep.c @@ -0,0 +1,55 @@ +// RUN: rm -rf %t +// RUN: split-file %s %t + +// RUN: sed -e "s|DIR|%/t|g" %t/vfs.yaml.in > %t/vfs.yaml + +// RUN: clang-scan-deps -format experimental-full -j 1 -- \ +// RUN: %clang -ivfsoverlay %t/vfs.yaml -fmodules -fimplicit-module-maps \ +// RUN: -fmodules-cache-path=%t/cache -fmodule-name=ModuleName \ +// RUN: -I %/t/remapped -c %t/header-impl.c -o %t/header-impl.o \ +// RUN: | FileCheck %s -DPREFIX=%/t + +// CHECK: "command-line": [ +// CHECK: "-fmodule-map-file=[[PREFIX]]/remapped/module.modulemap" +// CHECK: "file-deps": [ +// CHECK: "[[PREFIX]]/remapped/module.modulemap" + +//--- vfs.yaml.in +{ + "version": 0, + "case-sensitive": "false", + "roots": [ + { + "name": "DIR/remapped", + "type": "directory", + "contents": [ + { + "name": "module.modulemap", + "type": "file", + "external-contents": "DIR/original/module.modulemap" + }, + { + "name": "header.h", + "type": "file", + "external-contents": "DIR/original/header.h" + } + ] + } + ] +} + +//--- original/module.modulemap +module ModuleName { + header "header.h" + export * +} + +//--- original/header.h +int foo_function(void); + +//--- header-impl.c +#include + +int foo_function(void) { + return 0; +} From e0643f7b4396d18035c45122c05c7f0c4d617b2e Mon Sep 17 00:00:00 2001 From: Volodymyr Sapsai Date: Tue, 7 Oct 2025 22:22:05 -0700 Subject: [PATCH 4/7] Switch `getName` to `getNameAsRequested`. --- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp index 0c3d4dc349b05..8c63a014da31f 100644 --- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp +++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp @@ -689,7 +689,7 @@ void ModuleDepCollectorPP::EndOfMainFile() { if (OptionalFileEntryRef CurrentModuleMap = PP.getHeaderSearchInfo().getModuleMap().getModuleMapFileForUniquing( CurrentModule)) - MDC.addFileDep(CurrentModuleMap->getName()); + MDC.addFileDep(CurrentModuleMap->getNameAsRequested()); } for (const Module *M : From b9928d749ad2db936c65fbb05ef41a7e99558c12 Mon Sep 17 00:00:00 2001 From: Volodymyr Sapsai Date: Wed, 8 Oct 2025 13:48:44 -0700 Subject: [PATCH 5/7] Attempt to handle back slashes on Windows. --- clang/test/ClangScanDeps/modules-current-modulemap-file-dep.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/ClangScanDeps/modules-current-modulemap-file-dep.c b/clang/test/ClangScanDeps/modules-current-modulemap-file-dep.c index dee568c3c8b77..b265ca4ba0e8c 100644 --- a/clang/test/ClangScanDeps/modules-current-modulemap-file-dep.c +++ b/clang/test/ClangScanDeps/modules-current-modulemap-file-dep.c @@ -10,7 +10,7 @@ // RUN: | FileCheck %s -DPREFIX=%/t // CHECK: "command-line": [ -// CHECK: "-fmodule-map-file=[[PREFIX]]/remapped/module.modulemap" +// CHECK: "-fmodule-map-file=[[PREFIX]]/remapped{{[/\\]}}module.modulemap" // CHECK: "file-deps": [ // CHECK: "[[PREFIX]]/remapped/module.modulemap" From bbfebe52a62347b972c8b3adb215e97ea2582746 Mon Sep 17 00:00:00 2001 From: Volodymyr Sapsai Date: Wed, 8 Oct 2025 14:40:22 -0700 Subject: [PATCH 6/7] Try a different approach to handle backslash. --- clang/test/ClangScanDeps/modules-current-modulemap-file-dep.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/test/ClangScanDeps/modules-current-modulemap-file-dep.c b/clang/test/ClangScanDeps/modules-current-modulemap-file-dep.c index b265ca4ba0e8c..9ee04eb5f7c84 100644 --- a/clang/test/ClangScanDeps/modules-current-modulemap-file-dep.c +++ b/clang/test/ClangScanDeps/modules-current-modulemap-file-dep.c @@ -7,10 +7,10 @@ // RUN: %clang -ivfsoverlay %t/vfs.yaml -fmodules -fimplicit-module-maps \ // RUN: -fmodules-cache-path=%t/cache -fmodule-name=ModuleName \ // RUN: -I %/t/remapped -c %t/header-impl.c -o %t/header-impl.o \ -// RUN: | FileCheck %s -DPREFIX=%/t +// RUN: | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t // CHECK: "command-line": [ -// CHECK: "-fmodule-map-file=[[PREFIX]]/remapped{{[/\\]}}module.modulemap" +// CHECK: "-fmodule-map-file=[[PREFIX]]/remapped/module.modulemap" // CHECK: "file-deps": [ // CHECK: "[[PREFIX]]/remapped/module.modulemap" From 016e533922007e5f357587403747558a16b7c282 Mon Sep 17 00:00:00 2001 From: Volodymyr Sapsai Date: Thu, 13 Nov 2025 21:50:32 -0800 Subject: [PATCH 7/7] Switch to "output path" as discussed in PR. --- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp | 2 +- clang/test/ClangScanDeps/modules-current-modulemap-file-dep.c | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp index 6f41c2716e398..3a99f8c882b8f 100644 --- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp +++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp @@ -623,7 +623,7 @@ void ModuleDepCollectorPP::EndOfMainFile() { if (OptionalFileEntryRef CurrentModuleMap = PP.getHeaderSearchInfo().getModuleMap().getModuleMapFileForUniquing( CurrentModule)) - MDC.addFileDep(CurrentModuleMap->getNameAsRequested()); + MDC.addFileDep(CurrentModuleMap->getName()); } for (const Module *M : diff --git a/clang/test/ClangScanDeps/modules-current-modulemap-file-dep.c b/clang/test/ClangScanDeps/modules-current-modulemap-file-dep.c index 9ee04eb5f7c84..e34bc876057d5 100644 --- a/clang/test/ClangScanDeps/modules-current-modulemap-file-dep.c +++ b/clang/test/ClangScanDeps/modules-current-modulemap-file-dep.c @@ -12,7 +12,9 @@ // CHECK: "command-line": [ // CHECK: "-fmodule-map-file=[[PREFIX]]/remapped/module.modulemap" // CHECK: "file-deps": [ -// CHECK: "[[PREFIX]]/remapped/module.modulemap" +// CHECK: "[[PREFIX]]/original/module.modulemap" + +// Verify that "file-deps" references actual on-disk module map and not using the virtual path. //--- vfs.yaml.in {