[clang][deps] Simplify function discovering .pcm and .modulemap files

[jansvoboda11]

Summary

This patch simplifies (and renames) the appendCommonModuleArguments function.

It no longer tries to construct the command line for explicitly building modules. Instead, it only performs the DFS traversal of modular dependencies and queries the callbacks to collect paths to .pcm and .modulemap files.

This makes it more flexible and usable in two contexts:

• Generating additional command line arguments for the main TU in modular build. The std::vector<std::string> output parameters can be used to manually generate appropriate command line flags.
• Generate full command line for a module. The output parameters can be the corresponding parts of CompilerInvocation. (In a follow-up patch.)

[dexonsmith]

LGTM.

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

struct ModuleDeps {
   std::vector<std::string> NonPathCommandLine;
 
   // Used to track which modules that were discovered were directly imported by
   // the primary TU.
   bool ImportedByMainFile = false;
 
   /// Gets the full command line suitable for passing to clang.
   ///
-  /// \param LookupPCMPath this function is called to fill in `-fmodule-file=`
+  /// \param LookupPCMPath This function is called to fill in `-fmodule-file=`
   ///                      flags and for the `-o` flag. It needs to return a
   ///                      path for where the PCM for the given module is to
   ///                      be located.
-  /// \param LookupModuleDeps this fucntion is called to collect the full
+  /// \param LookupModuleDeps This function is called to collect the full
   ///                         transitive set of dependencies for this
   ///                         compilation.
   std::vector<std::string> getFullCommandLine(
       std::function<StringRef(ModuleID)> LookupPCMPath,
       std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps) const;
 };
 
 namespace detail {
-/// Append the `-fmodule-file=` and `-fmodule-map-file=` arguments for the
-/// modules in \c Modules transitively, along with other needed arguments to
-/// use explicitly built modules.
-void appendCommonModuleArguments(
+/// Collect the paths of PCM and module map files for the modules in \c Modules
+/// transitively.
+void collectPCMAndModuleMapPaths(
     llvm::ArrayRef<ModuleID> Modules,
     std::function<StringRef(ModuleID)> LookupPCMPath,
     std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps,
-    std::vector<std::string> &Result);
+    std::vector<std::string> &PCMPaths, std::vector<std::string> &ModMapPaths);
 } // namespace detail
 
 class ModuleDepCollector;
 
 /// Callback that records textual includes and direct modular includes/imports
 /// during preprocessing. At the end of the main file, it also collects
 /// transitive modular dependencies and passes everything to the
 /// \c DependencyConsumer of the parent \c ModuleDepCollector.

clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp

 namespace tooling{
 namespace dependencies{
 
 std::vector<std::string> FullDependencies::getAdditionalCommandLine(
     std::function<StringRef(ModuleID)> LookupPCMPath,
     std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps) const {
   std::vector<std::string> Ret = AdditionalNonPathCommandLine;
 
-  dependencies::detail::appendCommonModuleArguments(
-      ClangModuleDeps, LookupPCMPath, LookupModuleDeps, Ret);
+  Ret.push_back("-fno-implicit-modules");
+  Ret.push_back("-fno-implicit-module-maps");
+
+  std::vector<std::string> PCMPaths;
+  std::vector<std::string> ModMapPaths;
+  dependencies::detail::collectPCMAndModuleMapPaths(
+      ClangModuleDeps, LookupPCMPath, LookupModuleDeps, PCMPaths, ModMapPaths);
+  for (const std::string &PCMPath : PCMPaths)
+    Ret.push_back("-fmodule-file=" + PCMPath);
+  for (const std::string &ModMapPath : ModMapPaths)
+    Ret.push_back("-fmodule-map-file=" + ModMapPath);
 
   return Ret;
 }
 
 DependencyScanningTool::DependencyScanningTool(
     DependencyScanningService &Service)
     : Worker(Service) {}
 

clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

 std::vector<std::string> ModuleDeps::getFullCommandLine(
     std::function<StringRef(ModuleID)> LookupPCMPath,
     std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps) const {
   std::vector<std::string> Ret = NonPathCommandLine;
 
   // TODO: Build full command line. That also means capturing the original
   //       command line into NonPathCommandLine.
 
-  dependencies::detail::appendCommonModuleArguments(
-      ClangModuleDeps, LookupPCMPath, LookupModuleDeps, Ret);
+  Ret.push_back("-fno-implicit-modules");
+  Ret.push_back("-fno-implicit-module-maps");
+
+  std::vector<std::string> PCMPaths;
+  std::vector<std::string> ModMapPaths;
+  dependencies::detail::collectPCMAndModuleMapPaths(
+      ClangModuleDeps, LookupPCMPath, LookupModuleDeps, PCMPaths, ModMapPaths);
+  for (const std::string &PCMPath : PCMPaths)
+    Ret.push_back("-fmodule-file=" + PCMPath);
+  for (const std::string &ModMapPath : ModMapPaths)
+    Ret.push_back("-fmodule-map-file=" + ModMapPath);
 
   return Ret;
 }
 
-void dependencies::detail::appendCommonModuleArguments(
+void dependencies::detail::collectPCMAndModuleMapPaths(
     llvm::ArrayRef<ModuleID> Modules,
     std::function<StringRef(ModuleID)> LookupPCMPath,
     std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps,
-    std::vector<std::string> &Result) {
+    std::vector<std::string> &PCMPaths, std::vector<std::string> &ModMapPaths) {
   llvm::StringSet<> AlreadyAdded;
 
   std::function<void(llvm::ArrayRef<ModuleID>)> AddArgs =
       [&](llvm::ArrayRef<ModuleID> Modules) {
         for (const ModuleID &MID : Modules) {
           if (!AlreadyAdded.insert(MID.ModuleName + MID.ContextHash).second)
             continue;
           const ModuleDeps &M = LookupModuleDeps(MID);
           // Depth first traversal.
           AddArgs(M.ClangModuleDeps);
-          Result.push_back(("-fmodule-file=" + LookupPCMPath(MID)).str());
-          if (!M.ClangModuleMapFile.empty()) {
-            Result.push_back("-fmodule-map-file=" + M.ClangModuleMapFile);
-          }
+          PCMPaths.push_back(LookupPCMPath(MID).str());
+          if (!M.ClangModuleMapFile.empty())
+            ModMapPaths.push_back(M.ClangModuleMapFile);
         }
       };
 
-  Result.push_back("-fno-implicit-modules");
-  Result.push_back("-fno-implicit-module-maps");
   AddArgs(Modules);
 }
 
 void ModuleDepCollectorPP::FileChanged(SourceLocation Loc,
                                        FileChangeReason Reason,
                                        SrcMgr::CharacteristicKind FileType,
                                        FileID PrevFID) {
   if (Reason != PPCallbacks::EnterFile)

clang/test/ClangScanDeps/modules-full.cpp

 // CHECK-NEXT:           "context-hash": "[[CONTEXT_HASH_H1]]",
 // CHECK-NEXT:           "module-name": "header1"
 // CHECK-NEXT:         }
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "command-line": [
 // CHECK-NEXT:         "-fno-implicit-modules",
 // CHECK-NEXT:         "-fno-implicit-module-maps",
 // CHECK-NEXT:         "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[CONTEXT_HASH_H1]]/header2-{{[A-Z0-9]+}}.pcm",
-// CHECK-NEXT:         "-fmodule-map-file=[[PREFIX]]/Inputs/module.modulemap",
 // CHECK-NEXT:         "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[CONTEXT_HASH_H1]]/header1-{{[A-Z0-9]+}}.pcm",
+// CHECK-NEXT:         "-fmodule-map-file=[[PREFIX]]/Inputs/module.modulemap",
 // CHECK-NEXT:         "-fmodule-map-file=[[PREFIX]]/Inputs/module.modulemap"
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "file-deps": [
 // CHECK-NEXT:         "[[PREFIX]]/modules_cdb_input2.cpp"
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "input-file": "[[PREFIX]]/modules_cdb_input2.cpp"
 // CHECK-NEXT:     }
 // CHECK-NEXT:   ]
 // CHECK-NEXT: }