Skip to content

[Dependency Scanning] Teach DependencyScanningTool::getModuleDependencies to Process a List of Module Names #129915

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

[Dependency Scanning] Teach DependencyScanningTool::getModuleDependencies to Process a List of Module Names #129915

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7f29cb0
Changing DependencyScanningTool::getModuleDependencies to take a list…
qiongsiwu Mar 5, 2025
f816923
Renaming clang-scan-deps' module-name option to module-names.
qiongsiwu Mar 10, 2025
f98ed3a
Improve handling of failures when scanning for a list of modules by n…
qiongsiwu Mar 21, 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
6 changes: 3 additions & 3 deletions clang/include/clang/Frontend/FrontendActions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,12 +320,12 @@ class PrintPreprocessedAction : public PreprocessorFrontendAction {
};

class GetDependenciesByModuleNameAction : public PreprocessOnlyAction {
StringRef ModuleName;
ArrayRef<StringRef> ModuleNames;
void ExecuteAction() override;

public:
GetDependenciesByModuleNameAction(StringRef ModuleName)
: ModuleName(ModuleName) {}
GetDependenciesByModuleNameAction(ArrayRef<StringRef> ModuleNames)
: ModuleNames(ModuleNames) {}
};

} // end namespace clang
Expand Down
14 changes: 8 additions & 6 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 @@ -144,12 +144,14 @@ class DependencyScanningTool {
std::optional<llvm::MemoryBufferRef> TUBuffer = std::nullopt);

/// 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::DenseSet<ModuleID> &AlreadySeen,
LookupModuleOutputCallback LookupModuleOutput);
/// gather modular dependencies of modules specified by the the given list of
/// names, and return the information needed for explicit build.
llvm::Expected<ModuleDepsGraph>
getModuleDependencies(ArrayRef<StringRef> ModuleNames,
const std::vector<std::string> &CommandLine,
StringRef CWD,
const llvm::DenseSet<ModuleID> &AlreadySeen,
LookupModuleOutputCallback LookupModuleOutput);

llvm::vfs::FileSystem &getWorkerVFS() const { return Worker.getVFS(); }

Expand Down
6 changes: 3 additions & 3 deletions clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ class DependencyScanningWorker {
DependencyConsumer &DepConsumer,
DependencyActionController &Controller,
DiagnosticConsumer &DiagConsumer,
StringRef ModuleName);
ArrayRef<StringRef> ModuleNames);

/// Run the dependency scanning tool for a given clang driver command-line
/// for a specific translation unit via file system or memory buffer.
Expand All @@ -132,7 +132,7 @@ class DependencyScanningWorker {
const std::vector<std::string> &CommandLine,
DependencyConsumer &Consumer,
DependencyActionController &Controller,
StringRef ModuleName);
ArrayRef<StringRef> ModuleNames);

llvm::vfs::FileSystem &getVFS() const { return *BaseFS; }

Expand All @@ -156,7 +156,7 @@ class DependencyScanningWorker {
DependencyActionController &Controller,
DiagnosticConsumer &DC,
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS,
std::optional<StringRef> ModuleName);
std::optional<ArrayRef<StringRef>> ModuleNames);
};

} // end namespace dependencies
Expand Down
20 changes: 13 additions & 7 deletions clang/lib/Frontend/FrontendActions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1213,11 +1213,17 @@ void GetDependenciesByModuleNameAction::ExecuteAction() {
Preprocessor &PP = CI.getPreprocessor();
SourceManager &SM = PP.getSourceManager();
FileID MainFileID = SM.getMainFileID();
SourceLocation FileStart = SM.getLocForStartOfFile(MainFileID);
SmallVector<std::pair<IdentifierInfo *, SourceLocation>, 2> Path;
IdentifierInfo *ModuleID = PP.getIdentifierInfo(ModuleName);
Path.push_back(std::make_pair(ModuleID, FileStart));
auto ModResult = CI.loadModule(FileStart, Path, Module::Hidden, false);
PPCallbacks *CB = PP.getPPCallbacks();
CB->moduleImport(SourceLocation(), Path, ModResult);
SourceLocation SLoc = SM.getLocForStartOfFile(MainFileID);
for (auto ModuleName : ModuleNames) {
SmallVector<std::pair<IdentifierInfo *, SourceLocation>, 2> Path;
IdentifierInfo *ModuleID = PP.getIdentifierInfo(ModuleName);
Path.push_back(std::make_pair(ModuleID, SLoc));
auto ModResult = CI.loadModule(SLoc, Path, Module::Hidden, false);
Copy link
Contributor

@Bigcheese Bigcheese Mar 11, 2025

Choose a reason for hiding this comment

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

I think the type here is just ModuleLoadResult. The general rule for auto in LLVM is "Use auto if and only if it makes the code more readable or easier to maintain."

PPCallbacks *CB = PP.getPPCallbacks();
CB->moduleImport(SourceLocation(), Path, ModResult);
// FIXME: how do you know that this offset is correct?
SLoc = SLoc.getLocWithOffset(1);
Copy link
Contributor Author

@qiongsiwu qiongsiwu Mar 5, 2025

Choose a reason for hiding this comment

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

I am not quite sure setting an offset of 1 is correct. Will need to take a closer look.

Copy link
Member

@cyndyishida cyndyishida Mar 5, 2025

Choose a reason for hiding this comment

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

I used it as an incrementer as we need to allocate one source location for each module to import.
IIRC I found this pattern used in different parts of the codebase.

assert(SLoc <= SM.getLocForEndOfFile(MainFileID) &&
"Import location extends past file");
}
}
9 changes: 6 additions & 3 deletions clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,13 +155,16 @@ DependencyScanningTool::getTranslationUnitDependencies(
}

llvm::Expected<ModuleDepsGraph> DependencyScanningTool::getModuleDependencies(
StringRef ModuleName, const std::vector<std::string> &CommandLine,
StringRef CWD, const llvm::DenseSet<ModuleID> &AlreadySeen,
ArrayRef<StringRef> ModuleNames,
const std::vector<std::string> &CommandLine, StringRef CWD,
const llvm::DenseSet<ModuleID> &AlreadySeen,
LookupModuleOutputCallback LookupModuleOutput) {
FullDependencyConsumer Consumer(AlreadySeen);
CallbackActionController Controller(LookupModuleOutput);

assert(ModuleNames.size() && "GettingModuleDependencies for an empty list!");
llvm::Error Result = Worker.computeDependencies(CWD, CommandLine, Consumer,
Controller, ModuleName);
Controller, ModuleNames);
if (Result)
return std::move(Result);
return Consumer.takeModuleGraphDeps();
Expand Down
35 changes: 21 additions & 14 deletions clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,10 +287,11 @@ class DependencyScanningAction : public tooling::ToolAction {
DependencyScanningService &Service, StringRef WorkingDirectory,
DependencyConsumer &Consumer, DependencyActionController &Controller,
llvm::IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> DepFS,
bool DisableFree, std::optional<StringRef> ModuleName = std::nullopt)
bool DisableFree,
std::optional<ArrayRef<StringRef>> ModuleNames = std::nullopt)
: Service(Service), WorkingDirectory(WorkingDirectory),
Consumer(Consumer), Controller(Controller), DepFS(std::move(DepFS)),
DisableFree(DisableFree), ModuleName(ModuleName) {}
DisableFree(DisableFree), ModuleNames(ModuleNames) {}

bool runInvocation(std::shared_ptr<CompilerInvocation> Invocation,
FileManager *DriverFileMgr,
Expand Down Expand Up @@ -431,8 +432,9 @@ class DependencyScanningAction : public tooling::ToolAction {

if (Service.getFormat() == ScanningOutputFormat::P1689)
Action = std::make_unique<PreprocessOnlyAction>();
else if (ModuleName)
Action = std::make_unique<GetDependenciesByModuleNameAction>(*ModuleName);
else if (ModuleNames)
Action =
std::make_unique<GetDependenciesByModuleNameAction>(*ModuleNames);
else
Action = std::make_unique<ReadPCHAndPreprocessAction>();

Expand Down Expand Up @@ -478,7 +480,7 @@ class DependencyScanningAction : public tooling::ToolAction {
DependencyActionController &Controller;
llvm::IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> DepFS;
bool DisableFree;
std::optional<StringRef> ModuleName;
std::optional<ArrayRef<StringRef>> ModuleNames;
std::optional<CompilerInstance> ScanInstanceStorage;
std::shared_ptr<ModuleDepCollector> MDC;
std::vector<std::string> LastCC1Arguments;
Expand Down Expand Up @@ -546,7 +548,7 @@ llvm::Error DependencyScanningWorker::computeDependencies(
llvm::Error DependencyScanningWorker::computeDependencies(
StringRef WorkingDirectory, const std::vector<std::string> &CommandLine,
DependencyConsumer &Consumer, DependencyActionController &Controller,
StringRef ModuleName) {
ArrayRef<StringRef> ModuleNames) {
// Capture the emitted diagnostics and report them to the client
// in the case of a failure.
std::string DiagnosticOutput;
Expand All @@ -555,7 +557,7 @@ llvm::Error DependencyScanningWorker::computeDependencies(
TextDiagnosticPrinter DiagPrinter(DiagnosticsOS, DiagOpts.release());

if (computeDependencies(WorkingDirectory, CommandLine, Consumer, Controller,
DiagPrinter, ModuleName))
DiagPrinter, ModuleNames))
return llvm::Error::success();
return llvm::make_error<llvm::StringError>(DiagnosticsOS.str(),
llvm::inconvertibleErrorCode());
Expand Down Expand Up @@ -625,7 +627,7 @@ bool DependencyScanningWorker::scanDependencies(
StringRef WorkingDirectory, const std::vector<std::string> &CommandLine,
DependencyConsumer &Consumer, DependencyActionController &Controller,
DiagnosticConsumer &DC, llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS,
std::optional<StringRef> ModuleName) {
std::optional<ArrayRef<StringRef>> ModuleNames) {
auto FileMgr =
llvm::makeIntrusiveRefCnt<FileManager>(FileSystemOptions{}, FS);

Expand All @@ -648,7 +650,7 @@ bool DependencyScanningWorker::scanDependencies(
// always true for a driver invocation.
bool DisableFree = true;
DependencyScanningAction Action(Service, WorkingDirectory, Consumer,
Controller, DepFS, DisableFree, ModuleName);
Controller, DepFS, DisableFree, ModuleNames);

bool Success = false;
if (CommandLine[1] == "-cc1") {
Expand Down Expand Up @@ -729,13 +731,14 @@ bool DependencyScanningWorker::computeDependencies(
auto &FinalFS = ModifiedFS ? ModifiedFS : BaseFS;

return scanDependencies(WorkingDirectory, FinalCommandLine, Consumer,
Controller, DC, FinalFS, /*ModuleName=*/std::nullopt);
Controller, DC, FinalFS,
/*ModuleNames=*/std::nullopt);
}

bool DependencyScanningWorker::computeDependencies(
StringRef WorkingDirectory, const std::vector<std::string> &CommandLine,
DependencyConsumer &Consumer, DependencyActionController &Controller,
DiagnosticConsumer &DC, StringRef ModuleName) {
DiagnosticConsumer &DC, ArrayRef<StringRef> ModuleNames) {
// Reset what might have been modified in the previous worker invocation.
BaseFS->setCurrentWorkingDirectory(WorkingDirectory);

Expand All @@ -748,17 +751,21 @@ bool DependencyScanningWorker::computeDependencies(
InMemoryFS->setCurrentWorkingDirectory(WorkingDirectory);
SmallString<128> FakeInputPath;
// TODO: We should retry the creation if the path already exists.
llvm::sys::fs::createUniquePath(ModuleName + "-%%%%%%%%.input", FakeInputPath,
// FIXME: should we create files for multiple modules? I think so?
llvm::sys::fs::createUniquePath(ModuleNames[0] + "-%%%%%%%%.input",
Copy link
Contributor Author

@qiongsiwu qiongsiwu Mar 5, 2025

Choose a reason for hiding this comment

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

What is the reason that we still need to create this file in memory? Should I create a "fake" file for each module in the list?

Copy link
Member

@cyndyishida cyndyishida Mar 5, 2025

Choose a reason for hiding this comment

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

Someone should fact-check me, but I'm pretty sure compiler invocations always need a file to run an action over.

Copy link
Contributor

@Bigcheese Bigcheese Mar 11, 2025

Choose a reason for hiding this comment

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

Yeah, you always need a main file id. I'm not actually sure why we need to create a unique path on disk though. I would think the in memory file is enough.

Copy link
Member

@cyndyishida cyndyishida Mar 12, 2025

Choose a reason for hiding this comment

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

IIUC this shouldn't actually be writing any file to disk, just a unique name to create the in memory file.

FakeInputPath,
/*MakeAbsolute=*/false);
InMemoryFS->addFile(FakeInputPath, 0, llvm::MemoryBuffer::getMemBuffer(""));
std::string FakeString(ModuleNames.size(), ' ');
InMemoryFS->addFile(FakeInputPath, 0,
llvm::MemoryBuffer::getMemBuffer(FakeString));
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> InMemoryOverlay = InMemoryFS;

OverlayFS->pushOverlay(InMemoryOverlay);
auto ModifiedCommandLine = CommandLine;
ModifiedCommandLine.emplace_back(FakeInputPath);

return scanDependencies(WorkingDirectory, ModifiedCommandLine, Consumer,
Controller, DC, OverlayFS, ModuleName);
Controller, DC, OverlayFS, ModuleNames);
}

DependencyActionController::~DependencyActionController() {}
4 changes: 2 additions & 2 deletions clang/tools/clang-scan-deps/ClangScanDeps.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1025,8 +1025,8 @@ int clang_scan_deps_main(int argc, char **argv, const llvm::ToolContext &) {
}
} else if (ModuleName) {
auto MaybeModuleDepsGraph = WorkerTool.getModuleDependencies(
*ModuleName, Input->CommandLine, CWD, AlreadySeenModules,
LookupOutput);
ArrayRef<StringRef>({*ModuleName}), Input->CommandLine, CWD,
AlreadySeenModules, LookupOutput);
if (handleModuleResult(*ModuleName, MaybeModuleDepsGraph, *FD,
LocalIndex, DependencyOS, Errs))
HadErrors = true;
Expand Down