Skip to content

[clang][clang-scan-deps] Add named modules to format 'experimental-full' #145221

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jun 24, 2025
Merged

[clang][clang-scan-deps] Add named modules to format 'experimental-full' #145221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
981742e
[clang][clang-scan-deps] Add named modules to format 'experimental-full'
naveen-seth Jun 21, 2025
08b470d
Omit 'named-module-deps: […]' when empty to avoid updating all tests
naveen-seth Jun 23, 2025
1b4e90e
Avoid implicit conversion in non-empty check
naveen-seth Jun 23, 2025
2cde5be
"module-name" -> "named-module"
naveen-seth Jun 23, 2025
2628a65
Let consumers decide whether to handle named module scan results
naveen-seth Jun 23, 2025
868934a
Fix wording mistake in comment
naveen-seth Jun 24, 2025
74772e0
Make test windows-compatible
naveen-seth Jun 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "clang/Tooling/JSONCompilationDatabase.h"
#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/MapVector.h"
#include "llvm/ADT/STLExtras.h"
#include <functional>
#include <optional>
#include <string>
Expand Down Expand Up @@ -56,6 +57,9 @@ struct TranslationUnitDeps {
/// determined that the differences are benign for this compilation.
std::vector<ModuleID> ClangModuleDeps;

/// A list of the C++20 named modules this translation unit depends on.
std::vector<std::string> NamedModuleDeps;

/// The sequence of commands required to build the translation unit. Commands
/// should be executed in order.
///
Expand Down Expand Up @@ -188,13 +192,23 @@ class FullDependencyConsumer : public DependencyConsumer {
ContextHash = std::move(Hash);
}

void handleProvidedAndRequiredStdCXXModules(
std::optional<P1689ModuleInfo> Provided,
std::vector<P1689ModuleInfo> Requires) override {
ModuleName = Provided ? Provided->ModuleName : "";
Copy link
Collaborator

@shafik shafik Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static analysis flagged code around here and I am stuck trying to understand what ModuleName is being used for here. Is this dead code? If not there should really be a comment explaining how it is used.

Copy link
Contributor Author

@naveen-seth naveen-seth Aug 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ModuleName stores the name of the exported standard C++ module.
After takeTranslationUnitDeps(), it is stored in the TranslationUnitDeps.ID.ModuleName field (here).

I don't think this should be dead code because it is used in ClangScanDeps.cpp when using the --experimental-full format (first reassigned here, then used for printing here).

Maybe it would help improve clarity if we keep ContextHash and ModuleName together in a ModuleID field within FullDependencyConsumer, just as TranslationUnitDeps does, rather than keeping them ungrouped.
This would then more directly lead to the documentation of ModuleID.ModuleName (here).

llvm::transform(Requires, std::back_inserter(NamedModuleDeps),
[](const auto &Module) { return Module.ModuleName; });
}

TranslationUnitDeps takeTranslationUnitDeps();
ModuleDepsGraph takeModuleGraphDeps();

private:
std::vector<std::string> Dependencies;
std::vector<PrebuiltModuleDep> PrebuiltModuleDeps;
llvm::MapVector<ModuleID, ModuleDeps> ClangModuleDeps;
std::string ModuleName;
std::vector<std::string> NamedModuleDeps;
std::vector<ModuleID> DirectModuleDeps;
std::vector<Command> Commands;
std::string ContextHash;
Expand Down
2 changes: 2 additions & 0 deletions clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@ TranslationUnitDeps FullDependencyConsumer::takeTranslationUnitDeps() {
TranslationUnitDeps TU;

TU.ID.ContextHash = std::move(ContextHash);
TU.ID.ModuleName = std::move(ModuleName);
TU.NamedModuleDeps = std::move(NamedModuleDeps);
TU.FileDeps = std::move(Dependencies);
TU.PrebuiltModuleDeps = std::move(PrebuiltModuleDeps);
TU.Commands = std::move(Commands);
Expand Down
5 changes: 2 additions & 3 deletions clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -714,9 +714,8 @@ void ModuleDepCollectorPP::EndOfMainFile() {

MDC.Consumer.handleDependencyOutputOpts(*MDC.Opts);

if (MDC.Service.getFormat() == ScanningOutputFormat::P1689)
MDC.Consumer.handleProvidedAndRequiredStdCXXModules(
MDC.ProvidedStdCXXModule, MDC.RequiredStdCXXModules);
MDC.Consumer.handleProvidedAndRequiredStdCXXModules(
MDC.ProvidedStdCXXModule, MDC.RequiredStdCXXModules);

for (auto &&I : MDC.ModularDeps)
MDC.Consumer.handleModuleDependency(*I.second);
Expand Down
Loading