jansvoboda11 created this revision.
jansvoboda11 added a reviewer: Bigcheese.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The `clang-scan-deps` tool currently generates `-fmodule-file=` command-line 
arguments for the whole transitive closure of modular dependencies. This is not 
necessary, we only need to provide the direct dependencies on the command line. 
Information about transitive dependencies is stored within the `.pcm` files of 
direct dependencies. This makes the command lines shorter, but should be a NFC 
otherwise (unless there are bugs in the loading mechanism for explicit modules).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118915

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/test/ClangScanDeps/modules-full.cpp
  clang/test/ClangScanDeps/modules-pch.c
  clang/tools/clang-scan-deps/ClangScanDeps.cpp

Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===================================================================
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -296,14 +296,11 @@
       Modules.insert(I, {{MD.ID, InputIndex}, std::move(MD)});
     }
 
-    ID.AdditionalCommandLine =
-        GenerateModulesPathArgs
-            ? FD.getAdditionalArgs(
-                  [&](ModuleID MID) { return lookupPCMPath(MID); },
-                  [&](ModuleID MID) -> const ModuleDeps & {
-                    return lookupModuleDeps(MID);
-                  })
-            : FD.getAdditionalArgsWithoutModulePaths();
+    ID.AdditionalCommandLine = GenerateModulesPathArgs
+                                   ? FD.getAdditionalArgs([&](ModuleID MID) {
+                                       return lookupPCMPath(MID);
+                                     })
+                                   : FD.getAdditionalArgsWithoutModulePaths();
 
     Inputs.push_back(std::move(ID));
   }
@@ -337,10 +334,7 @@
           {"command-line",
            GenerateModulesPathArgs
                ? MD.getCanonicalCommandLine(
-                     [&](ModuleID MID) { return lookupPCMPath(MID); },
-                     [&](ModuleID MID) -> const ModuleDeps & {
-                       return lookupModuleDeps(MID);
-                     })
+                     [&](ModuleID MID) { return lookupPCMPath(MID); })
                : MD.getCanonicalCommandLineWithoutModulePaths()},
       };
       OutModules.push_back(std::move(O));
@@ -370,12 +364,16 @@
   StringRef lookupPCMPath(ModuleID MID) {
     auto PCMPath = PCMPaths.insert({MID, ""});
     if (PCMPath.second)
-      PCMPath.first->second = constructPCMPath(lookupModuleDeps(MID));
+      PCMPath.first->second = constructPCMPath(MID);
     return PCMPath.first->second;
   }
 
   /// Construct a path for the explicitly built PCM.
-  std::string constructPCMPath(const ModuleDeps &MD) const {
+  std::string constructPCMPath(ModuleID MID) const {
+    auto MDIt = Modules.find(IndexedModuleID{MID, 0});
+    assert(MDIt != Modules.end());
+    const ModuleDeps &MD = MDIt->second;
+
     StringRef Filename = llvm::sys::path::filename(MD.ImplicitModulePCMPath);
 
     SmallString<256> ExplicitPCMPath(
@@ -386,12 +384,6 @@
     return std::string(ExplicitPCMPath);
   }
 
-  const ModuleDeps &lookupModuleDeps(ModuleID MID) {
-    auto I = Modules.find(IndexedModuleID{MID, 0});
-    assert(I != Modules.end());
-    return I->second;
-  };
-
   struct IndexedModuleID {
     ModuleID ID;
     mutable size_t InputIndex;
Index: clang/test/ClangScanDeps/modules-pch.c
===================================================================
--- clang/test/ClangScanDeps/modules-pch.c
+++ clang/test/ClangScanDeps/modules-pch.c
@@ -94,7 +94,6 @@
 // CHECK-PCH-NEXT:         "-fno-implicit-modules",
 // CHECK-PCH-NEXT:         "-fno-implicit-module-maps",
 // CHECK-PCH-NEXT:         "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_COMMON_1]]/ModCommon1-{{.*}}.pcm",
-// CHECK-PCH-NEXT:         "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_COMMON_2]]/ModCommon2-{{.*}}.pcm",
 // CHECK-PCH-NEXT:         "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_PCH]]/ModPCH-{{.*}}.pcm"
 // CHECK-PCH-NEXT:       ],
 // CHECK-PCH-NEXT:       "file-deps": [
Index: clang/test/ClangScanDeps/modules-full.cpp
===================================================================
--- clang/test/ClangScanDeps/modules-full.cpp
+++ clang/test/ClangScanDeps/modules-full.cpp
@@ -166,9 +166,7 @@
 // CHECK-NEXT:         "-fno-implicit-modules"
 // CHECK-NEXT:         "-fno-implicit-module-maps"
 // CHECK-NO-ABS-NOT:   "-fmodule-file={{.*}}"
-// CHECK-ABS-NEXT:     "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H2_DINCLUDE]]/header2-{{[A-Z0-9]+}}.pcm"
 // CHECK-ABS-NEXT:     "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H1_DINCLUDE]]/header1-{{[A-Z0-9]+}}.pcm"
-// CHECK-CUSTOM-NEXT:  "-fmodule-file=[[PREFIX]]/custom/[[HASH_H2_DINCLUDE]]/header2-{{[A-Z0-9]+}}.pcm"
 // CHECK-CUSTOM-NEXT:  "-fmodule-file=[[PREFIX]]/custom/[[HASH_H1_DINCLUDE]]/header1-{{[A-Z0-9]+}}.pcm"
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "file-deps": [
Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -84,8 +84,7 @@
 }
 
 std::vector<std::string> ModuleDeps::getCanonicalCommandLine(
-    std::function<StringRef(ModuleID)> LookupPCMPath,
-    std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps) const {
+    std::function<StringRef(ModuleID)> LookupPCMPath) const {
   CompilerInvocation CI(BuildInvocation);
   FrontendOptions &FrontendOpts = CI.getFrontendOpts();
 
@@ -94,9 +93,8 @@
   FrontendOpts.Inputs.emplace_back(ClangModuleMapFile, ModuleMapInputKind);
   FrontendOpts.OutputFile = std::string(LookupPCMPath(ID));
 
-  dependencies::detail::collectPCMAndModuleMapPaths(
-      ClangModuleDeps, LookupPCMPath, LookupModuleDeps,
-      FrontendOpts.ModuleFiles, FrontendOpts.ModuleMapFiles);
+  for (ModuleID MID : ClangModuleDeps)
+    FrontendOpts.ModuleFiles.emplace_back(LookupPCMPath(MID));
 
   return serializeCompilerInvocation(CI);
 }
@@ -106,30 +104,6 @@
   return serializeCompilerInvocation(BuildInvocation);
 }
 
-void dependencies::detail::collectPCMAndModuleMapPaths(
-    llvm::ArrayRef<ModuleID> Modules,
-    std::function<StringRef(ModuleID)> LookupPCMPath,
-    std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps,
-    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);
-          PCMPaths.push_back(LookupPCMPath(MID).str());
-          if (!M.ClangModuleMapFile.empty())
-            ModMapPaths.push_back(M.ClangModuleMapFile);
-        }
-      };
-
-  AddArgs(Modules);
-}
-
 void ModuleDepCollectorPP::FileChanged(SourceLocation Loc,
                                        FileChangeReason Reason,
                                        SrcMgr::CharacteristicKind FileType,
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -14,16 +14,11 @@
 namespace dependencies{
 
 std::vector<std::string> FullDependencies::getAdditionalArgs(
-    std::function<StringRef(ModuleID)> LookupPCMPath,
-    std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps) const {
+    std::function<StringRef(ModuleID)> LookupPCMPath) const {
   std::vector<std::string> Ret = getAdditionalArgsWithoutModulePaths();
 
-  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 (ModuleID MID : ClangModuleDeps)
+    Ret.push_back(("-fmodule-file=" + LookupPCMPath(MID)).str());
 
   return Ret;
 }
Index: clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
===================================================================
--- clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -109,13 +109,8 @@
   ///                      arguments and the "-o" argument. It needs to return
   ///                      a path for where the PCM for the given module is to
   ///                      be located.
-  /// \param LookupModuleDeps This function is called to collect the full
-  ///                         transitive set of dependencies for this
-  ///                         compilation and fill in "-fmodule-map-file="
-  ///                         arguments.
   std::vector<std::string> getCanonicalCommandLine(
-      std::function<StringRef(ModuleID)> LookupPCMPath,
-      std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps) const;
+      std::function<StringRef(ModuleID)> LookupPCMPath) const;
 
   /// Gets the canonical command line suitable for passing to clang, excluding
   /// arguments containing modules-related paths: "-fmodule-file=", "-o",
@@ -123,16 +118,6 @@
   std::vector<std::string> getCanonicalCommandLineWithoutModulePaths() const;
 };
 
-namespace detail {
-/// 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> &PCMPaths, std::vector<std::string> &ModMapPaths);
-} // namespace detail
-
 class ModuleDepCollector;
 
 /// Callback that records textual includes and direct modular includes/imports
Index: clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
===================================================================
--- clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -53,9 +53,8 @@
   ///                         transitive set of dependencies for this
   ///                         compilation and fill in "-fmodule-map-file="
   ///                         arguments.
-  std::vector<std::string> getAdditionalArgs(
-      std::function<StringRef(ModuleID)> LookupPCMPath,
-      std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps) const;
+  std::vector<std::string>
+  getAdditionalArgs(std::function<StringRef(ModuleID)> LookupPCMPath) const;
 
   /// Get additional arguments suitable for appending to the original Clang
   /// command line, excluding arguments containing modules-related paths:
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D118915: [clang][d... Jan Svoboda via Phabricator via cfe-commits

Reply via email to