Author: Jan Svoboda
Date: 2022-03-12T11:32:51+01:00
New Revision: 7f6af607464e5bcf2bd8b7bc7724a5985d7a0c34

URL: 
https://github.com/llvm/llvm-project/commit/7f6af607464e5bcf2bd8b7bc7724a5985d7a0c34
DIFF: 
https://github.com/llvm/llvm-project/commit/7f6af607464e5bcf2bd8b7bc7724a5985d7a0c34.diff

LOG: [clang][deps] Generate '-fmodule-file=' only for direct dependencies

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).

Depends on D120465.

Reviewed By: Bigcheese

Differential Revision: https://reviews.llvm.org/D118915

Added: 
    

Modified: 
    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

Removed: 
    


################################################################################
diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
index 36447dd2e38e6..aee4ddee9707b 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -51,12 +51,8 @@ struct FullDependencies {
   ///                      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.
-  std::vector<std::string> getCommandLine(
-      std::function<StringRef(ModuleID)> LookupPCMPath,
-      std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps) const;
+  std::vector<std::string>
+  getCommandLine(std::function<StringRef(ModuleID)> LookupPCMPath) const;
 
   /// Get the full command line, excluding -fmodule-file=" arguments.
   std::vector<std::string> getCommandLineWithoutModulePaths() const;

diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h 
b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
index d0fa474eda4eb..7936f9c60f6b0 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -113,27 +113,14 @@ struct ModuleDeps {
   ///                      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.
   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
   /// "-fmodule-file=" and "-o" arguments.
   std::vector<std::string> getCanonicalCommandLineWithoutModulePaths() const;
 };
 
-namespace detail {
-/// Collect the paths of PCM for the modules in \c Modules transitively.
-void collectPCMPaths(
-    llvm::ArrayRef<ModuleID> Modules,
-    std::function<StringRef(ModuleID)> LookupPCMPath,
-    std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps,
-    std::vector<std::string> &PCMPaths);
-} // namespace detail
-
 class ModuleDepCollector;
 
 /// Callback that records textual includes and direct modular includes/imports

diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
index 12602fccc92e0..06d39c266e80c 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -14,15 +14,11 @@ namespace tooling {
 namespace dependencies {
 
 std::vector<std::string> FullDependencies::getCommandLine(
-    std::function<StringRef(ModuleID)> LookupPCMPath,
-    std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps) const {
+    std::function<StringRef(ModuleID)> LookupPCMPath) const {
   std::vector<std::string> Ret = getCommandLineWithoutModulePaths();
 
-  std::vector<std::string> PCMPaths;
-  dependencies::detail::collectPCMPaths(ClangModuleDeps, LookupPCMPath,
-                                        LookupModuleDeps, PCMPaths);
-  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;
 }

diff  --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp 
b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 0b77ba8f9b0e8..ed18a111e51f3 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -87,8 +87,7 @@ serializeCompilerInvocation(const CompilerInvocation &CI) {
 }
 
 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();
 
@@ -97,9 +96,8 @@ std::vector<std::string> ModuleDeps::getCanonicalCommandLine(
   FrontendOpts.Inputs.emplace_back(ClangModuleMapFile, ModuleMapInputKind);
   FrontendOpts.OutputFile = std::string(LookupPCMPath(ID));
 
-  dependencies::detail::collectPCMPaths(ClangModuleDeps, LookupPCMPath,
-                                        LookupModuleDeps,
-                                        FrontendOpts.ModuleFiles);
+  for (ModuleID MID : ClangModuleDeps)
+    FrontendOpts.ModuleFiles.emplace_back(LookupPCMPath(MID));
 
   return serializeCompilerInvocation(CI);
 }
@@ -109,28 +107,6 @@ ModuleDeps::getCanonicalCommandLineWithoutModulePaths() 
const {
   return serializeCompilerInvocation(BuildInvocation);
 }
 
-void dependencies::detail::collectPCMPaths(
-    llvm::ArrayRef<ModuleID> Modules,
-    std::function<StringRef(ModuleID)> LookupPCMPath,
-    std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps,
-    std::vector<std::string> &PCMPaths) {
-  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());
-        }
-      };
-
-  AddArgs(Modules);
-}
-
 void ModuleDepCollectorPP::FileChanged(SourceLocation Loc,
                                        FileChangeReason Reason,
                                        SrcMgr::CharacteristicKind FileType,

diff  --git a/clang/test/ClangScanDeps/modules-full.cpp 
b/clang/test/ClangScanDeps/modules-full.cpp
index 9802f365a1005..1881053073724 100644
--- a/clang/test/ClangScanDeps/modules-full.cpp
+++ b/clang/test/ClangScanDeps/modules-full.cpp
@@ -169,9 +169,7 @@
 // CHECK:              "-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": [

diff  --git a/clang/test/ClangScanDeps/modules-pch.c 
b/clang/test/ClangScanDeps/modules-pch.c
index 9ac43c17d7832..7258d365b3882 100644
--- a/clang/test/ClangScanDeps/modules-pch.c
+++ b/clang/test/ClangScanDeps/modules-pch.c
@@ -97,7 +97,6 @@
 // CHECK-PCH:              "-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": [

diff  --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp 
b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index e88290d479ff5..8d70808575699 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -298,10 +298,7 @@ class FullDeps {
 
     ID.CommandLine = GenerateModulesPathArgs
                          ? FD.getCommandLine(
-                               [&](ModuleID MID) { return lookupPCMPath(MID); 
},
-                               [&](ModuleID MID) -> const ModuleDeps & {
-                                 return lookupModuleDeps(MID);
-                               })
+                               [&](ModuleID MID) { return lookupPCMPath(MID); 
})
                          : FD.getCommandLineWithoutModulePaths();
 
     Inputs.push_back(std::move(ID));
@@ -336,10 +333,7 @@ class FullDeps {
           {"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));
@@ -369,12 +363,16 @@ class FullDeps {
   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(
@@ -385,12 +383,6 @@ class FullDeps {
     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;


        
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to