Skip to content

Commit 0714626

Browse files
bnbarhambnbarham
authored
Merge pull request #10670 from bnbarham/cherry-missing-scan
[stable/20250402][clang][Dependency Scanning] Report What a Module Exports during Scanning (llvm#137421)
2 parents bcf9a07 + 8a6f646 commit 0714626

File tree

5 files changed

+216
-41
lines changed

5 files changed

+216
-41
lines changed

‎clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -142,12 +142,25 @@ struct ModuleDeps {
142142
/// on, not including transitive dependencies.
143143
std::vector<PrebuiltModuleDep> PrebuiltModuleDeps;
144144

145-
/// A list of module identifiers this module directly depends on, not
146-
/// including transitive dependencies.
145+
/// This struct contains information about a single dependency.
146+
struct DepInfo {
147+
/// Identifies the dependency.
148+
ModuleID ID;
149+
150+
/// Indicates if the module that has this dependency exports it or not.
151+
bool Exported = false;
152+
153+
bool operator<(const DepInfo &Other) const {
154+
return std::tie(ID, Exported) < std::tie(Other.ID, Other.Exported);
155+
}
156+
};
157+
158+
/// A list of DepsInfo containing information about modules this module
159+
/// directly depends on, not including transitive dependencies.
147160
///
148161
/// This may include modules with a different context hash when it can be
149162
/// determined that the differences are benign for this compilation.
150-
std::vector<ModuleID> ClangModuleDeps;
163+
std::vector<ModuleDeps::DepInfo> ClangModuleDeps;
151164

152165
/// The CASID for the module input dependency tree, if any.
153166
std::optional<std::string> CASFileSystemRootID;
@@ -245,7 +258,8 @@ class ModuleDepCollectorPP final : public PPCallbacks {
245258
llvm::DenseSet<const Module *> &AddedModules);
246259

247260
/// Add discovered module dependency for the given module.
248-
void addOneModuleDep(const Module *M, const ModuleID ID, ModuleDeps &MD);
261+
void addOneModuleDep(const Module *M, bool Exported, const ModuleID ID,
262+
ModuleDeps &MD);
249263
};
250264

251265
/// Collects modular and non-modular dependencies of the main file by attaching
@@ -322,16 +336,16 @@ class ModuleDepCollector final : public DependencyCollector {
322336

323337
/// Collect module map files for given modules.
324338
llvm::DenseSet<const FileEntry *>
325-
collectModuleMapFiles(ArrayRef<ModuleID> ClangModuleDeps) const;
339+
collectModuleMapFiles(ArrayRef<ModuleDeps::DepInfo> ClangModuleDeps) const;
326340

327341
/// Add module map files to the invocation, if needed.
328342
void addModuleMapFiles(CompilerInvocation &CI,
329-
ArrayRef<ModuleID> ClangModuleDeps) const;
343+
ArrayRef<ModuleDeps::DepInfo> ClangModuleDeps) const;
330344
/// Add module files (pcm) to the invocation, if needed.
331345
void addModuleFiles(CompilerInvocation &CI,
332-
ArrayRef<ModuleID> ClangModuleDeps) const;
346+
ArrayRef<ModuleDeps::DepInfo> ClangModuleDeps) const;
333347
void addModuleFiles(CowCompilerInvocation &CI,
334-
ArrayRef<ModuleID> ClangModuleDeps) const;
348+
ArrayRef<ModuleDeps::DepInfo> ClangModuleDeps) const;
335349

336350
/// Add paths that require looking up outputs to the given dependencies.
337351
void addOutputPaths(CowCompilerInvocation &CI, ModuleDeps &Deps);

‎clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp‎

Lines changed: 38 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -391,10 +391,10 @@ ModuleDepCollector::getInvocationAdjustedForModuleBuildWithoutOutputs(
391391
}
392392

393393
llvm::DenseSet<const FileEntry *> ModuleDepCollector::collectModuleMapFiles(
394-
ArrayRef<ModuleID> ClangModuleDeps) const {
394+
ArrayRef<ModuleDeps::DepInfo> ClangModuleDeps) const {
395395
llvm::DenseSet<const FileEntry *> ModuleMapFiles;
396-
for (const ModuleID &MID : ClangModuleDeps) {
397-
ModuleDeps *MD = ModuleDepsByID.lookup(MID);
396+
for (const auto &Info : ClangModuleDeps) {
397+
ModuleDeps *MD = ModuleDepsByID.lookup(Info.ID);
398398
assert(MD && "Inconsistent dependency info");
399399
// TODO: Track ClangModuleMapFile as `FileEntryRef`.
400400
auto FE = ScanInstance.getFileManager().getOptionalFileRef(
@@ -406,21 +406,23 @@ llvm::DenseSet<const FileEntry *> ModuleDepCollector::collectModuleMapFiles(
406406
}
407407

408408
void ModuleDepCollector::addModuleMapFiles(
409-
CompilerInvocation &CI, ArrayRef<ModuleID> ClangModuleDeps) const {
409+
CompilerInvocation &CI,
410+
ArrayRef<ModuleDeps::DepInfo> ClangModuleDeps) const {
410411
if (Service.shouldEagerLoadModules())
411412
return; // Only pcm is needed for eager load.
412413

413-
for (const ModuleID &MID : ClangModuleDeps) {
414-
ModuleDeps *MD = ModuleDepsByID.lookup(MID);
414+
for (const auto &Info : ClangModuleDeps) {
415+
ModuleDeps *MD = ModuleDepsByID.lookup(Info.ID);
415416
assert(MD && "Inconsistent dependency info");
416417
CI.getFrontendOpts().ModuleMapFiles.push_back(MD->ClangModuleMapFile);
417418
}
418419
}
419420

420421
void ModuleDepCollector::addModuleFiles(
421-
CompilerInvocation &CI, ArrayRef<ModuleID> ClangModuleDeps) const {
422-
for (const ModuleID &MID : ClangModuleDeps) {
423-
ModuleDeps *MD = ModuleDepsByID.lookup(MID);
422+
CompilerInvocation &CI,
423+
ArrayRef<ModuleDeps::DepInfo> ClangModuleDeps) const {
424+
for (const auto &Info : ClangModuleDeps) {
425+
ModuleDeps *MD = ModuleDepsByID.lookup(Info.ID);
424426
std::string PCMPath =
425427
Controller.lookupModuleOutput(*MD, ModuleOutputKind::ModuleFile);
426428

@@ -432,14 +434,15 @@ void ModuleDepCollector::addModuleFiles(
432434
CI.getFrontendOpts().ModuleFiles.push_back(std::move(PCMPath));
433435
else
434436
CI.getHeaderSearchOpts().PrebuiltModuleFiles.insert(
435-
{MID.ModuleName, std::move(PCMPath)});
437+
{Info.ID.ModuleName, std::move(PCMPath)});
436438
}
437439
}
438440

439441
void ModuleDepCollector::addModuleFiles(
440-
CowCompilerInvocation &CI, ArrayRef<ModuleID> ClangModuleDeps) const {
441-
for (const ModuleID &MID : ClangModuleDeps) {
442-
ModuleDeps *MD = ModuleDepsByID.lookup(MID);
442+
CowCompilerInvocation &CI,
443+
ArrayRef<ModuleDeps::DepInfo> ClangModuleDeps) const {
444+
for (const auto &Info : ClangModuleDeps) {
445+
ModuleDeps *MD = ModuleDepsByID.lookup(Info.ID);
443446
std::string PCMPath =
444447
Controller.lookupModuleOutput(*MD, ModuleOutputKind::ModuleFile);
445448

@@ -451,7 +454,7 @@ void ModuleDepCollector::addModuleFiles(
451454
CI.getMutFrontendOpts().ModuleFiles.push_back(std::move(PCMPath));
452455
else
453456
CI.getMutHeaderSearchOpts().PrebuiltModuleFiles.insert(
454-
{MID.ModuleName, std::move(PCMPath)});
457+
{Info.ID.ModuleName, std::move(PCMPath)});
455458
}
456459
}
457460

@@ -481,10 +484,10 @@ void ModuleDepCollector::applyDiscoveredDependencies(CompilerInvocation &CI) {
481484
CI.getFrontendOpts().ModuleMapFiles.emplace_back(
482485
CurrentModuleMap->getNameAsRequested());
483486

484-
SmallVector<ModuleID> DirectDeps;
487+
SmallVector<ModuleDeps::DepInfo> DirectDeps;
485488
for (const auto &KV : ModularDeps)
486489
if (DirectModularDeps.contains(KV.first))
487-
DirectDeps.push_back(KV.second->ID);
490+
DirectDeps.push_back({KV.second->ID, /* Exported = */ false});
488491

489492
// TODO: Report module maps the same way it's done for modular dependencies.
490493
addModuleMapFiles(CI, DirectDeps);
@@ -633,9 +636,9 @@ static std::string getModuleContextHash(const ModuleDeps &MD,
633636
// example, case-insensitive paths to modulemap files. Usually such a case
634637
// would indicate a missed optimization to canonicalize, but it may be
635638
// difficult to canonicalize all cases when there is a VFS.
636-
for (const auto &ID : MD.ClangModuleDeps) {
637-
HashBuilder.add(ID.ModuleName);
638-
HashBuilder.add(ID.ContextHash);
639+
for (const auto &Info : MD.ClangModuleDeps) {
640+
HashBuilder.add(Info.ID.ModuleName);
641+
HashBuilder.add(Info.ID.ContextHash);
639642
}
640643

641644
HashBuilder.add(EagerLoadModules);
@@ -1008,22 +1011,30 @@ void ModuleDepCollectorPP::addAllSubmoduleDeps(
10081011
});
10091012
}
10101013

1011-
void ModuleDepCollectorPP::addOneModuleDep(const Module *M, const ModuleID ID,
1012-
ModuleDeps &MD) {
1013-
MD.ClangModuleDeps.push_back(ID);
1014+
void ModuleDepCollectorPP::addOneModuleDep(const Module *M, bool Exported,
1015+
const ModuleID ID, ModuleDeps &MD) {
1016+
MD.ClangModuleDeps.push_back({ID, Exported});
1017+
10141018
if (MD.IsInStableDirectories)
10151019
MD.IsInStableDirectories = MDC.ModularDeps[M]->IsInStableDirectories;
10161020
}
10171021

10181022
void ModuleDepCollectorPP::addModuleDep(
10191023
const Module *M, ModuleDeps &MD,
10201024
llvm::DenseSet<const Module *> &AddedModules) {
1025+
SmallVector<Module *> ExportedModulesVector;
1026+
M->getExportedModules(ExportedModulesVector);
1027+
llvm::DenseSet<const Module *> ExportedModulesSet(
1028+
ExportedModulesVector.begin(), ExportedModulesVector.end());
10211029
for (const Module *Import : M->Imports) {
1022-
if (Import->getTopLevelModule() != M->getTopLevelModule() &&
1030+
const Module *ImportedTopLevelModule = Import->getTopLevelModule();
1031+
if (ImportedTopLevelModule != M->getTopLevelModule() &&
10231032
!MDC.isPrebuiltModule(Import)) {
1024-
if (auto ImportID = handleTopLevelModule(Import->getTopLevelModule()))
1025-
if (AddedModules.insert(Import->getTopLevelModule()).second)
1026-
addOneModuleDep(Import->getTopLevelModule(), *ImportID, MD);
1033+
if (auto ImportID = handleTopLevelModule(ImportedTopLevelModule))
1034+
if (AddedModules.insert(ImportedTopLevelModule).second) {
1035+
bool Exported = ExportedModulesSet.contains(ImportedTopLevelModule);
1036+
addOneModuleDep(ImportedTopLevelModule, Exported, *ImportID, MD);
1037+
}
10271038
}
10281039
}
10291040
}
@@ -1047,7 +1058,7 @@ void ModuleDepCollectorPP::addAffectingClangModule(
10471058
!MDC.isPrebuiltModule(Affecting)) {
10481059
if (auto ImportID = handleTopLevelModule(Affecting))
10491060
if (AddedModules.insert(Affecting).second)
1050-
addOneModuleDep(Affecting, *ImportID, MD);
1061+
addOneModuleDep(Affecting, /* Exported = */ false, *ImportID, MD);
10511062
}
10521063
}
10531064
}

‎clang/test/ClangScanDeps/export.c‎

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
// Test correctly reporting what a module exports during dependency scanning.
2+
// Module A depends on modules B, C and D, but only exports B and C.
3+
// Module E depends on modules B, C and D, and exports all of them.
4+
5+
// RUN: rm -rf %t
6+
// RUN: split-file %s %t
7+
// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
8+
// RUN: clang-scan-deps -compilation-database \
9+
// RUN: %t/cdb.json -format experimental-full > %t/deps.db
10+
// RUN: cat %t/deps.db | sed 's:\\\\\?:/:g' | FileCheck %s
11+
12+
//--- cdb.json.template
13+
[
14+
{
15+
"directory": "DIR",
16+
"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",
17+
"file": "DIR/test.c"
18+
},
19+
]
20+
21+
//--- AH/A.h
22+
#include "B.h"
23+
#include "C.h"
24+
#include "D.h"
25+
26+
int funcA();
27+
28+
//--- AH/module.modulemap
29+
module A {
30+
header "A.h"
31+
32+
export B
33+
export C
34+
}
35+
36+
//--- BH/B.h
37+
//--- BH/module.modulemap
38+
module B {
39+
header "B.h"
40+
}
41+
42+
//--- CH/C.h
43+
//--- CH/module.modulemap
44+
module C {
45+
header "C.h"
46+
}
47+
48+
//--- DH/D.h
49+
//--- DH/module.modulemap
50+
module D {
51+
header "D.h"
52+
}
53+
54+
//--- EH/E.h
55+
#include "B.h"
56+
#include "C.h"
57+
#include "D.h"
58+
59+
//--- EH/module.modulemap
60+
module E {
61+
header "E.h"
62+
export *
63+
}
64+
65+
//--- test.c
66+
#include "A.h"
67+
#include "E.h"
68+
69+
int test1() {
70+
return funcA();
71+
}
72+
73+
// CHECK: {
74+
// CHECK-NEXT: "modules": [
75+
// CHECK-NEXT: {
76+
// CHECK-NEXT: "clang-module-deps": [
77+
// CHECK-NEXT: {
78+
// CHECK-NEXT: "context-hash": "[[HASH_MOD_B:.*]]",
79+
// CHECK-NEXT: "module-name": "B",
80+
// CHECK-NEXT: "exported": "true"
81+
// CHECK-NEXT: },
82+
// CHECK-NEXT: {
83+
// CHECK-NEXT: "context-hash": "[[HASH_MOD_C:.*]]",
84+
// CHECK-NEXT: "module-name": "C",
85+
// CHECK-NEXT: "exported": "true"
86+
// CHECK-NEXT: },
87+
// CHECK-NEXT: {
88+
// CHECK-NEXT: "context-hash": "[[HASH_MOD_D:.*]]",
89+
// CHECK-NEXT: "module-name": "D"
90+
// CHECK-NEXT: }
91+
// CHECK-NEXT: ],
92+
// CHECK-NEXT: "clang-modulemap-file":{{.*}},
93+
// CHECK-NEXT: "command-line": [
94+
// CHECK: ],
95+
// CHECK: "name": "A"
96+
// CHECK-NEXT: }
97+
// CHECK: {
98+
// CHECK: "name": "B"
99+
// CHECK: }
100+
// CHECK: {
101+
// CHECK: "name": "C"
102+
// CHECK: }
103+
// CHECK: {
104+
// CHECK: "name": "D"
105+
// CHECK: }
106+
// CHECK: {
107+
// CHECK-NEXT: "clang-module-deps": [
108+
// CHECK-NEXT: {
109+
// CHECK-NEXT: "context-hash": "[[HASH_MOD_B]]",
110+
// CHECK-NEXT: "module-name": "B",
111+
// CHECK-NEXT: "exported": "true"
112+
// CHECK-NEXT: },
113+
// CHECK-NEXT: {
114+
// CHECK-NEXT: "context-hash": "[[HASH_MOD_C]]",
115+
// CHECK-NEXT: "module-name": "C",
116+
// CHECK-NEXT: "exported": "true"
117+
// CHECK-NEXT: },
118+
// CHECK-NEXT: {
119+
// CHECK-NEXT: "context-hash": "[[HASH_MOD_D]]",
120+
// CHECK-NEXT: "module-name": "D",
121+
// CHECK-NEXT: "exported": "true"
122+
// CHECK-NEXT: }
123+
// CHECK-NEXT: ],
124+
// CHECK-NEXT: "clang-modulemap-file":{{.*}},
125+
// CHECK-NEXT: "command-line": [
126+
// CHECK: ],
127+
// CHECK: "name": "E"
128+
// CHECK-NEXT: }
129+
// CHECK: ]
130+
// CHECK: }
131+
132+
133+

‎clang/test/ClangScanDeps/optimize-vfs-pch.m‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@
5454
// CHECK-NEXT: "clang-module-deps": [
5555
// CHECK-NEXT: {
5656
// CHECK-NEXT: "context-hash": "{{.*}}",
57-
// CHECK-NEXT: "module-name": "E"
57+
// CHECK-NEXT: "module-name": "E",
58+
// CHECK-NEXT: "exported": "true"
5859
// CHECK-NEXT: }
5960
// CHECK-NEXT: ],
6061
// CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/modules/D/module.modulemap",

‎clang/tools/clang-scan-deps/ClangScanDeps.cpp‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -605,16 +605,32 @@ static auto toJSONStrings(llvm::json::OStream &JOS, Container &&Strings) {
605605
};
606606
}
607607

608+
static auto toJSONModuleID(llvm::json::OStream &JOS, StringRef ContextHash,
609+
StringRef ModuleName, bool Exported) {
610+
return JOS.object([&] {
611+
JOS.attribute("context-hash", StringRef(ContextHash));
612+
JOS.attribute("module-name", StringRef(ModuleName));
613+
if (Exported)
614+
JOS.attribute("exported", StringRef("true"));
615+
});
616+
}
617+
608618
// Technically, we don't need to sort the dependency list to get determinism.
609619
// Leaving these be will simply preserve the import order.
610620
static auto toJSONSorted(llvm::json::OStream &JOS, std::vector<ModuleID> V) {
611621
llvm::sort(V);
612622
return [&JOS, V = std::move(V)] {
613-
for (const ModuleID &MID : V)
614-
JOS.object([&] {
615-
JOS.attribute("context-hash", StringRef(MID.ContextHash));
616-
JOS.attribute("module-name", StringRef(MID.ModuleName));
617-
});
623+
for (const auto &MID : V)
624+
toJSONModuleID(JOS, MID.ContextHash, MID.ModuleName, false);
625+
};
626+
}
627+
628+
static auto toJSONSorted(llvm::json::OStream &JOS,
629+
std::vector<ModuleDeps::DepInfo> V) {
630+
llvm::sort(V);
631+
return [&JOS, V = std::move(V)] {
632+
for (const ModuleDeps::DepInfo &MID : V)
633+
toJSONModuleID(JOS, MID.ID.ContextHash, MID.ID.ModuleName, MID.Exported);
618634
};
619635
}
620636

0 commit comments

Comments
 (0)