[clang] ba55666 - [clang][deps] NFC: Split out the module-based API from the TU-based API
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Wed Feb 1 13:46:35 PST 2023
Author: Jan Svoboda
Date: 2023-02-01T13:46:26-08:00
New Revision: ba556660fe52a558c34556866aba4a0bb8bbbd23
URL: https://github.com/llvm/llvm-project/commit/ba556660fe52a558c34556866aba4a0bb8bbbd23
DIFF: https://github.com/llvm/llvm-project/commit/ba556660fe52a558c34556866aba4a0bb8bbbd23.diff
LOG: [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
Added:
Modified:
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
clang/test/ClangScanDeps/modules-full-by-mod-name.cpp
clang/tools/clang-scan-deps/ClangScanDeps.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
index 9d247f96c30b..52a08d294dcf 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -28,8 +28,14 @@ namespace dependencies {
using LookupModuleOutputCallback =
llvm::function_ref<std::string(const ModuleID &, ModuleOutputKind)>;
+/// Graph of modular dependencies.
+using ModuleDepsGraph = std::vector<ModuleDeps>;
+
/// The full dependencies and module graph for a specific input.
-struct FullDependencies {
+struct TranslationUnitDeps {
+ /// The graph of direct and transitive modular dependencies.
+ ModuleDepsGraph ModuleGraph;
+
/// The identifier of the C++20 module this translation unit exports.
///
/// If the translation unit is not a module then \c ID.ModuleName is empty.
@@ -62,11 +68,6 @@ struct FullDependencies {
std::vector<std::string> DriverCommandLine;
};
-struct FullDependenciesResult {
- FullDependencies FullDeps;
- std::vector<ModuleDeps> DiscoveredModules;
-};
-
/// The high-level implementation of the dependency discovery tool that runs on
/// an individual worker thread.
class DependencyScanningTool {
@@ -78,18 +79,15 @@ class DependencyScanningTool {
/// Print out the dependency information into a string using the dependency
/// file format that is specified in the options (-MD is the default) and
- /// return it. If \p ModuleName isn't empty, this function returns the
- /// dependency information of module \p ModuleName.
+ /// return it.
///
/// \returns A \c StringError with the diagnostic output if clang errors
/// occurred, dependency file contents otherwise.
llvm::Expected<std::string>
- getDependencyFile(const std::vector<std::string> &CommandLine, StringRef CWD,
- std::optional<StringRef> ModuleName = std::nullopt);
+ getDependencyFile(const std::vector<std::string> &CommandLine, StringRef CWD);
- /// Collect the full module dependency graph for the input, ignoring any
- /// modules which have already been seen. If \p ModuleName isn't empty, this
- /// function returns the full dependency information of module \p ModuleName.
+ /// Given a Clang driver command-line for a translation unit, gather the
+ /// modular dependencies and return the information needed for explicit build.
///
/// \param AlreadySeen This stores modules which have previously been
/// reported. Use the same instance for all calls to this
@@ -101,12 +99,21 @@ class DependencyScanningTool {
/// arguments for dependencies.
///
/// \returns a \c StringError with the diagnostic output if clang errors
- /// occurred, \c FullDependencies otherwise.
- llvm::Expected<FullDependenciesResult>
- getFullDependencies(const std::vector<std::string> &CommandLine,
- StringRef CWD, const llvm::StringSet<> &AlreadySeen,
- LookupModuleOutputCallback LookupModuleOutput,
- std::optional<StringRef> ModuleName = std::nullopt);
+ /// occurred, \c TranslationUnitDeps otherwise.
+ llvm::Expected<TranslationUnitDeps>
+ getTranslationUnitDependencies(const std::vector<std::string> &CommandLine,
+ StringRef CWD,
+ const llvm::StringSet<> &AlreadySeen,
+ LookupModuleOutputCallback LookupModuleOutput);
+
+ /// Given a compilation context specified via the Clang driver command-line,
+ /// gather modular dependencies of module with the given name, and return the
+ /// information needed for explicit build.
+ llvm::Expected<ModuleDepsGraph>
+ getModuleDependencies(StringRef ModuleName,
+ const std::vector<std::string> &CommandLine,
+ StringRef CWD, const llvm::StringSet<> &AlreadySeen,
+ LookupModuleOutputCallback LookupModuleOutput);
private:
DependencyScanningWorker Worker;
@@ -145,7 +152,8 @@ class FullDependencyConsumer : public DependencyConsumer {
return LookupModuleOutput(ID, Kind);
}
- FullDependenciesResult takeFullDependencies();
+ TranslationUnitDeps takeTranslationUnitDeps();
+ ModuleDepsGraph takeModuleGraphDeps();
private:
std::vector<std::string> Dependencies;
diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
index 458c4d936c83..5fb8d52f9ff0 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
@@ -31,7 +31,7 @@ class DependencyScanningWorkerFilesystem;
/// A command-line tool invocation that is part of building a TU.
///
-/// \see FullDependencies::Commands.
+/// \see TranslationUnitDeps::Commands.
struct Command {
std::string Executable;
std::vector<std::string> Arguments;
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
index 14d1859626f9..a7ab4dd3af6d 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -20,8 +20,7 @@ DependencyScanningTool::DependencyScanningTool(
: Worker(Service, std::move(FS)) {}
llvm::Expected<std::string> DependencyScanningTool::getDependencyFile(
- const std::vector<std::string> &CommandLine, StringRef CWD,
- std::optional<StringRef> ModuleName) {
+ const std::vector<std::string> &CommandLine, StringRef CWD) {
/// Prints out all of the gathered dependencies into a string.
class MakeDependencyPrinterConsumer : public DependencyConsumer {
public:
@@ -81,8 +80,7 @@ llvm::Expected<std::string> DependencyScanningTool::getDependencyFile(
};
MakeDependencyPrinterConsumer Consumer;
- auto Result =
- Worker.computeDependencies(CWD, CommandLine, Consumer, ModuleName);
+ auto Result = Worker.computeDependencies(CWD, CommandLine, Consumer);
if (Result)
return std::move(Result);
std::string Output;
@@ -90,39 +88,63 @@ llvm::Expected<std::string> DependencyScanningTool::getDependencyFile(
return Output;
}
-llvm::Expected<FullDependenciesResult>
-DependencyScanningTool::getFullDependencies(
+llvm::Expected<TranslationUnitDeps>
+DependencyScanningTool::getTranslationUnitDependencies(
const std::vector<std::string> &CommandLine, StringRef CWD,
const llvm::StringSet<> &AlreadySeen,
- LookupModuleOutputCallback LookupModuleOutput,
- std::optional<StringRef> ModuleName) {
+ LookupModuleOutputCallback LookupModuleOutput) {
+ FullDependencyConsumer Consumer(AlreadySeen, LookupModuleOutput);
+ llvm::Error Result = Worker.computeDependencies(CWD, CommandLine, Consumer);
+ if (Result)
+ return std::move(Result);
+ return Consumer.takeTranslationUnitDeps();
+}
+
+llvm::Expected<ModuleDepsGraph> DependencyScanningTool::getModuleDependencies(
+ StringRef ModuleName, const std::vector<std::string> &CommandLine,
+ StringRef CWD, const llvm::StringSet<> &AlreadySeen,
+ LookupModuleOutputCallback LookupModuleOutput) {
FullDependencyConsumer Consumer(AlreadySeen, LookupModuleOutput);
llvm::Error Result =
Worker.computeDependencies(CWD, CommandLine, Consumer, ModuleName);
if (Result)
return std::move(Result);
- return Consumer.takeFullDependencies();
+ return Consumer.takeModuleGraphDeps();
}
-FullDependenciesResult FullDependencyConsumer::takeFullDependencies() {
- FullDependenciesResult FDR;
- FullDependencies &FD = FDR.FullDeps;
+TranslationUnitDeps FullDependencyConsumer::takeTranslationUnitDeps() {
+ TranslationUnitDeps TU;
- FD.ID.ContextHash = std::move(ContextHash);
- FD.FileDeps = std::move(Dependencies);
- FD.PrebuiltModuleDeps = std::move(PrebuiltModuleDeps);
- FD.Commands = std::move(Commands);
+ TU.ID.ContextHash = std::move(ContextHash);
+ TU.FileDeps = std::move(Dependencies);
+ TU.PrebuiltModuleDeps = std::move(PrebuiltModuleDeps);
+ TU.Commands = std::move(Commands);
for (auto &&M : ClangModuleDeps) {
auto &MD = M.second;
if (MD.ImportedByMainFile)
- FD.ClangModuleDeps.push_back(MD.ID);
+ 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));
+ }
+
+ return TU;
+}
+
+ModuleDepsGraph FullDependencyConsumer::takeModuleGraphDeps() {
+ ModuleDepsGraph ModuleGraph;
+
+ for (auto &&M : ClangModuleDeps) {
+ auto &MD = M.second;
// TODO: Avoid handleModuleDependency even being called for modules
// we've already seen.
if (AlreadySeen.count(M.first))
continue;
- FDR.DiscoveredModules.push_back(std::move(MD));
+ ModuleGraph.push_back(std::move(MD));
}
- return FDR;
+ return ModuleGraph;
}
diff --git a/clang/test/ClangScanDeps/modules-full-by-mod-name.cpp b/clang/test/ClangScanDeps/modules-full-by-mod-name.cpp
index 41b1ef6ebe90..4109747bf48e 100644
--- a/clang/test/ClangScanDeps/modules-full-by-mod-name.cpp
+++ b/clang/test/ClangScanDeps/modules-full-by-mod-name.cpp
@@ -75,3 +75,5 @@
// CHECK-NEXT: "name": "header2"
// CHECK-NEXT: }
// CHECK-NEXT: ],
+// CHECK-NEXT: "translation-units": []
+// CHECK-NEXT: }
diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index b8256f640122..41bc0ea7f024 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -250,18 +250,22 @@ static llvm::json::Array toJSONSorted(std::vector<ModuleID> V) {
// Thread safe.
class FullDeps {
public:
- void mergeDeps(StringRef Input, FullDependenciesResult FDR,
+ void mergeDeps(StringRef Input, TranslationUnitDeps TUDeps,
size_t InputIndex) {
- FullDependencies &FD = FDR.FullDeps;
-
InputDeps ID;
ID.FileName = std::string(Input);
- ID.ContextHash = std::move(FD.ID.ContextHash);
- ID.FileDeps = std::move(FD.FileDeps);
- ID.ModuleDeps = std::move(FD.ClangModuleDeps);
+ ID.ContextHash = std::move(TUDeps.ID.ContextHash);
+ ID.FileDeps = std::move(TUDeps.FileDeps);
+ ID.ModuleDeps = std::move(TUDeps.ClangModuleDeps);
+ mergeDeps(std::move(TUDeps.ModuleGraph), InputIndex);
+ ID.DriverCommandLine = std::move(TUDeps.DriverCommandLine);
+ ID.Commands = std::move(TUDeps.Commands);
+ Inputs.push_back(std::move(ID));
+ }
+ void mergeDeps(ModuleDepsGraph Graph, size_t InputIndex) {
std::unique_lock<std::mutex> ul(Lock);
- for (const ModuleDeps &MD : FDR.DiscoveredModules) {
+ for (const ModuleDeps &MD : Graph) {
auto I = Modules.find({MD.ID, 0});
if (I != Modules.end()) {
I->first.InputIndex = std::min(I->first.InputIndex, InputIndex);
@@ -269,10 +273,6 @@ class FullDeps {
}
Modules.insert(I, {{MD.ID, InputIndex}, std::move(MD)});
}
-
- ID.DriverCommandLine = std::move(FD.DriverCommandLine);
- ID.Commands = std::move(FD.Commands);
- Inputs.push_back(std::move(ID));
}
void printFullOutput(raw_ostream &OS) {
@@ -379,13 +379,12 @@ class FullDeps {
std::vector<InputDeps> Inputs;
};
-static bool handleFullDependencyToolResult(
- const std::string &Input,
- llvm::Expected<FullDependenciesResult> &MaybeFullDeps, FullDeps &FD,
- size_t InputIndex, SharedStream &OS, SharedStream &Errs) {
- if (!MaybeFullDeps) {
+static bool handleTranslationUnitResult(
+ StringRef Input, llvm::Expected<TranslationUnitDeps> &MaybeTUDeps,
+ FullDeps &FD, size_t InputIndex, SharedStream &OS, SharedStream &Errs) {
+ if (!MaybeTUDeps) {
llvm::handleAllErrors(
- MaybeFullDeps.takeError(), [&Input, &Errs](llvm::StringError &Err) {
+ MaybeTUDeps.takeError(), [&Input, &Errs](llvm::StringError &Err) {
Errs.applyLocked([&](raw_ostream &OS) {
OS << "Error while scanning dependencies for " << Input << ":\n";
OS << Err.getMessage();
@@ -393,7 +392,25 @@ static bool handleFullDependencyToolResult(
});
return true;
}
- FD.mergeDeps(Input, std::move(*MaybeFullDeps), InputIndex);
+ FD.mergeDeps(Input, std::move(*MaybeTUDeps), InputIndex);
+ return false;
+}
+
+static bool handleModuleResult(
+ StringRef ModuleName, llvm::Expected<ModuleDepsGraph> &MaybeModuleGraph,
+ FullDeps &FD, size_t InputIndex, SharedStream &OS, SharedStream &Errs) {
+ if (!MaybeModuleGraph) {
+ llvm::handleAllErrors(MaybeModuleGraph.takeError(),
+ [&ModuleName, &Errs](llvm::StringError &Err) {
+ Errs.applyLocked([&](raw_ostream &OS) {
+ OS << "Error while scanning dependencies for "
+ << ModuleName << ":\n";
+ OS << Err.getMessage();
+ });
+ });
+ return true;
+ }
+ FD.mergeDeps(std::move(*MaybeModuleGraph), InputIndex);
return false;
}
@@ -571,17 +588,23 @@ int main(int argc, const char **argv) {
// Run the tool on it.
if (Format == ScanningOutputFormat::Make) {
- auto MaybeFile = WorkerTools[I]->getDependencyFile(
- Input->CommandLine, CWD, MaybeModuleName);
+ auto MaybeFile =
+ WorkerTools[I]->getDependencyFile(Input->CommandLine, CWD);
if (handleMakeDependencyToolResult(Filename, MaybeFile, DependencyOS,
Errs))
HadErrors = true;
+ } else if (MaybeModuleName) {
+ auto MaybeModuleDepsGraph = WorkerTools[I]->getModuleDependencies(
+ *MaybeModuleName, Input->CommandLine, CWD, AlreadySeenModules,
+ LookupOutput);
+ if (handleModuleResult(*MaybeModuleName, MaybeModuleDepsGraph, FD,
+ LocalIndex, DependencyOS, Errs))
+ HadErrors = true;
} else {
- auto MaybeFullDeps = WorkerTools[I]->getFullDependencies(
- Input->CommandLine, CWD, AlreadySeenModules, LookupOutput,
- MaybeModuleName);
- if (handleFullDependencyToolResult(Filename, MaybeFullDeps, FD,
- LocalIndex, DependencyOS, Errs))
+ auto MaybeTUDeps = WorkerTools[I]->getTranslationUnitDependencies(
+ Input->CommandLine, CWD, AlreadySeenModules, LookupOutput);
+ if (handleTranslationUnitResult(Filename, MaybeTUDeps, FD, LocalIndex,
+ DependencyOS, Errs))
HadErrors = true;
}
}
More information about the cfe-commits
mailing list