[clang][DepScan] Pass references to ModuleDeps instead of ModuleID in lookupModuleOutput callbacks, NFCI #131688
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
…ModuleID in lookupModuleOutput callbacks, NFC This allows clients to reference more read-only attributes, like IsInStableDirectories.
cyndyishida
requested review from
qiongsiwu,
Bigcheese,
benlangmuir and
jansvoboda11
|
@llvm/pr-subscribers-clang Author: Cyndy Ishida (cyndyishida) ChangesThis allows clients to reference more read-only attributes, like IsInStableDirectories. Full diff: https://github.com/llvm/llvm-project/pull/131688.diff 5 Files Affected:
diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
index d13c3ee76d74f..0809e4f84b9cd 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -26,7 +26,7 @@ namespace dependencies {
/// A callback to lookup module outputs for "-fmodule-file=", "-o" etc.
using LookupModuleOutputCallback =
- std::function<std::string(const ModuleID &, ModuleOutputKind)>;
+ llvm::function_ref<std::string(const ModuleDeps &, ModuleOutputKind)>;
/// Graph of modular dependencies.
using ModuleDepsGraph = std::vector<ModuleDeps>;
@@ -211,16 +211,16 @@ class CallbackActionController : public DependencyActionController {
CallbackActionController(LookupModuleOutputCallback LMO)
: LookupModuleOutput(std::move(LMO)) {
if (!LookupModuleOutput) {
- LookupModuleOutput = [](const ModuleID &,
+ LookupModuleOutput = [](const ModuleDeps &,
ModuleOutputKind) -> std::string {
llvm::report_fatal_error("unexpected call to lookupModuleOutput");
};
}
}
- std::string lookupModuleOutput(const ModuleID &ID,
+ std::string lookupModuleOutput(const ModuleDeps &MD,
ModuleOutputKind Kind) override {
- return LookupModuleOutput(ID, Kind);
+ return LookupModuleOutput(MD, Kind);
}
private:
diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
index 5f0b983ebb58f..3e232c79397ce 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
@@ -68,7 +68,7 @@ class DependencyActionController {
public:
virtual ~DependencyActionController();
- virtual std::string lookupModuleOutput(const ModuleID &ID,
+ virtual std::string lookupModuleOutput(const ModuleDeps &MD,
ModuleOutputKind Kind) = 0;
};
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
index 2b4c2bb76434a..b015e79f400cf 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -118,7 +118,7 @@ llvm::Expected<P1689Rule> DependencyScanningTool::getP1689ModuleDependencyFile(
class P1689ActionController : public DependencyActionController {
public:
// The lookupModuleOutput is for clang modules. P1689 format don't need it.
- std::string lookupModuleOutput(const ModuleID &,
+ std::string lookupModuleOutput(const ModuleDeps &,
ModuleOutputKind Kind) override {
return "";
}
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 1c36039efeae7..d715ef874e002 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -196,17 +196,17 @@ static std::vector<std::string> splitString(std::string S, char Separator) {
void ModuleDepCollector::addOutputPaths(CowCompilerInvocation &CI,
ModuleDeps &Deps) {
CI.getMutFrontendOpts().OutputFile =
- Controller.lookupModuleOutput(Deps.ID, ModuleOutputKind::ModuleFile);
+ Controller.lookupModuleOutput(Deps, ModuleOutputKind::ModuleFile);
if (!CI.getDiagnosticOpts().DiagnosticSerializationFile.empty())
CI.getMutDiagnosticOpts().DiagnosticSerializationFile =
Controller.lookupModuleOutput(
- Deps.ID, ModuleOutputKind::DiagnosticSerializationFile);
+ Deps, ModuleOutputKind::DiagnosticSerializationFile);
if (!CI.getDependencyOutputOpts().OutputFile.empty()) {
- CI.getMutDependencyOutputOpts().OutputFile = Controller.lookupModuleOutput(
- Deps.ID, ModuleOutputKind::DependencyFile);
+ CI.getMutDependencyOutputOpts().OutputFile =
+ Controller.lookupModuleOutput(Deps, ModuleOutputKind::DependencyFile);
CI.getMutDependencyOutputOpts().Targets =
splitString(Controller.lookupModuleOutput(
- Deps.ID, ModuleOutputKind::DependencyTargets),
+ Deps, ModuleOutputKind::DependencyTargets),
'\0');
if (!CI.getDependencyOutputOpts().OutputFile.empty() &&
CI.getDependencyOutputOpts().Targets.empty()) {
@@ -404,8 +404,10 @@ void ModuleDepCollector::addModuleMapFiles(
void ModuleDepCollector::addModuleFiles(
CompilerInvocation &CI, ArrayRef<ModuleID> ClangModuleDeps) const {
for (const ModuleID &MID : ClangModuleDeps) {
+ ModuleDeps *MD = ModuleDepsByID.lookup(MID);
std::string PCMPath =
- Controller.lookupModuleOutput(MID, ModuleOutputKind::ModuleFile);
+ Controller.lookupModuleOutput(*MD, ModuleOutputKind::ModuleFile);
+
if (Service.shouldEagerLoadModules())
CI.getFrontendOpts().ModuleFiles.push_back(std::move(PCMPath));
else
@@ -417,8 +419,10 @@ void ModuleDepCollector::addModuleFiles(
void ModuleDepCollector::addModuleFiles(
CowCompilerInvocation &CI, ArrayRef<ModuleID> ClangModuleDeps) const {
for (const ModuleID &MID : ClangModuleDeps) {
+ ModuleDeps *MD = ModuleDepsByID.lookup(MID);
std::string PCMPath =
- Controller.lookupModuleOutput(MID, ModuleOutputKind::ModuleFile);
+ Controller.lookupModuleOutput(*MD, ModuleOutputKind::ModuleFile);
+
if (Service.shouldEagerLoadModules())
CI.getMutFrontendOpts().ModuleFiles.push_back(std::move(PCMPath));
else
diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index 5b255974cea15..fa63649bb9028 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -709,9 +709,10 @@ static std::string constructPCMPath(ModuleID MID, StringRef OutputDir) {
return std::string(ExplicitPCMPath);
}
-static std::string lookupModuleOutput(const ModuleID &MID, ModuleOutputKind MOK,
+static std::string lookupModuleOutput(const ModuleDeps &MD,
+ ModuleOutputKind MOK,
StringRef OutputDir) {
- std::string PCMPath = constructPCMPath(MID, OutputDir);
+ std::string PCMPath = constructPCMPath(MD.ID, OutputDir);
switch (MOK) {
case ModuleOutputKind::ModuleFile:
return PCMPath;
@@ -974,8 +975,8 @@ int clang_scan_deps_main(int argc, char **argv, const llvm::ToolContext &) {
std::string OutputDir(ModuleFilesDir);
if (OutputDir.empty())
OutputDir = getModuleCachePath(Input->CommandLine);
- auto LookupOutput = [&](const ModuleID &MID, ModuleOutputKind MOK) {
- return ::lookupModuleOutput(MID, MOK, OutputDir);
+ auto LookupOutput = [&](const ModuleDeps &MD, ModuleOutputKind MOK) {
+ return ::lookupModuleOutput(MD, MOK, OutputDir);
};
// Run the tool on it.
|
Bigcheese
approved these changes
Mar 17, 2025
cyndyishida
added a commit
to cyndyishida/llvm-project
that referenced
this pull request
Mar 17, 2025
… lookupModuleOutput callbacks, NFCI (llvm#131688) This allows clients to reference more read-only attributes, like IsInStableDirectories. (cherry picked from commit ad8f0e2)
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/51/builds/12720 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/186/builds/7389 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/168/builds/9829 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/72/builds/9252 Here is the relevant piece of the build log for the reference |
cyndyishida
added a commit
to cyndyishida/llvm-project
that referenced
this pull request
Mar 18, 2025
… lookupModuleOutput callbacks, NFCI (llvm#131688) This allows clients to reference more read-only attributes, like IsInStableDirectories. (cherry picked from commit ad8f0e2)
cyndyishida
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Mar 19, 2025
… lookupModuleOutput callbacks, NFCI (llvm#131688) This allows clients to reference more read-only attributes, like IsInStableDirectories. (cherry picked from commit ad8f0e2)
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 allows clients to reference more read-only attributes, like IsInStableDirectories.