[clang] c75b331 - [clang][deps] Remove `ModuleDeps::ImportedByMainFile`

Jan Svoboda via cfe-commits cfe-commits at lists.llvm.org
Fri Jul 28 12:05:44 PDT 2023


Author: Jan Svoboda
Date: 2023-07-28T12:04:35-07:00
New Revision: c75b331fc23192a8249dc5e95e053258f5fb5194

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

LOG: [clang][deps] Remove `ModuleDeps::ImportedByMainFile`

This information is already exposed via `TranslationUnitDeps::ClangModuleDeps` on the `DependencyScanningTool` level, and this patch also adds it on the `DependencyScanningWorker` level via `DependencyConsumer::handleDirectModuleDependency()`.

Besides being redundant, this bit of information is misleading for clients that share single `ModuleDeps` instance between multiple TUs (by using the `AlreadySeen` set). The module can be imported directly in some TUs but transitively in others.

Reviewed By: benlangmuir

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

Added: 
    

Modified: 
    clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
    clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
    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/DependencyScanningTool.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
index 3bb9fe6dae7f7c..f2cd781f57d654 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -162,6 +162,10 @@ class FullDependencyConsumer : public DependencyConsumer {
     ClangModuleDeps[MD.ID] = std::move(MD);
   }
 
+  void handleDirectModuleDependency(ModuleID ID) override {
+    DirectModuleDeps.push_back(ID);
+  }
+
   void handleContextHash(std::string Hash) override {
     ContextHash = std::move(Hash);
   }
@@ -173,6 +177,7 @@ class FullDependencyConsumer : public DependencyConsumer {
   std::vector<std::string> Dependencies;
   std::vector<PrebuiltModuleDep> PrebuiltModuleDeps;
   llvm::MapVector<ModuleID, ModuleDeps> ClangModuleDeps;
+  std::vector<ModuleID> DirectModuleDeps;
   std::vector<Command> Commands;
   std::string ContextHash;
   std::vector<std::string> OutputPaths;

diff  --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
index 350acb8f8a79eb..778d579bfb5015 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
@@ -56,6 +56,8 @@ class DependencyConsumer {
 
   virtual void handleModuleDependency(ModuleDeps MD) = 0;
 
+  virtual void handleDirectModuleDependency(ModuleID MD) = 0;
+
   virtual void handleContextHash(std::string Hash) = 0;
 };
 

diff  --git a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
index 30f779bfef8a85..914d55eadefe85 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -136,10 +136,6 @@ struct ModuleDeps {
   /// determined that the 
diff erences are benign for this compilation.
   std::vector<ModuleID> ClangModuleDeps;
 
-  // Used to track which modules that were discovered were directly imported by
-  // the primary TU.
-  bool ImportedByMainFile = false;
-
   /// Compiler invocation that can be used to build this module. Does not
   /// include argv[0].
   std::vector<std::string> BuildArguments;
@@ -172,8 +168,6 @@ class ModuleDepCollectorPP final : public PPCallbacks {
 private:
   /// The parent dependency collector.
   ModuleDepCollector &MDC;
-  /// Working set of direct modular dependencies.
-  llvm::SetVector<const Module *> DirectModularDeps;
 
   void handleImport(const Module *Imported);
 
@@ -243,6 +237,8 @@ class ModuleDepCollector final : public DependencyCollector {
   llvm::DenseMap<ModuleID, ModuleDeps *> ModuleDepsByID;
   /// Direct modular dependencies that have already been built.
   llvm::MapVector<const Module *, PrebuiltModuleDep> DirectPrebuiltModularDeps;
+  /// Working set of direct modular dependencies.
+  llvm::SetVector<const Module *> DirectModularDeps;
   /// Options that control the dependency output generation.
   std::unique_ptr<DependencyOutputOptions> Opts;
   /// The original Clang invocation passed to dependency scanner.

diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
index a4959889144196..4219f671658613 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -34,16 +34,12 @@ class MakeDependencyPrinterConsumer : public DependencyConsumer {
     Dependencies.push_back(std::string(File));
   }
 
-  void handlePrebuiltModuleDependency(PrebuiltModuleDep PMD) override {
-    // Same as `handleModuleDependency`.
-  }
-
-  void handleModuleDependency(ModuleDeps MD) override {
-    // These are ignored for the make format as it can't support the full
-    // set of deps, and handleFileDependency handles enough for implicitly
-    // built modules to work.
-  }
-
+  // These are ignored for the make format as it can't support the full
+  // set of deps, and handleFileDependency handles enough for implicitly
+  // built modules to work.
+  void handlePrebuiltModuleDependency(PrebuiltModuleDep PMD) override {}
+  void handleModuleDependency(ModuleDeps MD) override {}
+  void handleDirectModuleDependency(ModuleID ID) override {}
   void handleContextHash(std::string Hash) override {}
 
   void printDependencies(std::string &S) {
@@ -179,14 +175,13 @@ TranslationUnitDeps FullDependencyConsumer::takeTranslationUnitDeps() {
 
   for (auto &&M : ClangModuleDeps) {
     auto &MD = M.second;
-    if (MD.ImportedByMainFile)
-      TU.ClangModuleDeps.push_back(MD.ID);
     // TODO: Avoid handleModuleDependency even being called for modules
     //   we've already seen.
     if (AlreadySeen.count(M.first))
       continue;
     TU.ModuleGraph.push_back(std::move(MD));
   }
+  TU.ClangModuleDeps = std::move(DirectModuleDeps);
 
   return TU;
 }

diff  --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index aac24ca724aa17..060c5389b06d26 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -242,7 +242,7 @@ void ModuleDepCollector::applyDiscoveredDependencies(CompilerInvocation &CI) {
 
     SmallVector<ModuleID> DirectDeps;
     for (const auto &KV : ModularDeps)
-      if (KV.second->ImportedByMainFile)
+      if (DirectModularDeps.contains(KV.first))
         DirectDeps.push_back(KV.second->ID);
 
     // TODO: Report module maps the same way it's done for modular dependencies.
@@ -364,7 +364,7 @@ void ModuleDepCollectorPP::handleImport(const Module *Imported) {
     MDC.DirectPrebuiltModularDeps.insert(
         {TopLevelModule, PrebuiltModuleDep{TopLevelModule}});
   else
-    DirectModularDeps.insert(TopLevelModule);
+    MDC.DirectModularDeps.insert(TopLevelModule);
 }
 
 void ModuleDepCollectorPP::EndOfMainFile() {
@@ -394,9 +394,9 @@ void ModuleDepCollectorPP::EndOfMainFile() {
   for (const Module *M :
        MDC.ScanInstance.getPreprocessor().getAffectingClangModules())
     if (!MDC.isPrebuiltModule(M))
-      DirectModularDeps.insert(M);
+      MDC.DirectModularDeps.insert(M);
 
-  for (const Module *M : DirectModularDeps)
+  for (const Module *M : MDC.DirectModularDeps)
     handleTopLevelModule(M);
 
   MDC.Consumer.handleDependencyOutputOpts(*MDC.Opts);
@@ -408,6 +408,13 @@ void ModuleDepCollectorPP::EndOfMainFile() {
   for (auto &&I : MDC.ModularDeps)
     MDC.Consumer.handleModuleDependency(*I.second);
 
+  for (const Module *M : MDC.DirectModularDeps) {
+    auto It = MDC.ModularDeps.find(M);
+    // Only report direct dependencies that were successfully handled.
+    if (It != MDC.ModularDeps.end())
+      MDC.Consumer.handleDirectModuleDependency(MDC.ModularDeps[M]->ID);
+  }
+
   for (auto &&I : MDC.FileDeps)
     MDC.Consumer.handleFileDependency(I);
 
@@ -435,7 +442,6 @@ ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
   ModuleDeps &MD = *ModI.first->second;
 
   MD.ID.ModuleName = M->getFullModuleName();
-  MD.ImportedByMainFile = DirectModularDeps.contains(M);
   MD.IsSystem = M->IsSystem;
 
   ModuleMap &ModMapInfo =


        


More information about the cfe-commits mailing list