[PATCH] D156563: [clang][deps] Remove `ModuleDeps::ImportedByMainFile`
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc75b331fc231: [clang][deps] Remove
`ModuleDeps::ImportedByMainFile` (authored by jansvoboda11).
Changed prior to commit:
https://reviews.llvm.org/D156563?vs=545234&id=545238#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D156563/new/
https://reviews.llvm.org/D156563
Files:
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
Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -242,7 +242,7 @@
SmallVector 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 @@
MDC.DirectPrebuiltModularDeps.insert(
{TopLevelModule, PrebuiltModuleDep{TopLevelModule}});
else
-DirectModularDeps.insert(TopLevelModule);
+MDC.DirectModularDeps.insert(TopLevelModule);
}
void ModuleDepCollectorPP::EndOfMainFile() {
@@ -394,9 +394,9 @@
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 @@
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 @@
ModuleDeps &MD = *ModI.first->second;
MD.ID.ModuleName = M->getFullModuleName();
- MD.ImportedByMainFile = DirectModularDeps.contains(M);
MD.IsSystem = M->IsSystem;
ModuleMap &ModMapInfo =
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -34,16 +34,12 @@
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 @@
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;
}
Index: clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
===
--- clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -136,10 +136,6 @@
/// determined that the differences are benign for this compilation.
std::vector ClangModuleDeps;
- // Used to track which modules that were discovered were directly imported by
- // the primary TU.
- bool ImportedByMainFile = false;
-
/// Compiler
[PATCH] D156563: [clang][deps] Remove `ModuleDeps::ImportedByMainFile`
benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. This flag was always weird to me, thanks for cleaning it up. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156563/new/ https://reviews.llvm.org/D156563 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D156563: [clang][deps] Remove `ModuleDeps::ImportedByMainFile`
jansvoboda11 created this revision.
jansvoboda11 added a reviewer: benlangmuir.
Herald added a subscriber: ributzka.
Herald added a project: All.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D156563
Files:
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
Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -242,7 +242,7 @@
SmallVector 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 @@
MDC.DirectPrebuiltModularDeps.insert(
{TopLevelModule, PrebuiltModuleDep{TopLevelModule}});
else
-DirectModularDeps.insert(TopLevelModule);
+MDC.DirectModularDeps.insert(TopLevelModule);
}
void ModuleDepCollectorPP::EndOfMainFile() {
@@ -394,9 +394,9 @@
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 @@
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 @@
ModuleDeps &MD = *ModI.first->second;
MD.ID.ModuleName = M->getFullModuleName();
- MD.ImportedByMainFile = DirectModularDeps.contains(M);
MD.IsSystem = M->IsSystem;
ModuleMap &ModMapInfo =
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -34,16 +34,12 @@
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 @@
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;
}
Index: clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
===
--- clang/include/clang/Tooling/DependencyScanning/ModuleDepC
