[clang] 3190cf2 - [clang][deps] NFC: Extract ModuleID struct

Jan Svoboda via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 24 03:57:49 PDT 2021


Author: Jan Svoboda
Date: 2021-03-24T11:57:43+01:00
New Revision: 3190cf2017511e7a0570ea9a050a5f28f99d2bf6

URL: https://github.com/llvm/llvm-project/commit/3190cf2017511e7a0570ea9a050a5f28f99d2bf6
DIFF: https://github.com/llvm/llvm-project/commit/3190cf2017511e7a0570ea9a050a5f28f99d2bf6.diff

LOG: [clang][deps] NFC: Extract ModuleID struct

This patch extracts the `ModuleName` and `ContextHash` members of `ClangModuleDep`, `FullDependencies` and `ModuleDeps` into a single struct `ModuleID`. This makes it easier to understand how the full dependency graph works.

Reviewed By: Bigcheese, dexonsmith

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

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/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 1c106ed4b765a..b4fa27f531e32 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -22,19 +22,10 @@ namespace dependencies{
 
 /// The full dependencies and module graph for a specific input.
 struct FullDependencies {
-  /// The name of the C++20 module this translation unit exports. This may
-  /// include `:` for C++20 module partitons.
+  /// The identifier of the C++20 module this translation unit exports.
   ///
-  /// If the translation unit is not a module then this will be empty.
-  std::string ExportedModuleName;
-
-  /// The context hash represents the set of compiler options that may make one
-  /// version of a module incompatible with another. This includes things like
-  /// language mode, predefined macros, header search paths, etc...
-  ///
-  /// Modules with the same name but a 
diff erent \c ContextHash should be
-  /// treated as separate modules for the purpose of a build.
-  std::string ContextHash;
+  /// If the translation unit is not a module then \c ID.ModuleName is empty.
+  ModuleID ID;
 
   /// A collection of absolute paths to files that this translation unit
   /// directly depends on, not including transitive dependencies.
@@ -45,7 +36,7 @@ struct FullDependencies {
   ///
   /// This may include modules with a 
diff erent context hash when it can be
   /// determined that the 
diff erences are benign for this compilation.
-  std::vector<ClangModuleDep> ClangModuleDeps;
+  std::vector<ModuleID> ClangModuleDeps;
 
   /// A partial addtional set of command line arguments that can be used to
   /// build this translation unit.
@@ -65,8 +56,8 @@ struct FullDependencies {
   ///                         transitive set of dependencies for this
   ///                         compilation.
   std::vector<std::string> getAdditionalCommandLine(
-      std::function<StringRef(ClangModuleDep)> LookupPCMPath,
-      std::function<const ModuleDeps &(ClangModuleDep)> LookupModuleDeps) const;
+      std::function<StringRef(ModuleID)> LookupPCMPath,
+      std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps) const;
 };
 
 struct FullDependenciesResult {

diff  --git a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
index c490bb38c167d..2e487c7d89f3b 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -28,16 +28,9 @@ namespace dependencies {
 
 class DependencyConsumer;
 
-/// This is used to refer to a specific module.
-///
-/// See \c ModuleDeps for details about what these members mean.
-struct ClangModuleDep {
-  std::string ModuleName;
-  std::string ContextHash;
-};
-
-struct ModuleDeps {
-  /// The name of the module. This may include `:` for C++20 module partitons,
+/// 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;
 
@@ -48,6 +41,11 @@ struct ModuleDeps {
   /// Modules with the same name but a 
diff erent \c ContextHash should be
   /// treated as separate modules for the purpose of a build.
   std::string ContextHash;
+};
+
+struct ModuleDeps {
+  /// The identifier of the module.
+  ModuleID ID;
 
   /// The path to the modulemap file which defines this module.
   ///
@@ -62,12 +60,12 @@ struct ModuleDeps {
   /// on, not including transitive dependencies.
   llvm::StringSet<> FileDeps;
 
-  /// A list of modules this module directly depends on, not including
-  /// transitive dependencies.
+  /// A list of module identifiers this module directly depends on, not
+  /// including transitive dependencies.
   ///
   /// This may include modules with a 
diff erent context hash when it can be
   /// determined that the 
diff erences are benign for this compilation.
-  std::vector<ClangModuleDep> ClangModuleDeps;
+  std::vector<ModuleID> ClangModuleDeps;
 
   /// A partial command line that can be used to build this module.
   ///
@@ -89,8 +87,8 @@ struct ModuleDeps {
   ///                         transitive set of dependencies for this
   ///                         compilation.
   std::vector<std::string> getFullCommandLine(
-      std::function<StringRef(ClangModuleDep)> LookupPCMPath,
-      std::function<const ModuleDeps &(ClangModuleDep)> LookupModuleDeps) const;
+      std::function<StringRef(ModuleID)> LookupPCMPath,
+      std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps) const;
 };
 
 namespace detail {
@@ -98,9 +96,9 @@ namespace detail {
 /// modules in \c Modules transitively, along with other needed arguments to
 /// use explicitly built modules.
 void appendCommonModuleArguments(
-    llvm::ArrayRef<ClangModuleDep> Modules,
-    std::function<StringRef(ClangModuleDep)> LookupPCMPath,
-    std::function<const ModuleDeps &(ClangModuleDep)> LookupModuleDeps,
+    llvm::ArrayRef<ModuleID> Modules,
+    std::function<StringRef(ModuleID)> LookupPCMPath,
+    std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps,
     std::vector<std::string> &Result);
 } // namespace detail
 

diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
index 16040c2f46260..3c61242da575d 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -14,8 +14,8 @@ namespace tooling{
 namespace dependencies{
 
 std::vector<std::string> FullDependencies::getAdditionalCommandLine(
-    std::function<StringRef(ClangModuleDep)> LookupPCMPath,
-    std::function<const ModuleDeps &(ClangModuleDep)> LookupModuleDeps) const {
+    std::function<StringRef(ModuleID)> LookupPCMPath,
+    std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps) const {
   std::vector<std::string> Ret = AdditionalNonPathCommandLine;
 
   dependencies::detail::appendCommonModuleArguments(
@@ -109,7 +109,7 @@ DependencyScanningTool::getFullDependencies(
     }
 
     void handleModuleDependency(ModuleDeps MD) override {
-      ClangModuleDeps[MD.ContextHash + MD.ModuleName] = std::move(MD);
+      ClangModuleDeps[MD.ID.ContextHash + MD.ID.ModuleName] = std::move(MD);
     }
 
     void handleContextHash(std::string Hash) override {
@@ -119,14 +119,14 @@ DependencyScanningTool::getFullDependencies(
     FullDependenciesResult getFullDependencies() const {
       FullDependencies FD;
 
-      FD.ContextHash = std::move(ContextHash);
+      FD.ID.ContextHash = std::move(ContextHash);
 
       FD.FileDeps.assign(Dependencies.begin(), Dependencies.end());
 
       for (auto &&M : ClangModuleDeps) {
         auto &MD = M.second;
         if (MD.ImportedByMainFile)
-          FD.ClangModuleDeps.push_back({MD.ModuleName, ContextHash});
+          FD.ClangModuleDeps.push_back({MD.ID.ModuleName, ContextHash});
       }
 
       FullDependenciesResult FDR;

diff  --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index f74ce7304df5a..42a0b5af9d22e 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -18,8 +18,8 @@ using namespace tooling;
 using namespace dependencies;
 
 std::vector<std::string> ModuleDeps::getFullCommandLine(
-    std::function<StringRef(ClangModuleDep)> LookupPCMPath,
-    std::function<const ModuleDeps &(ClangModuleDep)> LookupModuleDeps) const {
+    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
@@ -32,21 +32,21 @@ std::vector<std::string> ModuleDeps::getFullCommandLine(
 }
 
 void dependencies::detail::appendCommonModuleArguments(
-    llvm::ArrayRef<ClangModuleDep> Modules,
-    std::function<StringRef(ClangModuleDep)> LookupPCMPath,
-    std::function<const ModuleDeps &(ClangModuleDep)> LookupModuleDeps,
+    llvm::ArrayRef<ModuleID> Modules,
+    std::function<StringRef(ModuleID)> LookupPCMPath,
+    std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps,
     std::vector<std::string> &Result) {
   llvm::StringSet<> AlreadyAdded;
 
-  std::function<void(llvm::ArrayRef<ClangModuleDep>)> AddArgs =
-      [&](llvm::ArrayRef<ClangModuleDep> Modules) {
-        for (const ClangModuleDep &CMD : Modules) {
-          if (!AlreadyAdded.insert(CMD.ModuleName + CMD.ContextHash).second)
+  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(CMD);
+          const ModuleDeps &M = LookupModuleDeps(MID);
           // Depth first traversal.
           AddArgs(M.ClangModuleDeps);
-          Result.push_back(("-fmodule-file=" + LookupPCMPath(CMD)).str());
+          Result.push_back(("-fmodule-file=" + LookupPCMPath(MID)).str());
           if (!M.ClangModuleMapFile.empty()) {
             Result.push_back("-fmodule-map-file=" + M.ClangModuleMapFile);
           }
@@ -133,7 +133,7 @@ void ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
   auto ModI = MDC.Deps.insert(
       std::make_pair(MDC.ContextHash + M->getFullModuleName(), ModuleDeps{}));
 
-  if (!ModI.first->second.ModuleName.empty())
+  if (!ModI.first->second.ID.ModuleName.empty())
     return;
 
   ModuleDeps &MD = ModI.first->second;
@@ -144,9 +144,9 @@ void ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
                                    .getContainingModuleMapFile(M);
 
   MD.ClangModuleMapFile = std::string(ModuleMap ? ModuleMap->getName() : "");
-  MD.ModuleName = M->getFullModuleName();
+  MD.ID.ModuleName = M->getFullModuleName();
   MD.ImplicitModulePCMPath = std::string(M->getASTFile()->getName());
-  MD.ContextHash = MDC.ContextHash;
+  MD.ID.ContextHash = MDC.ContextHash;
   serialization::ModuleFile *MF =
       MDC.Instance.getASTReader()->getModuleManager().lookup(M->getASTFile());
   MDC.Instance.getASTReader()->visitInputFiles(

diff  --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index 1bb65ef12d6b9..a8ff42ab104ca 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -222,16 +222,16 @@ static llvm::json::Array toJSONSorted(const llvm::StringSet<> &Set) {
   return llvm::json::Array(Strings);
 }
 
-static llvm::json::Array toJSONSorted(std::vector<ClangModuleDep> V) {
-  llvm::sort(V, [](const ClangModuleDep &A, const ClangModuleDep &B) {
+static llvm::json::Array toJSONSorted(std::vector<ModuleID> V) {
+  llvm::sort(V, [](const ModuleID &A, const ModuleID &B) {
     return std::tie(A.ModuleName, A.ContextHash) <
            std::tie(B.ModuleName, B.ContextHash);
   });
 
   llvm::json::Array Ret;
-  for (const ClangModuleDep &CMD : V)
+  for (const ModuleID &MID : V)
     Ret.push_back(llvm::json::Object(
-        {{"module-name", CMD.ModuleName}, {"context-hash", CMD.ContextHash}}));
+        {{"module-name", MID.ModuleName}, {"context-hash", MID.ContextHash}}));
   return Ret;
 }
 
@@ -244,26 +244,25 @@ class FullDeps {
 
     InputDeps ID;
     ID.FileName = std::string(Input);
-    ID.ContextHash = std::move(FD.ContextHash);
+    ID.ContextHash = std::move(FD.ID.ContextHash);
     ID.FileDeps = std::move(FD.FileDeps);
     ID.ModuleDeps = std::move(FD.ClangModuleDeps);
 
     std::unique_lock<std::mutex> ul(Lock);
     for (const ModuleDeps &MD : FDR.DiscoveredModules) {
-      auto I = Modules.find({MD.ContextHash, MD.ModuleName, 0});
+      auto I = Modules.find({MD.ID, 0});
       if (I != Modules.end()) {
         I->first.InputIndex = std::min(I->first.InputIndex, InputIndex);
         continue;
       }
-      Modules.insert(
-          I, {{MD.ContextHash, MD.ModuleName, InputIndex}, std::move(MD)});
+      Modules.insert(I, {{MD.ID, InputIndex}, std::move(MD)});
     }
 
     if (FullCommandLine)
       ID.AdditonalCommandLine = FD.getAdditionalCommandLine(
-          [&](ClangModuleDep CMD) { return lookupPCMPath(CMD); },
-          [&](ClangModuleDep CMD) -> const ModuleDeps & {
-            return lookupModuleDeps(CMD);
+          [&](ModuleID MID) { return lookupPCMPath(MID); },
+          [&](ModuleID MID) -> const ModuleDeps & {
+            return lookupModuleDeps(MID);
           });
 
     Inputs.push_back(std::move(ID));
@@ -271,13 +270,13 @@ class FullDeps {
 
   void printFullOutput(raw_ostream &OS) {
     // Sort the modules by name to get a deterministic order.
-    std::vector<ContextModulePair> ModuleNames;
+    std::vector<IndexedModuleID> ModuleIDs;
     for (auto &&M : Modules)
-      ModuleNames.push_back(M.first);
-    llvm::sort(ModuleNames,
-               [](const ContextModulePair &A, const ContextModulePair &B) {
-                 return std::tie(A.ModuleName, A.InputIndex) <
-                        std::tie(B.ModuleName, B.InputIndex);
+      ModuleIDs.push_back(M.first);
+    llvm::sort(ModuleIDs,
+               [](const IndexedModuleID &A, const IndexedModuleID &B) {
+                 return std::tie(A.ID.ModuleName, A.InputIndex) <
+                        std::tie(B.ID.ModuleName, B.InputIndex);
                });
 
     llvm::sort(Inputs, [](const InputDeps &A, const InputDeps &B) {
@@ -287,20 +286,20 @@ class FullDeps {
     using namespace llvm::json;
 
     Array OutModules;
-    for (auto &&ModName : ModuleNames) {
-      auto &MD = Modules[ModName];
+    for (auto &&ModID : ModuleIDs) {
+      auto &MD = Modules[ModID];
       Object O{
-          {"name", MD.ModuleName},
-          {"context-hash", MD.ContextHash},
+          {"name", MD.ID.ModuleName},
+          {"context-hash", MD.ID.ContextHash},
           {"file-deps", toJSONSorted(MD.FileDeps)},
           {"clang-module-deps", toJSONSorted(MD.ClangModuleDeps)},
           {"clang-modulemap-file", MD.ClangModuleMapFile},
           {"command-line",
            FullCommandLine
                ? MD.getFullCommandLine(
-                     [&](ClangModuleDep CMD) { return lookupPCMPath(CMD); },
-                     [&](ClangModuleDep CMD) -> const ModuleDeps & {
-                       return lookupModuleDeps(CMD);
+                     [&](ModuleID MID) { return lookupPCMPath(MID); },
+                     [&](ModuleID MID) -> const ModuleDeps & {
+                       return lookupModuleDeps(MID);
                      })
                : MD.NonPathCommandLine},
       };
@@ -328,33 +327,31 @@ class FullDeps {
   }
 
 private:
-  StringRef lookupPCMPath(ClangModuleDep CMD) {
-    return Modules[ContextModulePair{CMD.ContextHash, CMD.ModuleName, 0}]
-        .ImplicitModulePCMPath;
+  StringRef lookupPCMPath(ModuleID MID) {
+    return Modules[IndexedModuleID{MID, 0}].ImplicitModulePCMPath;
   }
 
-  const ModuleDeps &lookupModuleDeps(ClangModuleDep CMD) {
-    auto I =
-        Modules.find(ContextModulePair{CMD.ContextHash, CMD.ModuleName, 0});
+  const ModuleDeps &lookupModuleDeps(ModuleID MID) {
+    auto I = Modules.find(IndexedModuleID{MID, 0});
     assert(I != Modules.end());
     return I->second;
   };
 
-  struct ContextModulePair {
-    std::string ContextHash;
-    std::string ModuleName;
+  struct IndexedModuleID {
+    ModuleID ID;
     mutable size_t InputIndex;
 
-    bool operator==(const ContextModulePair &Other) const {
-      return ContextHash == Other.ContextHash && ModuleName == Other.ModuleName;
+    bool operator==(const IndexedModuleID &Other) const {
+      return ID.ModuleName == Other.ID.ModuleName &&
+             ID.ContextHash == Other.ID.ContextHash;
     }
   };
 
-  struct ContextModulePairHasher {
-    std::size_t operator()(const ContextModulePair &CMP) const {
+  struct IndexedModuleIDHasher {
+    std::size_t operator()(const IndexedModuleID &IMID) const {
       using llvm::hash_combine;
 
-      return hash_combine(CMP.ContextHash, CMP.ModuleName);
+      return hash_combine(IMID.ID.ModuleName, IMID.ID.ContextHash);
     }
   };
 
@@ -362,12 +359,12 @@ class FullDeps {
     std::string FileName;
     std::string ContextHash;
     std::vector<std::string> FileDeps;
-    std::vector<ClangModuleDep> ModuleDeps;
+    std::vector<ModuleID> ModuleDeps;
     std::vector<std::string> AdditonalCommandLine;
   };
 
   std::mutex Lock;
-  std::unordered_map<ContextModulePair, ModuleDeps, ContextModulePairHasher>
+  std::unordered_map<IndexedModuleID, ModuleDeps, IndexedModuleIDHasher>
       Modules;
   std::vector<InputDeps> Inputs;
 };


        


More information about the cfe-commits mailing list