From cba3672022f2f116026afcd6c6ff762952962785 Mon Sep 17 00:00:00 2001 From: Qiongsi Wu Date: Tue, 20 May 2025 16:52:50 -0700 Subject: [PATCH] Revert "[clang][Dependency Scanning] Report What a Module Exports during Scanning (#137421) (#10604)" This reverts commit af6cc838120372a6932975bd8e614d336f6a3514. --- .../DependencyScanning/ModuleDepCollector.h | 30 ++-- .../DependencyScanning/ModuleDepCollector.cpp | 65 ++++----- clang/test/ClangScanDeps/export.c | 133 ------------------ .../ClangScanDeps/optimize-vfs-pch-tree.m | 1 - clang/test/ClangScanDeps/optimize-vfs-pch.m | 3 +- clang/tools/clang-scan-deps/ClangScanDeps.cpp | 26 +--- clang/tools/libclang/CDependencies.cpp | 8 +- 7 files changed, 45 insertions(+), 221 deletions(-) delete mode 100644 clang/test/ClangScanDeps/export.c diff --git a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h index 325f15fef0174..6ee5b08277bb3 100644 --- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h +++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h @@ -142,25 +142,12 @@ struct ModuleDeps { /// on, not including transitive dependencies. std::vector PrebuiltModuleDeps; - /// This struct contains information about a single dependency. - struct DepInfo { - /// Identifies the dependency. - ModuleID ID; - - /// Indicates if the module that has this dependency exports it or not. - bool Exported = false; - - bool operator<(const DepInfo &Other) const { - return std::tie(ID, Exported) < std::tie(Other.ID, Other.Exported); - } - }; - - /// A list of DepsInfo containing information about 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 different context hash when it can be /// determined that the differences are benign for this compilation. - std::vector ClangModuleDeps; + std::vector ClangModuleDeps; /// The CASID for the module input dependency tree, if any. std::optional CASFileSystemRootID; @@ -258,8 +245,7 @@ class ModuleDepCollectorPP final : public PPCallbacks { llvm::DenseSet &AddedModules); /// Add discovered module dependency for the given module. - void addOneModuleDep(const Module *M, bool Exported, const ModuleID ID, - ModuleDeps &MD); + void addOneModuleDep(const Module *M, const ModuleID ID, ModuleDeps &MD); }; /// Collects modular and non-modular dependencies of the main file by attaching @@ -336,16 +322,16 @@ class ModuleDepCollector final : public DependencyCollector { /// Collect module map files for given modules. llvm::DenseSet - collectModuleMapFiles(ArrayRef ClangModuleDeps) const; + collectModuleMapFiles(ArrayRef ClangModuleDeps) const; /// Add module map files to the invocation, if needed. void addModuleMapFiles(CompilerInvocation &CI, - ArrayRef ClangModuleDeps) const; + ArrayRef ClangModuleDeps) const; /// Add module files (pcm) to the invocation, if needed. void addModuleFiles(CompilerInvocation &CI, - ArrayRef ClangModuleDeps) const; + ArrayRef ClangModuleDeps) const; void addModuleFiles(CowCompilerInvocation &CI, - ArrayRef ClangModuleDeps) const; + ArrayRef ClangModuleDeps) const; /// Add paths that require looking up outputs to the given dependencies. void addOutputPaths(CowCompilerInvocation &CI, ModuleDeps &Deps); diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp index 6c40708c0ad16..ad40e2f47d3d2 100644 --- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp +++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp @@ -393,10 +393,10 @@ ModuleDepCollector::getInvocationAdjustedForModuleBuildWithoutOutputs( } llvm::DenseSet ModuleDepCollector::collectModuleMapFiles( - ArrayRef ClangModuleDeps) const { + ArrayRef ClangModuleDeps) const { llvm::DenseSet ModuleMapFiles; - for (const auto &Info : ClangModuleDeps) { - ModuleDeps *MD = ModuleDepsByID.lookup(Info.ID); + for (const ModuleID &MID : ClangModuleDeps) { + ModuleDeps *MD = ModuleDepsByID.lookup(MID); assert(MD && "Inconsistent dependency info"); // TODO: Track ClangModuleMapFile as `FileEntryRef`. auto FE = ScanInstance.getFileManager().getFile(MD->ClangModuleMapFile); @@ -407,23 +407,21 @@ llvm::DenseSet ModuleDepCollector::collectModuleMapFiles( } void ModuleDepCollector::addModuleMapFiles( - CompilerInvocation &CI, - ArrayRef ClangModuleDeps) const { + CompilerInvocation &CI, ArrayRef ClangModuleDeps) const { if (Service.shouldEagerLoadModules()) return; // Only pcm is needed for eager load. - for (const auto &Info : ClangModuleDeps) { - ModuleDeps *MD = ModuleDepsByID.lookup(Info.ID); + for (const ModuleID &MID : ClangModuleDeps) { + ModuleDeps *MD = ModuleDepsByID.lookup(MID); assert(MD && "Inconsistent dependency info"); CI.getFrontendOpts().ModuleMapFiles.push_back(MD->ClangModuleMapFile); } } void ModuleDepCollector::addModuleFiles( - CompilerInvocation &CI, - ArrayRef ClangModuleDeps) const { - for (const auto &Info : ClangModuleDeps) { - ModuleDeps *MD = ModuleDepsByID.lookup(Info.ID); + CompilerInvocation &CI, ArrayRef ClangModuleDeps) const { + for (const ModuleID &MID : ClangModuleDeps) { + ModuleDeps *MD = ModuleDepsByID.lookup(MID); std::string PCMPath = Controller.lookupModuleOutput(*MD, ModuleOutputKind::ModuleFile); @@ -436,15 +434,14 @@ void ModuleDepCollector::addModuleFiles( CI.getFrontendOpts().ModuleFiles.push_back(std::move(PCMPath)); else CI.getHeaderSearchOpts().PrebuiltModuleFiles.insert( - {Info.ID.ModuleName, std::move(PCMPath)}); + {MID.ModuleName, std::move(PCMPath)}); } } void ModuleDepCollector::addModuleFiles( - CowCompilerInvocation &CI, - ArrayRef ClangModuleDeps) const { - for (const auto &Info : ClangModuleDeps) { - ModuleDeps *MD = ModuleDepsByID.lookup(Info.ID); + CowCompilerInvocation &CI, ArrayRef ClangModuleDeps) const { + for (const ModuleID &MID : ClangModuleDeps) { + ModuleDeps *MD = ModuleDepsByID.lookup(MID); std::string PCMPath = Controller.lookupModuleOutput(*MD, ModuleOutputKind::ModuleFile); @@ -457,7 +454,7 @@ void ModuleDepCollector::addModuleFiles( CI.getMutFrontendOpts().ModuleFiles.push_back(std::move(PCMPath)); else CI.getMutHeaderSearchOpts().PrebuiltModuleFiles.insert( - {Info.ID.ModuleName, std::move(PCMPath)}); + {MID.ModuleName, std::move(PCMPath)}); } } @@ -487,10 +484,10 @@ void ModuleDepCollector::applyDiscoveredDependencies(CompilerInvocation &CI) { CI.getFrontendOpts().ModuleMapFiles.emplace_back( CurrentModuleMap->getNameAsRequested()); - SmallVector DirectDeps; + SmallVector DirectDeps; for (const auto &KV : ModularDeps) if (DirectModularDeps.contains(KV.first)) - DirectDeps.push_back({KV.second->ID, /* Exported = */ false}); + DirectDeps.push_back(KV.second->ID); // TODO: Report module maps the same way it's done for modular dependencies. addModuleMapFiles(CI, DirectDeps); @@ -639,9 +636,9 @@ static std::string getModuleContextHash(const ModuleDeps &MD, // example, case-insensitive paths to modulemap files. Usually such a case // would indicate a missed optimization to canonicalize, but it may be // difficult to canonicalize all cases when there is a VFS. - for (const auto &Info : MD.ClangModuleDeps) { - HashBuilder.add(Info.ID.ModuleName); - HashBuilder.add(Info.ID.ContextHash); + for (const auto &ID : MD.ClangModuleDeps) { + HashBuilder.add(ID.ModuleName); + HashBuilder.add(ID.ContextHash); } HashBuilder.add(EagerLoadModules); @@ -1021,10 +1018,9 @@ void ModuleDepCollectorPP::addAllSubmoduleDeps( }); } -void ModuleDepCollectorPP::addOneModuleDep(const Module *M, bool Exported, - const ModuleID ID, ModuleDeps &MD) { - MD.ClangModuleDeps.push_back({ID, Exported}); - +void ModuleDepCollectorPP::addOneModuleDep(const Module *M, const ModuleID ID, + ModuleDeps &MD) { + MD.ClangModuleDeps.push_back(ID); if (MD.IsInStableDirectories) MD.IsInStableDirectories = MDC.ModularDeps[M]->IsInStableDirectories; } @@ -1032,19 +1028,12 @@ void ModuleDepCollectorPP::addOneModuleDep(const Module *M, bool Exported, void ModuleDepCollectorPP::addModuleDep( const Module *M, ModuleDeps &MD, llvm::DenseSet &AddedModules) { - SmallVector ExportedModulesVector; - M->getExportedModules(ExportedModulesVector); - llvm::DenseSet ExportedModulesSet( - ExportedModulesVector.begin(), ExportedModulesVector.end()); for (const Module *Import : M->Imports) { - const Module *ImportedTopLevelModule = Import->getTopLevelModule(); - if (ImportedTopLevelModule != M->getTopLevelModule() && + if (Import->getTopLevelModule() != M->getTopLevelModule() && !MDC.isPrebuiltModule(Import)) { - if (auto ImportID = handleTopLevelModule(ImportedTopLevelModule)) - if (AddedModules.insert(ImportedTopLevelModule).second) { - bool Exported = ExportedModulesSet.contains(ImportedTopLevelModule); - addOneModuleDep(ImportedTopLevelModule, Exported, *ImportID, MD); - } + if (auto ImportID = handleTopLevelModule(Import->getTopLevelModule())) + if (AddedModules.insert(Import->getTopLevelModule()).second) + addOneModuleDep(Import->getTopLevelModule(), *ImportID, MD); } } } @@ -1068,7 +1057,7 @@ void ModuleDepCollectorPP::addAffectingClangModule( !MDC.isPrebuiltModule(Affecting)) { if (auto ImportID = handleTopLevelModule(Affecting)) if (AddedModules.insert(Affecting).second) - addOneModuleDep(Affecting, /* Exported = */ false, *ImportID, MD); + addOneModuleDep(Affecting, *ImportID, MD); } } } diff --git a/clang/test/ClangScanDeps/export.c b/clang/test/ClangScanDeps/export.c deleted file mode 100644 index a68747c9e6fe0..0000000000000 --- a/clang/test/ClangScanDeps/export.c +++ /dev/null @@ -1,133 +0,0 @@ -// Test correctly reporting what a module exports during dependency scanning. -// Module A depends on modules B, C and D, but only exports B and C. -// Module E depends on modules B, C and D, and exports all of them. - -// RUN: rm -rf %t -// RUN: split-file %s %t -// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json -// RUN: clang-scan-deps -compilation-database \ -// RUN: %t/cdb.json -format experimental-full > %t/deps.db -// RUN: cat %t/deps.db | sed 's:\\\\\?:/:g' | FileCheck %s - -//--- cdb.json.template -[ - { - "directory": "DIR", - "command": "clang -c DIR/test.c -I DIR/AH -I DIR/BH -I DIR/CH -I DIR/DH -I DIR/EH -fmodules -fmodules-cache-path=DIR/cache", - "file": "DIR/test.c" - }, -] - -//--- AH/A.h -#include "B.h" -#include "C.h" -#include "D.h" - -int funcA(); - -//--- AH/module.modulemap -module A { - header "A.h" - - export B - export C -} - -//--- BH/B.h -//--- BH/module.modulemap -module B { - header "B.h" -} - -//--- CH/C.h -//--- CH/module.modulemap -module C { - header "C.h" -} - -//--- DH/D.h -//--- DH/module.modulemap -module D { - header "D.h" -} - -//--- EH/E.h -#include "B.h" -#include "C.h" -#include "D.h" - -//--- EH/module.modulemap -module E { - header "E.h" - export * -} - -//--- test.c -#include "A.h" -#include "E.h" - -int test1() { - return funcA(); -} - -// CHECK: { -// CHECK-NEXT: "modules": [ -// CHECK-NEXT: { -// CHECK-NEXT: "clang-module-deps": [ -// CHECK-NEXT: { -// CHECK-NEXT: "context-hash": "[[HASH_MOD_B:.*]]", -// CHECK-NEXT: "module-name": "B", -// CHECK-NEXT: "exported": "true" -// CHECK-NEXT: }, -// CHECK-NEXT: { -// CHECK-NEXT: "context-hash": "[[HASH_MOD_C:.*]]", -// CHECK-NEXT: "module-name": "C", -// CHECK-NEXT: "exported": "true" -// CHECK-NEXT: }, -// CHECK-NEXT: { -// CHECK-NEXT: "context-hash": "[[HASH_MOD_D:.*]]", -// CHECK-NEXT: "module-name": "D" -// CHECK-NEXT: } -// CHECK-NEXT: ], -// CHECK-NEXT: "clang-modulemap-file":{{.*}}, -// CHECK-NEXT: "command-line": [ -// CHECK: ], -// CHECK: "name": "A" -// CHECK-NEXT: } -// CHECK: { -// CHECK: "name": "B" -// CHECK: } -// CHECK: { -// CHECK: "name": "C" -// CHECK: } -// CHECK: { -// CHECK: "name": "D" -// CHECK: } -// CHECK: { -// CHECK-NEXT: "clang-module-deps": [ -// CHECK-NEXT: { -// CHECK-NEXT: "context-hash": "[[HASH_MOD_B]]", -// CHECK-NEXT: "module-name": "B", -// CHECK-NEXT: "exported": "true" -// CHECK-NEXT: }, -// CHECK-NEXT: { -// CHECK-NEXT: "context-hash": "[[HASH_MOD_C]]", -// CHECK-NEXT: "module-name": "C", -// CHECK-NEXT: "exported": "true" -// CHECK-NEXT: }, -// CHECK-NEXT: { -// CHECK-NEXT: "context-hash": "[[HASH_MOD_D]]", -// CHECK-NEXT: "module-name": "D", -// CHECK-NEXT: "exported": "true" -// CHECK-NEXT: } -// CHECK-NEXT: ], -// CHECK-NEXT: "clang-modulemap-file":{{.*}}, -// CHECK-NEXT: "command-line": [ -// CHECK: ], -// CHECK: "name": "E" -// CHECK-NEXT: } -// CHECK: ] -// CHECK: } - - - diff --git a/clang/test/ClangScanDeps/optimize-vfs-pch-tree.m b/clang/test/ClangScanDeps/optimize-vfs-pch-tree.m index 3c6a2a1b74e2f..0c2ea25c40907 100644 --- a/clang/test/ClangScanDeps/optimize-vfs-pch-tree.m +++ b/clang/test/ClangScanDeps/optimize-vfs-pch-tree.m @@ -54,7 +54,6 @@ // CHECK-NEXT: { // CHECK-NEXT: "context-hash": "{{.*}}", // CHECK-NEXT: "module-name": "E" -// CHECK-NEXT: "exported": "true" // CHECK-NEXT: } // CHECK-NEXT: ], // CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/modules/D/module.modulemap", diff --git a/clang/test/ClangScanDeps/optimize-vfs-pch.m b/clang/test/ClangScanDeps/optimize-vfs-pch.m index 190a0509644ab..0b5cb08d365ee 100644 --- a/clang/test/ClangScanDeps/optimize-vfs-pch.m +++ b/clang/test/ClangScanDeps/optimize-vfs-pch.m @@ -54,8 +54,7 @@ // CHECK-NEXT: "clang-module-deps": [ // CHECK-NEXT: { // CHECK-NEXT: "context-hash": "{{.*}}", -// CHECK-NEXT: "module-name": "E", -// CHECK-NEXT: "exported": "true" +// CHECK-NEXT: "module-name": "E" // CHECK-NEXT: } // CHECK-NEXT: ], // CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/modules/D/module.modulemap", diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp index 7d88a03748190..cb108351431d9 100644 --- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp +++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp @@ -604,32 +604,16 @@ static auto toJSONStrings(llvm::json::OStream &JOS, Container &&Strings) { }; } -static auto toJSONModuleID(llvm::json::OStream &JOS, StringRef ContextHash, - StringRef ModuleName, bool Exported) { - return JOS.object([&] { - JOS.attribute("context-hash", StringRef(ContextHash)); - JOS.attribute("module-name", StringRef(ModuleName)); - if (Exported) - JOS.attribute("exported", StringRef("true")); - }); -} - // Technically, we don't need to sort the dependency list to get determinism. // Leaving these be will simply preserve the import order. static auto toJSONSorted(llvm::json::OStream &JOS, std::vector V) { llvm::sort(V); return [&JOS, V = std::move(V)] { - for (const auto &MID : V) - toJSONModuleID(JOS, MID.ContextHash, MID.ModuleName, false); - }; -} - -static auto toJSONSorted(llvm::json::OStream &JOS, - std::vector V) { - llvm::sort(V); - return [&JOS, V = std::move(V)] { - for (const ModuleDeps::DepInfo &MID : V) - toJSONModuleID(JOS, MID.ID.ContextHash, MID.ID.ModuleName, MID.Exported); + for (const ModuleID &MID : V) + JOS.object([&] { + JOS.attribute("context-hash", StringRef(MID.ContextHash)); + JOS.attribute("module-name", StringRef(MID.ModuleName)); + }); }; } diff --git a/clang/tools/libclang/CDependencies.cpp b/clang/tools/libclang/CDependencies.cpp index 99f5153bc04bd..4195b6c609bf7 100644 --- a/clang/tools/libclang/CDependencies.cpp +++ b/clang/tools/libclang/CDependencies.cpp @@ -292,8 +292,8 @@ static CXErrorCode getFullDependencies(DependencyScanningWorker *Worker, MD.forEachFileDep([&](StringRef File) { FileDeps.emplace_back(File); }); M.FileDeps = cxstring::createSet(FileDeps); std::vector Modules; - for (const auto &DepInfo : MD.ClangModuleDeps) - Modules.push_back(DepInfo.ID.ModuleName + ":" + DepInfo.ID.ContextHash); + for (ModuleID &MID : MD.ClangModuleDeps) + Modules.push_back(MID.ModuleName + ":" + MID.ContextHash); M.ModuleDeps = cxstring::createSet(Modules); M.BuildArguments = cxstring::createSet(MD.getBuildArguments()); } @@ -622,8 +622,8 @@ clang_experimental_DepGraphModule_getModuleDeps(CXDepGraphModule CXDepMod) { const ModuleDeps &ModDeps = *unwrap(CXDepMod)->ModDeps; std::vector Modules; Modules.reserve(ModDeps.ClangModuleDeps.size()); - for (const auto &DepInfo : ModDeps.ClangModuleDeps) - Modules.push_back(DepInfo.ID.ModuleName + ":" + DepInfo.ID.ContextHash); + for (const ModuleID &MID : ModDeps.ClangModuleDeps) + Modules.push_back(MID.ModuleName + ":" + MID.ContextHash); return unwrap(CXDepMod)->StrMgr.createCStringsOwned(std::move(Modules)); }