[clang][deps] Avoid generating arguments for missing module map files

[jansvoboda11]

Summary

Only modules that were built from preprocessed sources have PresumedModuleMapFile. Encode the optionality into the type system and avoid generating the -fmodule-map-file= arguments in such cases.

[dexonsmith]

Is there a way to test this?

[dexonsmith]

In D108544#2960279, @dexonsmith wrote:

Is there a way to test this?

Marking "request changes" for clarity.

Since this drops -fmodule-map-file= arguments, I imagine you can add a test that demonstrates it. But if that's currently awkward to observe because you need another piece to land in a follow-up, I'm fine to delay testing until then.

[jansvoboda11]

I created D108647 to more directly/correctly address the issues I'm seeing at the moment in explicit builds.

I think dealing with the possibility of empty PresumedModuleMapFile is still valuable, but I'm not aware how to test this at the moment (create module from non-preprocessed sources).

[dexonsmith]

In D108544#2962890, @jansvoboda11 wrote:

I created D108647 to more directly/correctly address the issues I'm seeing at the moment in explicit builds.

I think dealing with the possibility of empty PresumedModuleMapFile is still valuable, but I'm not aware how to test this at the moment (create module from non-preprocessed sources).

I'm not sure it is possible, based it being filled in always in ASTReader.cpp. I think this changed in ab75597ddac52f24e9cbd794cded195262ef670e but the comment wasn't updated. I suggest adding an assert that the module map file is not empty.

clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h

 //===- ModuleDepCollector.h - Callbacks to collect deps ---------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 

 namespace dependencies {
 
 class DependencyConsumer;
 
 /// Modular dependency that has already been built prior to the dependency scan.
 struct PrebuiltModuleDep {
   std::string ModuleName;
   std::string PCMFile;
-  std::string ModuleMapFile;
+  Optional<std::string> ModuleMapFile;
 
   explicit PrebuiltModuleDep(const Module *M)
       : ModuleName(M->getTopLevelModuleName()),
         PCMFile(M->getASTFile()->getName()),
-        ModuleMapFile(M->PresumedModuleMapFile) {}
+        ModuleMapFile(M->PresumedModuleMapFile.empty()
+                          ? None
+                          : Optional<std::string>(M->PresumedModuleMapFile)) {}
 };
 
 /// This is used to identify a specific module.
 struct ModuleID {
   /// The name of the module. This may include `:` for C++20 module partitions,
   /// or a header-name for C++20 header units.
   std::string ModuleName;
 

clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp

 //===- DependencyScanningTool.cpp - clang-scan-deps service ---------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 

 FullDependencies::getAdditionalArgsWithoutModulePaths() const {
   std::vector<std::string> Args{
       "-fno-implicit-modules",
       "-fno-implicit-module-maps",
   };
 
   for (const PrebuiltModuleDep &PMD : PrebuiltModuleDeps) {
     Args.push_back("-fmodule-file=" + PMD.ModuleName + "=" + PMD.PCMFile);
-    Args.push_back("-fmodule-map-file=" + PMD.ModuleMapFile);
+    if (PMD.ModuleMapFile)
+      Args.push_back("-fmodule-map-file=" + *PMD.ModuleMapFile);
   }
 
   return Args;
 }
 
 DependencyScanningTool::DependencyScanningTool(
     DependencyScanningService &Service)
     : Worker(Service) {}

clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

 //===- ModuleDepCollector.cpp - Callbacks to collect deps -------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
CompilerInvocation ModuleDepCollector::makeInvocationForModuleBuildWithoutPaths(
   CI.getLangOpts()->ModuleName = Deps.ID.ModuleName;
   CI.getFrontendOpts().IsSystemModule = Deps.IsSystem;
 
   CI.getLangOpts()->ImplicitModules = false;
 
   // Report the prebuilt modules this module uses.
   for (const auto &PrebuiltModule : Deps.PrebuiltModuleDeps) {
     CI.getFrontendOpts().ModuleFiles.push_back(PrebuiltModule.PCMFile);
-    CI.getFrontendOpts().ModuleMapFiles.push_back(PrebuiltModule.ModuleMapFile);
+    if (PrebuiltModule.ModuleMapFile)
+      CI.getFrontendOpts().ModuleMapFiles.push_back(
+          *PrebuiltModule.ModuleMapFile);
   }
 
   CI.getPreprocessorOpts().ImplicitPCHInclude.clear();
 
   return CI;
 }
 
 static std::vector<std::string>