[clang][deps] Stop relying on name of inferred module maps

[jansvoboda11]

What the special case for "__inferred_module.map" was getting at is that in general we cannot report overridden files as dependencies: neither the build system nor Clang itself won't know what to do with such files that might not exist on the file system. Moreover, Clang always overrides files based on the command-line arguments, so whatever we drop this way in the scanner, the explicit build will be able to recreate.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Jan Svoboda (jansvoboda11)

Changes

What the special case for "__inferred_module.map" was getting at is that in general we cannot report overridden files as dependencies: neither the build system nor Clang itself won't know what to do with such files that might not exist on the file system. Moreover, Clang always overrides files based on the command-line arguments, so whatever we drop this way in the scanner, the explicit build will be able to recreate.

---

Full diff: https://github.com/llvm/llvm-project/pull/114085.diff

1 Files Affected:

• (modified) clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp (+4-14)

diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 637416cd1fc621..7ee335ecb8510f 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -599,14 +599,8 @@ ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
   MDC.ScanInstance.getASTReader()->visitInputFileInfos(
       *MF, /*IncludeSystem=*/true,
       [&](const serialization::InputFileInfo &IFI, bool IsSystem) {
-        // The __inferred_module.map file is an insignificant implementation
-        // detail of implicitly-built modules. The PCM will also report the
-        // actual on-disk module map file that allowed inferring the module,
-        // which is what we need for building the module explicitly
-        // Let's ignore this file.
-        if (StringRef(IFI.Filename).ends_with("__inferred_module.map"))
-          return;
-        MDC.addFileDep(MD, IFI.Filename);
+        if (!IFI.Overridden)
+          MDC.addFileDep(MD, IFI.Filename);
       });
 
   llvm::DenseSet<const Module *> SeenDeps;
@@ -617,12 +611,8 @@ ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
   MDC.ScanInstance.getASTReader()->visitInputFileInfos(
       *MF, /*IncludeSystem=*/true,
       [&](const serialization::InputFileInfo &IFI, bool IsSystem) {
-        if (!(IFI.TopLevel && IFI.ModuleMap))
-          return;
-        if (StringRef(IFI.FilenameAsRequested)
-                .ends_with("__inferred_module.map"))
-          return;
-        MD.ModuleMapFileDeps.emplace_back(IFI.FilenameAsRequested);
+        if (IFI.TopLevel && IFI.ModuleMap && !IFI.Overridden)
+          MD.ModuleMapFileDeps.emplace_back(IFI.FilenameAsRequested);
       });
 
   CowCompilerInvocation CI =

[benlangmuir]

Can this be tested?

[jansvoboda11]

Can this be tested?

Hmm, I'm not sure. The current tests for inferred module maps still pass. The other ways how SourceManager ends up overwriting a file are:

• by using the -remap-file option, which is unsupported with modules (ASTWriter intentionally asserts),
• using Clang rewriters (not sure if those can somehow end up in module files),
• performing code completion (again, not sure if that can somehow end up in a module file).

WDYT?

[benlangmuir]

Okay, sounds impractical