[PATCH] D131934: [clang][deps] Compute command-lines for dependencies immediately
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5482432bf6cc: [clang][deps] Compute command-lines for
dependencies immediately (authored by benlangmuir).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131934/new/
https://reviews.llvm.org/D131934
Files:
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/tools/clang-scan-deps/ClangScanDeps.cpp
Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -6,6 +6,7 @@
//
//===--===//
+#include "clang/Driver/Driver.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Tooling/CommonOptionsParser.h"
#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
@@ -268,10 +269,7 @@
Modules.insert(I, {{MD.ID, InputIndex}, std::move(MD)});
}
-ID.CommandLine =
-FD.getCommandLine([&](const ModuleID , ModuleOutputKind MOK) {
- return lookupModuleOutput(MID, MOK);
-});
+ID.CommandLine = FD.CommandLine;
Inputs.push_back(std::move(ID));
}
@@ -301,10 +299,7 @@
{"file-deps", toJSONSorted(MD.FileDeps)},
{"clang-module-deps", toJSONSorted(MD.ClangModuleDeps)},
{"clang-modulemap-file", MD.ClangModuleMapFile},
- {"command-line", MD.getCanonicalCommandLine(
- [&](const ModuleID , ModuleOutputKind MOK) {
- return lookupModuleOutput(MID, MOK);
- })},
+ {"command-line", MD.getCanonicalCommandLine()},
};
OutModules.push_back(std::move(O));
}
@@ -330,42 +325,6 @@
}
private:
- std::string lookupModuleOutput(const ModuleID , ModuleOutputKind MOK) {
-// Cache the PCM path, since it will be queried repeatedly for each module.
-// The other outputs are only queried once during getCanonicalCommandLine.
-auto PCMPath = PCMPaths.insert({MID, ""});
-if (PCMPath.second)
- PCMPath.first->second = constructPCMPath(MID);
-switch (MOK) {
-case ModuleOutputKind::ModuleFile:
- return PCMPath.first->second;
-case ModuleOutputKind::DependencyFile:
- return PCMPath.first->second + ".d";
-case ModuleOutputKind::DependencyTargets:
- // Null-separate the list of targets.
- return join(ModuleDepTargets, StringRef("\0", 1));
-case ModuleOutputKind::DiagnosticSerializationFile:
- return PCMPath.first->second + ".diag";
-}
-llvm_unreachable("Fully covered switch above!");
- }
-
- /// Construct a path for the explicitly built PCM.
- std::string constructPCMPath(ModuleID MID) const {
-auto MDIt = Modules.find(IndexedModuleID{MID, 0});
-assert(MDIt != Modules.end());
-const ModuleDeps = MDIt->second;
-
-StringRef Filename = llvm::sys::path::filename(MD.ImplicitModulePCMPath);
-StringRef ModuleCachePath = llvm::sys::path::parent_path(
-llvm::sys::path::parent_path(MD.ImplicitModulePCMPath));
-
-SmallString<256> ExplicitPCMPath(!ModuleFilesDir.empty() ? ModuleFilesDir
- : ModuleCachePath);
-llvm::sys::path::append(ExplicitPCMPath, MD.ID.ContextHash, Filename);
-return std::string(ExplicitPCMPath);
- }
-
struct IndexedModuleID {
ModuleID ID;
mutable size_t InputIndex;
@@ -395,7 +354,6 @@
std::mutex Lock;
std::unordered_map
Modules;
- std::unordered_map PCMPaths;
std::vector Inputs;
};
@@ -417,6 +375,42 @@
return false;
}
+/// Construct a path for the explicitly built PCM.
+static std::string constructPCMPath(ModuleID MID, StringRef OutputDir) {
+ SmallString<256> ExplicitPCMPath(OutputDir);
+ llvm::sys::path::append(ExplicitPCMPath, MID.ContextHash,
+ MID.ModuleName + "-" + MID.ContextHash + ".pcm");
+ return std::string(ExplicitPCMPath);
+}
+
+static std::string lookupModuleOutput(const ModuleID , ModuleOutputKind MOK,
+ StringRef OutputDir) {
+ std::string PCMPath = constructPCMPath(MID, OutputDir);
+ switch (MOK) {
+ case ModuleOutputKind::ModuleFile:
+return PCMPath;
+ case ModuleOutputKind::DependencyFile:
+return PCMPath + ".d";
+ case ModuleOutputKind::DependencyTargets:
+// Null-separate the list of targets.
+return join(ModuleDepTargets, StringRef("\0", 1));
+ case ModuleOutputKind::DiagnosticSerializationFile:
+return PCMPath +
[PATCH] D131934: [clang][deps] Compute command-lines for dependencies immediately
benlangmuir marked 2 inline comments as done.
benlangmuir added inline comments.
Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:403
+
+static std::string getModuleCachePath(ArrayRef Args) {
+ for (StringRef Arg : llvm::reverse(Args)) {
jansvoboda11 wrote:
> Can you split this into separate patch?
This is hard because we don't have the association between input command-line
and module dependencies in the previous code. Per our offline discussion I've
mentioned this in the commit message instead.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131934/new/
https://reviews.llvm.org/D131934
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D131934: [clang][deps] Compute command-lines for dependencies immediately
benlangmuir updated this revision to Diff 453085.
benlangmuir edited the summary of this revision.
benlangmuir added a comment.
Per review
- Add typedef for callback type
- Add comment about mapping paths to "-"
- Update commit message for module cache path computation change
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131934/new/
https://reviews.llvm.org/D131934
Files:
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/tools/clang-scan-deps/ClangScanDeps.cpp
Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -6,6 +6,7 @@
//
//===--===//
+#include "clang/Driver/Driver.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Tooling/CommonOptionsParser.h"
#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
@@ -268,10 +269,7 @@
Modules.insert(I, {{MD.ID, InputIndex}, std::move(MD)});
}
-ID.CommandLine =
-FD.getCommandLine([&](const ModuleID , ModuleOutputKind MOK) {
- return lookupModuleOutput(MID, MOK);
-});
+ID.CommandLine = FD.CommandLine;
Inputs.push_back(std::move(ID));
}
@@ -301,10 +299,7 @@
{"file-deps", toJSONSorted(MD.FileDeps)},
{"clang-module-deps", toJSONSorted(MD.ClangModuleDeps)},
{"clang-modulemap-file", MD.ClangModuleMapFile},
- {"command-line", MD.getCanonicalCommandLine(
- [&](const ModuleID , ModuleOutputKind MOK) {
- return lookupModuleOutput(MID, MOK);
- })},
+ {"command-line", MD.getCanonicalCommandLine()},
};
OutModules.push_back(std::move(O));
}
@@ -330,42 +325,6 @@
}
private:
- std::string lookupModuleOutput(const ModuleID , ModuleOutputKind MOK) {
-// Cache the PCM path, since it will be queried repeatedly for each module.
-// The other outputs are only queried once during getCanonicalCommandLine.
-auto PCMPath = PCMPaths.insert({MID, ""});
-if (PCMPath.second)
- PCMPath.first->second = constructPCMPath(MID);
-switch (MOK) {
-case ModuleOutputKind::ModuleFile:
- return PCMPath.first->second;
-case ModuleOutputKind::DependencyFile:
- return PCMPath.first->second + ".d";
-case ModuleOutputKind::DependencyTargets:
- // Null-separate the list of targets.
- return join(ModuleDepTargets, StringRef("\0", 1));
-case ModuleOutputKind::DiagnosticSerializationFile:
- return PCMPath.first->second + ".diag";
-}
-llvm_unreachable("Fully covered switch above!");
- }
-
- /// Construct a path for the explicitly built PCM.
- std::string constructPCMPath(ModuleID MID) const {
-auto MDIt = Modules.find(IndexedModuleID{MID, 0});
-assert(MDIt != Modules.end());
-const ModuleDeps = MDIt->second;
-
-StringRef Filename = llvm::sys::path::filename(MD.ImplicitModulePCMPath);
-StringRef ModuleCachePath = llvm::sys::path::parent_path(
-llvm::sys::path::parent_path(MD.ImplicitModulePCMPath));
-
-SmallString<256> ExplicitPCMPath(!ModuleFilesDir.empty() ? ModuleFilesDir
- : ModuleCachePath);
-llvm::sys::path::append(ExplicitPCMPath, MD.ID.ContextHash, Filename);
-return std::string(ExplicitPCMPath);
- }
-
struct IndexedModuleID {
ModuleID ID;
mutable size_t InputIndex;
@@ -395,7 +354,6 @@
std::mutex Lock;
std::unordered_map
Modules;
- std::unordered_map PCMPaths;
std::vector Inputs;
};
@@ -417,6 +375,42 @@
return false;
}
+/// Construct a path for the explicitly built PCM.
+static std::string constructPCMPath(ModuleID MID, StringRef OutputDir) {
+ SmallString<256> ExplicitPCMPath(OutputDir);
+ llvm::sys::path::append(ExplicitPCMPath, MID.ContextHash,
+ MID.ModuleName + "-" + MID.ContextHash + ".pcm");
+ return std::string(ExplicitPCMPath);
+}
+
+static std::string lookupModuleOutput(const ModuleID , ModuleOutputKind MOK,
+ StringRef OutputDir) {
+ std::string PCMPath = constructPCMPath(MID, OutputDir);
+ switch (MOK) {
+ case ModuleOutputKind::ModuleFile:
+return PCMPath;
+ case ModuleOutputKind::DependencyFile:
+return PCMPath + ".d";
+ case ModuleOutputKind::DependencyTargets:
+// Null-separate the list of targets.
+return join(ModuleDepTargets, StringRef("\0", 1));
+ case
[PATCH] D131934: [clang][deps] Compute command-lines for dependencies immediately
jansvoboda11 accepted this revision.
jansvoboda11 added a comment.
This revision is now accepted and ready to land.
LGTM overall with some minor nitpicks.
Comment at:
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h:95
+ const llvm::StringSet<> ,
+ llvm::function_ref
+ LookupModuleOutput,
Creating a type alias might make this more readable & easier to refactor later.
Comment at: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp:102
+ if (!CI.getDiagnosticOpts().DiagnosticSerializationFile.empty())
+CI.getDiagnosticOpts().DiagnosticSerializationFile = "-";
+ if (!CI.getDependencyOutputOpts().OutputFile.empty())
Can you point out in a comment that this value is just temporary for context
hash computation and will be replaced by real path later on?
Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:403
+
+static std::string getModuleCachePath(ArrayRef Args) {
+ for (StringRef Arg : llvm::reverse(Args)) {
Can you split this into separate patch?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131934/new/
https://reviews.llvm.org/D131934
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D131934: [clang][deps] Compute command-lines for dependencies immediately
benlangmuir created this revision.
benlangmuir added a reviewer: jansvoboda11.
Herald added a project: All.
benlangmuir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Instead of delaying the generation of command-lines to after all
dependencies are reported, compute them immediately. This is partly in
preparation for splitting the TU driver command into its constituent cc1
and other jobs, but it also just simplifies working with the compiler
invocation for modules if they are not "without paths".
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D131934
Files:
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/tools/clang-scan-deps/ClangScanDeps.cpp
Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -6,6 +6,7 @@
//
//===--===//
+#include "clang/Driver/Driver.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Tooling/CommonOptionsParser.h"
#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
@@ -268,10 +269,7 @@
Modules.insert(I, {{MD.ID, InputIndex}, std::move(MD)});
}
-ID.CommandLine =
-FD.getCommandLine([&](const ModuleID , ModuleOutputKind MOK) {
- return lookupModuleOutput(MID, MOK);
-});
+ID.CommandLine = FD.CommandLine;
Inputs.push_back(std::move(ID));
}
@@ -301,10 +299,7 @@
{"file-deps", toJSONSorted(MD.FileDeps)},
{"clang-module-deps", toJSONSorted(MD.ClangModuleDeps)},
{"clang-modulemap-file", MD.ClangModuleMapFile},
- {"command-line", MD.getCanonicalCommandLine(
- [&](const ModuleID , ModuleOutputKind MOK) {
- return lookupModuleOutput(MID, MOK);
- })},
+ {"command-line", MD.getCanonicalCommandLine()},
};
OutModules.push_back(std::move(O));
}
@@ -330,42 +325,6 @@
}
private:
- std::string lookupModuleOutput(const ModuleID , ModuleOutputKind MOK) {
-// Cache the PCM path, since it will be queried repeatedly for each module.
-// The other outputs are only queried once during getCanonicalCommandLine.
-auto PCMPath = PCMPaths.insert({MID, ""});
-if (PCMPath.second)
- PCMPath.first->second = constructPCMPath(MID);
-switch (MOK) {
-case ModuleOutputKind::ModuleFile:
- return PCMPath.first->second;
-case ModuleOutputKind::DependencyFile:
- return PCMPath.first->second + ".d";
-case ModuleOutputKind::DependencyTargets:
- // Null-separate the list of targets.
- return join(ModuleDepTargets, StringRef("\0", 1));
-case ModuleOutputKind::DiagnosticSerializationFile:
- return PCMPath.first->second + ".diag";
-}
-llvm_unreachable("Fully covered switch above!");
- }
-
- /// Construct a path for the explicitly built PCM.
- std::string constructPCMPath(ModuleID MID) const {
-auto MDIt = Modules.find(IndexedModuleID{MID, 0});
-assert(MDIt != Modules.end());
-const ModuleDeps = MDIt->second;
-
-StringRef Filename = llvm::sys::path::filename(MD.ImplicitModulePCMPath);
-StringRef ModuleCachePath = llvm::sys::path::parent_path(
-llvm::sys::path::parent_path(MD.ImplicitModulePCMPath));
-
-SmallString<256> ExplicitPCMPath(!ModuleFilesDir.empty() ? ModuleFilesDir
- : ModuleCachePath);
-llvm::sys::path::append(ExplicitPCMPath, MD.ID.ContextHash, Filename);
-return std::string(ExplicitPCMPath);
- }
-
struct IndexedModuleID {
ModuleID ID;
mutable size_t InputIndex;
@@ -395,7 +354,6 @@
std::mutex Lock;
std::unordered_map
Modules;
- std::unordered_map PCMPaths;
std::vector Inputs;
};
@@ -417,6 +375,42 @@
return false;
}
+/// Construct a path for the explicitly built PCM.
+static std::string constructPCMPath(ModuleID MID, StringRef OutputDir) {
+ SmallString<256> ExplicitPCMPath(OutputDir);
+ llvm::sys::path::append(ExplicitPCMPath, MID.ContextHash,
+ MID.ModuleName + "-" + MID.ContextHash + ".pcm");
+ return std::string(ExplicitPCMPath);
+}
+
+static std::string lookupModuleOutput(const ModuleID , ModuleOutputKind MOK,
+ StringRef OutputDir) {
+ std::string PCMPath = constructPCMPath(MID, OutputDir);
+ switch (MOK) {
+ case ModuleOutputKind::ModuleFile:
+return PCMPath;
