Revert "[clang][Dependency Scanning] Report What a Module Exports during Scanning (#137421)" #140820
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ing Scanning (llvm#137421)" This reverts commit ea1bfbf.
|
@llvm/pr-subscribers-clang Author: Qiongsi Wu (qiongsiwu) ChangesThis reverts commit ea1bfbf. The commit did not solve the fundamental issue we need to handle and is no longer necessary. rdar://144794793 Full diff: https://github.com/llvm/llvm-project/pull/140820.diff 5 Files Affected:
diff --git a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
index 06717a64c9a78..d2d0d56e5212c 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -178,25 +178,12 @@ struct ModuleDeps {
/// on, not including transitive dependencies.
std::vector<PrebuiltModuleDep> 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<ModuleDeps::DepInfo> ClangModuleDeps;
+ std::vector<ModuleID> ClangModuleDeps;
/// The set of libraries or frameworks to link against when
/// an entity from this module is used.
@@ -283,8 +270,7 @@ class ModuleDepCollectorPP final : public PPCallbacks {
llvm::DenseSet<const Module *> &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
@@ -366,16 +352,16 @@ class ModuleDepCollector final : public DependencyCollector {
/// Collect module map files for given modules.
llvm::DenseSet<const FileEntry *>
- collectModuleMapFiles(ArrayRef<ModuleDeps::DepInfo> ClangModuleDeps) const;
+ collectModuleMapFiles(ArrayRef<ModuleID> ClangModuleDeps) const;
/// Add module map files to the invocation, if needed.
void addModuleMapFiles(CompilerInvocation &CI,
- ArrayRef<ModuleDeps::DepInfo> ClangModuleDeps) const;
+ ArrayRef<ModuleID> ClangModuleDeps) const;
/// Add module files (pcm) to the invocation, if needed.
void addModuleFiles(CompilerInvocation &CI,
- ArrayRef<ModuleDeps::DepInfo> ClangModuleDeps) const;
+ ArrayRef<ModuleID> ClangModuleDeps) const;
void addModuleFiles(CowCompilerInvocation &CI,
- ArrayRef<ModuleDeps::DepInfo> ClangModuleDeps) const;
+ ArrayRef<ModuleID> 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 45901cbcedcb7..eb674246e2d51 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -389,10 +389,10 @@ ModuleDepCollector::getInvocationAdjustedForModuleBuildWithoutOutputs(
}
llvm::DenseSet<const FileEntry *> ModuleDepCollector::collectModuleMapFiles(
- ArrayRef<ModuleDeps::DepInfo> ClangModuleDeps) const {
+ ArrayRef<ModuleID> ClangModuleDeps) const {
llvm::DenseSet<const FileEntry *> 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().getOptionalFileRef(
@@ -404,23 +404,21 @@ llvm::DenseSet<const FileEntry *> ModuleDepCollector::collectModuleMapFiles(
}
void ModuleDepCollector::addModuleMapFiles(
- CompilerInvocation &CI,
- ArrayRef<ModuleDeps::DepInfo> ClangModuleDeps) const {
+ CompilerInvocation &CI, ArrayRef<ModuleID> ClangModuleDeps) const {
if (Service.shouldEagerLoadModules())
return; // Only pcm is needed for eager load.
- for (const auto &Info : ClangModuleDeps) {
- ModuleDeps *MD = ModuleDepsByID.loo
8000
kup(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<ModuleDeps::DepInfo> ClangModuleDeps) const {
- for (const auto &Info : ClangModuleDeps) {
- ModuleDeps *MD = ModuleDepsByID.lookup(Info.ID);
+ CompilerInvocation &CI, ArrayRef<ModuleID> ClangModuleDeps) const {
+ for (const ModuleID &MID : ClangModuleDeps) {
+ ModuleDeps *MD = ModuleDepsByID.lookup(MID);
std::string PCMPath =
Controller.lookupModuleOutput(*MD, ModuleOutputKind::ModuleFile);
@@ -428,15 +426,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<ModuleDeps::DepInfo> ClangModuleDeps) const {
- for (const auto &Info : ClangModuleDeps) {
- ModuleDeps *MD = ModuleDepsByID.lookup(Info.ID);
+ CowCompilerInvocation &CI, ArrayRef<ModuleID> ClangModuleDeps) const {
+ for (const ModuleID &MID : ClangModuleDeps) {
+ ModuleDeps *MD = ModuleDepsByID.lookup(MID);
std::string PCMPath =
Controller.lookupModuleOutput(*MD, ModuleOutputKind::ModuleFile);
@@ -444,7 +441,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)});
}
}
@@ -474,10 +471,10 @@ void ModuleDepCollector::applyDiscoveredDependencies(CompilerInvocation &CI) {
CI.getFrontendOpts().ModuleMapFiles.emplace_back(
CurrentModuleMap->getNameAsRequested());
- SmallVector<ModuleDeps::DepInfo> DirectDeps;
+ SmallVector<ModuleID> 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);
@@ -600,9 +597,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);
@@ -926,10 +923,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;
}
@@ -937,19 +933,12 @@ void ModuleDepCollectorPP::addOneModuleDep(const Module *M, bool Exported,
void ModuleDepCollectorPP::addModuleDep(
const Module *M, ModuleDeps &MD,
llvm::DenseSet<const Module *> &AddedModules) {
- SmallVector<Module *> ExportedModulesVector;
- M->getExportedModules(ExportedModulesVector);
- llvm::DenseSet<const Module *> 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);
}
}
}
@@ -973,7 +962,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.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 3b42267f4d5f4..1ce466d99deaf 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -353,32 +353,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<ModuleID> 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<ModuleDeps::DepInfo> 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));
+ });
};
}
|
cyndyishida
approved these changes
May 21, 2025
google-yfyang
pushed a commit
to google-yfyang/llvm-project
that referenced
this pull request
May 29, 2025
…ing Scanning (llvm#137421)" (llvm#140820) This reverts commit ea1bfbf. The commit did not solve the fundamental issue we need to handle and is no longer necessary. rdar://144794793
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This reverts commit ea1bfbf.
The commit did not solve the fundamental issue we need to handle and is no longer necessary.
rdar://144794793