[clang] b9d5b0c - [clang][deps] NFC: Stop assuming the TU's context hash

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-05-17T09:16:00+02:00
New Revision: b9d5b0c20180e41f24722e622b8c3a3bb510fc1d

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

LOG: [clang][deps] NFC: Stop assuming the TU's context hash

The context hash of modular dependencies can be different from the context hash of the original translation unit if we modify their `CompilerInvocation`s.

Stop assuming the TU's context hash everywhere.

No functionality change here, since we're still currently using the unmodified TU CompilerInvocation to compute the context hash.

Reviewed By: dexonsmith

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

Added: 
    

Modified: 
    clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
    clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
    clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
index 95876bb665f10..8d191278f22ab 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -147,7 +147,7 @@ class ModuleDepCollectorPP final : public PPCallbacks {
   /// Traverses the previously collected direct modular dependencies to discover
   /// transitive modular dependencies and fills the parent \c ModuleDepCollector
   /// with both.
-  void handleTopLevelModule(const Module *M);
+  ModuleID handleTopLevelModule(const Module *M);
   void addAllSubmoduleDeps(const Module *M, ModuleDeps &MD,
                            llvm::DenseSet<const Module *> &AddedModules);
   void addModuleDep(const Module *M, ModuleDeps &MD,
@@ -173,13 +173,13 @@ class ModuleDepCollector final : public DependencyCollector {
   DependencyConsumer &Consumer;
   /// Path to the main source file.
   std::string MainFile;
-  /// The module hash identifying the compilation conditions.
+  /// Hash identifying the compilation conditions of the current TU.
   std::string ContextHash;
   /// Non-modular file dependencies. This includes the main source file and
   /// textually included header files.
   std::vector<std::string> FileDeps;
   /// Direct and transitive modular dependencies of the main source file.
-  std::unordered_map<std::string, ModuleDeps> ModularDeps;
+  std::unordered_map<const Module *, ModuleDeps> ModularDeps;
   /// Options that control the dependency output generation.
   std::unique_ptr<DependencyOutputOptions> Opts;
 };

diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
index 35817d3f7ae9d..196b05bafea07 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -143,7 +143,7 @@ DependencyScanningTool::getFullDependencies(
       for (auto &&M : ClangModuleDeps) {
         auto &MD = M.second;
         if (MD.ImportedByMainFile)
-          FD.ClangModuleDeps.push_back({MD.ID.ModuleName, ContextHash});
+          FD.ClangModuleDeps.push_back(MD.ID);
       }
 
       FullDependenciesResult FDR;

diff  --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 7c858178cdd53..517312aec8954 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -144,8 +144,6 @@ void ModuleDepCollectorPP::handleImport(const Module *Imported) {
     return;
 
   const Module *TopLevelModule = Imported->getTopLevelModule();
-  MDC.ModularDeps[MDC.ContextHash + TopLevelModule->getFullModuleName()]
-      .ImportedByMainFile = true;
   DirectModularDeps.insert(TopLevelModule);
 }
 
@@ -164,28 +162,27 @@ void ModuleDepCollectorPP::EndOfMainFile() {
     MDC.Consumer.handleFileDependency(*MDC.Opts, I);
 }
 
-void ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
+ModuleID ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
   assert(M == M->getTopLevelModule() && "Expected top level module!");
 
-  auto ModI = MDC.ModularDeps.insert(
-      std::make_pair(MDC.ContextHash + M->getFullModuleName(), ModuleDeps{}));
-
-  if (!ModI.first->second.ID.ModuleName.empty())
-    return;
+  // If this module has been handled already, just return its ID.
+  auto ModI = MDC.ModularDeps.insert({M, ModuleDeps{}});
+  if (!ModI.second)
+    return ModI.first->second.ID;
 
   ModuleDeps &MD = ModI.first->second;
 
+  MD.ID.ModuleName = M->getFullModuleName();
+  MD.ImportedByMainFile = DirectModularDeps.contains(M);
+  MD.ImplicitModulePCMPath = std::string(M->getASTFile()->getName());
+  MD.IsSystem = M->IsSystem;
+
   const FileEntry *ModuleMap = Instance.getPreprocessor()
                                    .getHeaderSearchInfo()
                                    .getModuleMap()
                                    .getContainingModuleMapFile(M);
-
-  MD.Invocation = Instance.getInvocationPtr();
   MD.ClangModuleMapFile = std::string(ModuleMap ? ModuleMap->getName() : "");
-  MD.ID.ModuleName = M->getFullModuleName();
-  MD.ImplicitModulePCMPath = std::string(M->getASTFile()->getName());
-  MD.ID.ContextHash = MDC.ContextHash;
-  MD.IsSystem = M->IsSystem;
+
   serialization::ModuleFile *MF =
       MDC.Instance.getASTReader()->getModuleManager().lookup(M->getASTFile());
   MDC.Instance.getASTReader()->visitInputFiles(
@@ -193,8 +190,16 @@ void ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
         MD.FileDeps.insert(IF.getFile()->getName());
       });
 
+  // FIXME: Prepare the CompilerInvocation for building this module **now**, so
+  //        that we store the actual context hash for this module (not just the
+  //        context hash inherited from the original TU).
+  MD.Invocation = Instance.getInvocationPtr();
+  MD.ID.ContextHash = MD.Invocation->getModuleHash();
+
   llvm::DenseSet<const Module *> AddedModules;
   addAllSubmoduleDeps(M, MD, AddedModules);
+
+  return MD.ID;
 }
 
 void ModuleDepCollectorPP::addAllSubmoduleDeps(
@@ -211,11 +216,9 @@ void ModuleDepCollectorPP::addModuleDep(
     llvm::DenseSet<const Module *> &AddedModules) {
   for (const Module *Import : M->Imports) {
     if (Import->getTopLevelModule() != M->getTopLevelModule()) {
+      ModuleID ImportID = handleTopLevelModule(Import->getTopLevelModule());
       if (AddedModules.insert(Import->getTopLevelModule()).second)
-        MD.ClangModuleDeps.push_back(
-            {std::string(Import->getTopLevelModuleName()),
-             Instance.getInvocation().getModuleHash()});
-      handleTopLevelModule(Import->getTopLevelModule());
+        MD.ClangModuleDeps.push_back(ImportID);
     }
   }
 }