Skip to content

Commit

Permalink
[clang][deps] Remove ModuleDeps::ImportedByMainFile
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jansvoboda11 committed Jul 28, 2023
1 parent 8a077cf commit c75b331
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,6 @@ struct ModuleDeps {
/// determined that the differences 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;
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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.
Expand Down
19 changes: 7 additions & 12 deletions clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}
Expand Down
16 changes: 11 additions & 5 deletions clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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);
Expand All @@ -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);

Expand Down Expand Up @@ -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 =
Expand Down

0 comments on commit c75b331

Please sign in to comment.