Skip to content

Commit ba55666

Browse files
jansvoboda11jansvoboda11
committed
[clang][deps] NFC: Split out the module-based API from the TU-based API
For users of the C++ API, the return type of `getFullDependencies` doesn't make sense when asking for dependencies of a module. In the returned `FullDependenciesResult` instance, only `DiscoveredModules` is useful (the graph of modular dependecies). The `FullDeps` member is trying to describe a translation unit it was never given. Its command line also refers to a file in the in-memory VFS we create in the scanner, leaking the implementation detail. This patch splits the API and improves layering and naming of the return types. Depends on D140175. Reviewed By: artemcm Differential Revision: https://reviews.llvm.org/D140176
1 parent e60fcfd commit ba55666

File tree

5 files changed

+120
-65
lines changed

5 files changed

+120
-65
lines changed

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

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,14 @@ namespace dependencies {
2828
using LookupModuleOutputCallback =
2929
llvm::function_ref<std::string(const ModuleID &, ModuleOutputKind)>;
3030

31+
/// Graph of modular dependencies.
32+
using ModuleDepsGraph = std::vector<ModuleDeps>;
33+
3134
/// The full dependencies and module graph for a specific input.
32-
struct FullDependencies {
35+
struct TranslationUnitDeps {
36+
/// The graph of direct and transitive modular dependencies.
37+
ModuleDepsGraph ModuleGraph;
38+
3339
/// The identifier of the C++20 module this translation unit exports.
3440
///
3541
/// If the translation unit is not a module then \c ID.ModuleName is empty.
@@ -62,11 +68,6 @@ struct FullDependencies {
6268
std::vector<std::string> DriverCommandLine;
6369
};
6470

65-
struct FullDependenciesResult {
66-
FullDependencies FullDeps;
67-
std::vector<ModuleDeps> DiscoveredModules;
68-
};
69-
7071
/// The high-level implementation of the dependency discovery tool that runs on
7172
/// an individual worker thread.
7273
class DependencyScanningTool {
@@ -78,18 +79,15 @@ class DependencyScanningTool {
7879

7980
/// Print out the dependency information into a string using the dependency
8081
/// file format that is specified in the options (-MD is the default) and
81-
/// return it. If \p ModuleName isn't empty, this function returns the
82-
/// dependency information of module \p ModuleName.
82+
/// return it.
8383
///
8484
/// \returns A \c StringError with the diagnostic output if clang errors
8585
/// occurred, dependency file contents otherwise.
8686
llvm::Expected<std::string>
87-
getDependencyFile(const std::vector<std::string> &CommandLine, StringRef CWD,
88-
std::optional<StringRef> ModuleName = std::nullopt);
87+
getDependencyFile(const std::vector<std::string> &CommandLine, StringRef CWD);
8988

90-
/// Collect the full module dependency graph for the input, ignoring any
91-
/// modules which have already been seen. If \p ModuleName isn't empty, this
92-
/// function returns the full dependency information of module \p ModuleName.
89+
/// Given a Clang driver command-line for a translation unit, gather the
90+
/// modular dependencies and return the information needed for explicit build.
9391
///
9492
/// \param AlreadySeen This stores modules which have previously been
9593
/// reported. Use the same instance for all calls to this
@@ -101,12 +99,21 @@ class DependencyScanningTool {
10199
/// arguments for dependencies.
102100
///
103101
/// \returns a \c StringError with the diagnostic output if clang errors
104-
/// occurred, \c FullDependencies otherwise.
105-
llvm::Expected<FullDependenciesResult>
106-
getFullDependencies(const std::vector<std::string> &CommandLine,
107-
StringRef CWD, const llvm::StringSet<> &AlreadySeen,
108-
LookupModuleOutputCallback LookupModuleOutput,
109-
std::optional<StringRef> ModuleName = std::nullopt);
102+
/// occurred, \c TranslationUnitDeps otherwise.
103+
llvm::Expected<TranslationUnitDeps>
104+
getTranslationUnitDependencies(const std::vector<std::string> &CommandLine,
105+
StringRef CWD,
106+
const llvm::StringSet<> &AlreadySeen,
107+
LookupModuleOutputCallback LookupModuleOutput);
108+
109+
/// Given a compilation context specified via the Clang driver command-line,
110+
/// gather modular dependencies of module with the given name, and return the
111+
/// information needed for explicit build.
112+
llvm::Expected<ModuleDepsGraph>
113+
getModuleDependencies(StringRef ModuleName,
114+
const std::vector<std::string> &CommandLine,
115+
StringRef CWD, const llvm::StringSet<> &AlreadySeen,
116+
LookupModuleOutputCallback LookupModuleOutput);
110117

111118
private:
112119
DependencyScanningWorker Worker;
@@ -145,7 +152,8 @@ class FullDependencyConsumer : public DependencyConsumer {
145152
return LookupModuleOutput(ID, Kind);
146153
}
147154

148-
FullDependenciesResult takeFullDependencies();
155+
TranslationUnitDeps takeTranslationUnitDeps();
156+
ModuleDepsGraph takeModuleGraphDeps();
149157

150158
private:
151159
std::vector<std::string> Dependencies;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ class DependencyScanningWorkerFilesystem;
3131

3232
/// A command-line tool invocation that is part of building a TU.
3333
///
34-
/// \see FullDependencies::Commands.
34+
/// \see TranslationUnitDeps::Commands.
3535
struct Command {
3636
std::string Executable;
3737
std::vector<std::string> Arguments;

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

Lines changed: 41 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,7 @@ DependencyScanningTool::DependencyScanningTool(
2020
: Worker(Service, std::move(FS)) {}
2121

2222
llvm::Expected<std::string> DependencyScanningTool::getDependencyFile(
23-
const std::vector<std::string> &CommandLine, StringRef CWD,
24-
std::optional<StringRef> ModuleName) {
23+
const std::vector<std::string> &CommandLine, StringRef CWD) {
2524
/// Prints out all of the gathered dependencies into a string.
2625
class MakeDependencyPrinterConsumer : public DependencyConsumer {
2726
public:
@@ -81,48 +80,71 @@ llvm::Expected<std::string> DependencyScanningTool::getDependencyFile(
8180
};
8281

8382
MakeDependencyPrinterConsumer Consumer;
84-
auto Result =
85-
Worker.computeDependencies(CWD, CommandLine, Consumer, ModuleName);
83+
auto Result = Worker.computeDependencies(CWD, CommandLine, Consumer);
8684
if (Result)
8785
return std::move(Result);
8886
std::string Output;
8987
Consumer.printDependencies(Output);
9088
return Output;
9189
}
9290

93-
llvm::Expected<FullDependenciesResult>
94-
DependencyScanningTool::getFullDependencies(
91+
llvm::Expected<TranslationUnitDeps>
92+
DependencyScanningTool::getTranslationUnitDependencies(
9593
const std::vector<std::string> &CommandLine, StringRef CWD,
9694
const llvm::StringSet<> &AlreadySeen,
97-
LookupModuleOutputCallback LookupModuleOutput,
98-
std::optional<StringRef> ModuleName) {
95+
LookupModuleOutputCallback LookupModuleOutput) {
96+
FullDependencyConsumer Consumer(AlreadySeen, LookupModuleOutput);
97+
llvm::Error Result = Worker.computeDependencies(CWD, CommandLine, Consumer);
98+
if (Result)
99+
return std::move(Result);
100+
return Consumer.takeTranslationUnitDeps();
101+
}
102+
103+
llvm::Expected<ModuleDepsGraph> DependencyScanningTool::getModuleDependencies(
104+
StringRef ModuleName, const std::vector<std::string> &CommandLine,
105+
StringRef CWD, const llvm::StringSet<> &AlreadySeen,
106+
LookupModuleOutputCallback LookupModuleOutput) {
99107
FullDependencyConsumer Consumer(AlreadySeen, LookupModuleOutput);
100108
llvm::Error Result =
101109
Worker.computeDependencies(CWD, CommandLine, Consumer, ModuleName);
102110
if (Result)
103111
return std::move(Result);
104-
return Consumer.takeFullDependencies();
112+
return Consumer.takeModuleGraphDeps();
105113
}
106114

107-
FullDependenciesResult FullDependencyConsumer::takeFullDependencies() {
108-
FullDependenciesResult FDR;
109-
FullDependencies &FD = FDR.FullDeps;
115+
TranslationUnitDeps FullDependencyConsumer::takeTranslationUnitDeps() {
116+
TranslationUnitDeps TU;
110117

111-
FD.ID.ContextHash = std::move(ContextHash);
112-
FD.FileDeps = std::move(Dependencies);
113-
FD.PrebuiltModuleDeps = std::move(PrebuiltModuleDeps);
114-
FD.Commands = std::move(Commands);
118+
TU.ID.ContextHash = std::move(ContextHash);
119+
TU.FileDeps = std::move(Dependencies);
120+
TU.PrebuiltModuleDeps = std::move(PrebuiltModuleDeps);
121+
TU.Commands = std::move(Commands);
115122

116123
for (auto &&M : ClangModuleDeps) {
117124
auto &MD = M.second;
118125
if (MD.ImportedByMainFile)
119-
FD.ClangModuleDeps.push_back(MD.ID);
126+
TU.ClangModuleDeps.push_back(MD.ID);
127+
// TODO: Avoid handleModuleDependency even being called for modules
128+
// we've already seen.
129+
if (AlreadySeen.count(M.first))
130+
continue;
131+
TU.ModuleGraph.push_back(std::move(MD));
132+
}
133+
134+
return TU;
135+
}
136+
137+
ModuleDepsGraph FullDependencyConsumer::takeModuleGraphDeps() {
138+
ModuleDepsGraph ModuleGraph;
139+
140+
for (auto &&M : ClangModuleDeps) {
141+
auto &MD = M.second;
120142
// TODO: Avoid handleModuleDependency even being called for modules
121143
// we've already seen.
122144
if (AlreadySeen.count(M.first))
123145
continue;
124-
FDR.DiscoveredModules.push_back(std::move(MD));
146+
ModuleGraph.push_back(std::move(MD));
125147
}
126148

127-
return FDR;
149+
return ModuleGraph;
128150
}

‎clang/test/ClangScanDeps/modules-full-by-mod-name.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,3 +75,5 @@
7575
// CHECK-NEXT: "name": "header2"
7676
// CHECK-NEXT: }
7777
// CHECK-NEXT: ],
78+
// CHECK-NEXT: "translation-units": []
79+
// CHECK-NEXT: }

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

Lines changed: 48 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -250,29 +250,29 @@ static llvm::json::Array toJSONSorted(std::vector<ModuleID> V) {
250250
// Thread safe.
251251
class FullDeps {
252252
public:
253-
void mergeDeps(StringRef Input, FullDependenciesResult FDR,
253+
void mergeDeps(StringRef Input, TranslationUnitDeps TUDeps,
254254
size_t InputIndex) {
255-
FullDependencies &FD = FDR.FullDeps;
256-
257255
InputDeps ID;
258256
ID.FileName = std::string(Input);
259-
ID.ContextHash = std::move(FD.ID.ContextHash);
260-
ID.FileDeps = std::move(FD.FileDeps);
261-
ID.ModuleDeps = std::move(FD.ClangModuleDeps);
257+
ID.ContextHash = std::move(TUDeps.ID.ContextHash);
258+
ID.FileDeps = std::move(TUDeps.FileDeps);
259+
ID.ModuleDeps = std::move(TUDeps.ClangModuleDeps);
260+
mergeDeps(std::move(TUDeps.ModuleGraph), InputIndex);
261+
ID.DriverCommandLine = std::move(TUDeps.DriverCommandLine);
262+
ID.Commands = std::move(TUDeps.Commands);
263+
Inputs.push_back(std::move(ID));
264+
}
262265

266+
void mergeDeps(ModuleDepsGraph Graph, size_t InputIndex) {
263267
std::unique_lock<std::mutex> ul(Lock);
264-
for (const ModuleDeps &MD : FDR.DiscoveredModules) {
268+
for (const ModuleDeps &MD : Graph) {
265269
auto I = Modules.find({MD.ID, 0});
266270
if (I != Modules.end()) {
267271
I->first.InputIndex = std::min(I->first.InputIndex, InputIndex);
268272
continue;
269273
}
270274
Modules.insert(I, {{MD.ID, InputIndex}, std::move(MD)});
271275
}
272-
273-
ID.DriverCommandLine = std::move(FD.DriverCommandLine);
274-
ID.Commands = std::move(FD.Commands);
275-
Inputs.push_back(std::move(ID));
276276
}
277277

278278
void printFullOutput(raw_ostream &OS) {
@@ -379,21 +379,38 @@ class FullDeps {
379379
std::vector<InputDeps> Inputs;
380380
};
381381

382-
static bool handleFullDependencyToolResult(
383-
const std::string &Input,
384-
llvm::Expected<FullDependenciesResult> &MaybeFullDeps, FullDeps &FD,
385-
size_t InputIndex, SharedStream &OS, SharedStream &Errs) {
386-
if (!MaybeFullDeps) {
382+
static bool handleTranslationUnitResult(
383+
StringRef Input, llvm::Expected<TranslationUnitDeps> &MaybeTUDeps,
384+
FullDeps &FD, size_t InputIndex, SharedStream &OS, SharedStream &Errs) {
385+
if (!MaybeTUDeps) {
387386
llvm::handleAllErrors(
388-
MaybeFullDeps.takeError(), [&Input, &Errs](llvm::StringError &Err) {
387+
MaybeTUDeps.takeError(), [&Input, &Errs](llvm::StringError &Err) {
389388
Errs.applyLocked([&](raw_ostream &OS) {
390389
OS << "Error while scanning dependencies for " << Input << ":\n";
391390
OS << Err.getMessage();
392391
});
393392
});
394393
return true;
395394
}
396-
FD.mergeDeps(Input, std::move(*MaybeFullDeps), InputIndex);
395+
FD.mergeDeps(Input, std::move(*MaybeTUDeps), InputIndex);
396+
return false;
397+
}
398+
399+
static bool handleModuleResult(
400+
StringRef ModuleName, llvm::Expected<ModuleDepsGraph> &MaybeModuleGraph,
401+
FullDeps &FD, size_t InputIndex, SharedStream &OS, SharedStream &Errs) {
402+
if (!MaybeModuleGraph) {
403+
llvm::handleAllErrors(MaybeModuleGraph.takeError(),
404+
[&ModuleName, &Errs](llvm::StringError &Err) {
405+
Errs.applyLocked([&](raw_ostream &OS) {
406+
OS << "Error while scanning dependencies for "
407+
<< ModuleName << ":\n";
408+
OS << Err.getMessage();
409+
});
410+
});
411+
return true;
412+
}
413+
FD.mergeDeps(std::move(*MaybeModuleGraph), InputIndex);
397414
return false;
398415
}
399416

@@ -571,17 +588,23 @@ int main(int argc, const char **argv) {
571588

572589
// Run the tool on it.
573590
if (Format == ScanningOutputFormat::Make) {
574-
auto MaybeFile = WorkerTools[I]->getDependencyFile(
575-
Input->CommandLine, CWD, MaybeModuleName);
591+
auto MaybeFile =
592+
WorkerTools[I]->getDependencyFile(Input->CommandLine, CWD);
576593
if (handleMakeDependencyToolResult(Filename, MaybeFile, DependencyOS,
577594
Errs))
578595
HadErrors = true;
596+
} else if (MaybeModuleName) {
597+
auto MaybeModuleDepsGraph = WorkerTools[I]->getModuleDependencies(
598+
*MaybeModuleName, Input->CommandLine, CWD, AlreadySeenModules,
599+
LookupOutput);
600+
if (handleModuleResult(*MaybeModuleName, MaybeModuleDepsGraph, FD,
601+
LocalIndex, DependencyOS, Errs))
602+
HadErrors = true;
579603
} else {
580-
auto MaybeFullDeps = WorkerTools[I]->getFullDependencies(
581-
Input->CommandLine, CWD, AlreadySeenModules, LookupOutput,
582-
MaybeModuleName);
583-
if (handleFullDependencyToolResult(Filename, MaybeFullDeps, FD,
584-
LocalIndex, DependencyOS, Errs))
604+
auto MaybeTUDeps = WorkerTools[I]->getTranslationUnitDependencies(
605+
Input->CommandLine, CWD, AlreadySeenModules, LookupOutput);
606+
if (handleTranslationUnitResult(Filename, MaybeTUDeps, FD, LocalIndex,
607+
DependencyOS, Errs))
585608
HadErrors = true;
586609
}
587610
}

0 commit comments

Comments
 (0)