Skip to content

Commit 1da456b

Browse files
jansvoboda11jansvoboda11
committed
[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 (cherry picked from commit ba55666)
1 parent 92436eb commit 1da456b

File tree

5 files changed

+126
-66
lines changed

5 files changed

+126
-66
lines changed

‎clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,14 @@ namespace dependencies {
4040
using LookupModuleOutputCallback =
4141
llvm::function_ref<std::string(const ModuleID &, ModuleOutputKind)>;
4242

43+
/// Graph of modular dependencies.
44+
using ModuleDepsGraph = std::vector<ModuleDeps>;
45+
4346
/// The full dependencies and module graph for a specific input.
44-
struct FullDependencies {
47+
struct TranslationUnitDeps {
48+
/// The graph of direct and transitive modular dependencies.
49+
ModuleDepsGraph ModuleGraph;
50+
4551
/// The identifier of the C++20 module this translation unit exports.
4652
///
4753
/// If the translation unit is not a module then \c ID.ModuleName is empty.
@@ -77,11 +83,6 @@ struct FullDependencies {
7783
std::vector<std::string> DriverCommandLine;
7884
};
7985

80-
struct FullDependenciesResult {
81-
FullDependencies FullDeps;
82-
std::vector<ModuleDeps> DiscoveredModules;
83-
};
84-
8586
/// The high-level implementation of the dependency discovery tool that runs on
8687
/// an individual worker thread.
8788
class DependencyScanningTool {
@@ -93,14 +94,12 @@ class DependencyScanningTool {
9394

9495
/// Print out the dependency information into a string using the dependency
9596
/// file format that is specified in the options (-MD is the default) and
96-
/// return it. If \p ModuleName isn't empty, this function returns the
97-
/// dependency information of module \p ModuleName.
97+
/// return it.
9898
///
9999
/// \returns A \c StringError with the diagnostic output if clang errors
100100
/// occurred, dependency file contents otherwise.
101101
llvm::Expected<std::string>
102-
getDependencyFile(const std::vector<std::string> &CommandLine, StringRef CWD,
103-
llvm::Optional<StringRef> ModuleName = None);
102+
getDependencyFile(const std::vector<std::string> &CommandLine, StringRef CWD);
104103

105104
/// Collect dependency tree.
106105
llvm::Expected<llvm::cas::ObjectProxy>
@@ -133,9 +132,8 @@ class DependencyScanningTool {
133132
DiagnosticConsumer &DiagsConsumer, raw_ostream *VerboseOS,
134133
bool DiagGenerationAsCompilation);
135134

136-
/// Collect the full module dependency graph for the input, ignoring any
137-
/// modules which have already been seen. If \p ModuleName isn't empty, this
138-
/// function returns the full dependency information of module \p ModuleName.
135+
/// Given a Clang driver command-line for a translation unit, gather the
136+
/// modular dependencies and return the information needed for explicit build.
139137
///
140138
/// \param AlreadySeen This stores modules which have previously been
141139
/// reported. Use the same instance for all calls to this
@@ -147,13 +145,23 @@ class DependencyScanningTool {
147145
/// arguments for dependencies.
148146
///
149147
/// \returns a \c StringError with the diagnostic output if clang errors
150-
/// occurred, \c FullDependencies otherwise.
151-
llvm::Expected<FullDependenciesResult>
152-
getFullDependencies(const std::vector<std::string> &CommandLine,
153-
StringRef CWD, const llvm::StringSet<> &AlreadySeen,
154-
LookupModuleOutputCallback LookupModuleOutput,
155-
llvm::Optional<StringRef> ModuleName = None,
156-
DepscanPrefixMapping PrefixMapping = {});
148+
/// occurred, \c TranslationUnitDeps otherwise.
149+
llvm::Expected<TranslationUnitDeps>
150+
getTranslationUnitDependencies(const std::vector<std::string> &CommandLine,
151+
StringRef CWD,
152+
const llvm::StringSet<> &AlreadySeen,
153+
LookupModuleOutputCallback LookupModuleOutput,
154+
DepscanPrefixMapping PrefixMapping);
155+
156+
/// Given a compilation context specified via the Clang driver command-line,
157+
/// gather modular dependencies of module with the given name, and return the
158+
/// information needed for explicit build.
159+
llvm::Expected<ModuleDepsGraph>
160+
getModuleDependencies(StringRef ModuleName,
161+
const std::vector<std::string> &CommandLine,
162+
StringRef CWD, const llvm::StringSet<> &AlreadySeen,
163+
LookupModuleOutputCallback LookupModuleOutput,
164+
DepscanPrefixMapping PrefixMapping);
157165

158166
ScanningOutputFormat getScanningFormat() const {
159167
return Worker.getScanningFormat();
@@ -231,7 +239,8 @@ class FullDependencyConsumer : public DependencyConsumer {
231239
return LookupModuleOutput(ID, Kind);
232240
}
233241

234-
FullDependenciesResult takeFullDependencies();
242+
TranslationUnitDeps takeTranslationUnitDeps();
243+
ModuleDepsGraph takeModuleGraphDeps();
235244

236245
private:
237246
std::vector<std::string> Dependencies;

‎clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ struct DepscanPrefixMapping;
3535

3636
/// A command-line tool invocation that is part of building a TU.
3737
///
38-
/// \see FullDependencies::Commands.
38+
/// \see TranslationUnitDeps::Commands.
3939
struct Command {
4040
std::string Executable;
4141
std::vector<std::string> Arguments;

‎clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp

Lines changed: 44 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,7 @@ DependencyScanningTool::DependencyScanningTool(
2828
: Worker(Service, std::move(FS)) {}
2929

3030
llvm::Expected<std::string> DependencyScanningTool::getDependencyFile(
31-
const std::vector<std::string> &CommandLine, StringRef CWD,
32-
llvm::Optional<StringRef> ModuleName) {
31+
const std::vector<std::string> &CommandLine, StringRef CWD) {
3332
/// Prints out all of the gathered dependencies into a string.
3433
class MakeDependencyPrinterConsumer : public DependencyConsumer {
3534
public:
@@ -90,8 +89,7 @@ llvm::Expected<std::string> DependencyScanningTool::getDependencyFile(
9089
};
9190

9291
MakeDependencyPrinterConsumer Consumer;
93-
auto Result =
94-
Worker.computeDependencies(CWD, CommandLine, Consumer, ModuleName);
92+
auto Result = Worker.computeDependencies(CWD, CommandLine, Consumer);
9593
if (Result)
9694
return std::move(Result);
9795
std::string Output;
@@ -539,19 +537,32 @@ DependencyScanningTool::getIncludeTreeFromCompilerInvocation(
539537
return Consumer.getIncludeTree();
540538
}
541539

542-
llvm::Expected<FullDependenciesResult>
543-
DependencyScanningTool::getFullDependencies(
540+
llvm::Expected<TranslationUnitDeps>
541+
DependencyScanningTool::getTranslationUnitDependencies(
544542
const std::vector<std::string> &CommandLine, StringRef CWD,
545543
const llvm::StringSet<> &AlreadySeen,
546544
LookupModuleOutputCallback LookupModuleOutput,
547-
llvm::Optional<StringRef> ModuleName, DepscanPrefixMapping PrefixMapping) {
545+
DepscanPrefixMapping PrefixMapping) {
546+
FullDependencyConsumer Consumer(AlreadySeen, LookupModuleOutput,
547+
Worker.getCASFS(), std::move(PrefixMapping));
548+
llvm::Error Result = Worker.computeDependencies(CWD, CommandLine, Consumer);
549+
if (Result)
550+
return std::move(Result);
551+
return Consumer.takeTranslationUnitDeps();
552+
}
553+
554+
llvm::Expected<ModuleDepsGraph> DependencyScanningTool::getModuleDependencies(
555+
StringRef ModuleName, const std::vector<std::string> &CommandLine,
556+
StringRef CWD, const llvm::StringSet<> &AlreadySeen,
557+
LookupModuleOutputCallback LookupModuleOutput,
558+
DepscanPrefixMapping PrefixMapping) {
548559
FullDependencyConsumer Consumer(AlreadySeen, LookupModuleOutput,
549560
Worker.getCASFS(), std::move(PrefixMapping));
550561
llvm::Error Result =
551562
Worker.computeDependencies(CWD, CommandLine, Consumer, ModuleName);
552563
if (Result)
553564
return std::move(Result);
554-
return Consumer.takeFullDependencies();
565+
return Consumer.takeModuleGraphDeps();
555566
}
556567

557568
Error FullDependencyConsumer::initialize(CompilerInstance &ScanInstance,
@@ -695,26 +706,40 @@ Error FullDependencyConsumer::finalizeModuleInvocation(CompilerInvocation &CI,
695706
return llvm::Error::success();
696707
}
697708

698-
FullDependenciesResult FullDependencyConsumer::takeFullDependencies() {
699-
FullDependenciesResult FDR;
700-
FullDependencies &FD = FDR.FullDeps;
709+
TranslationUnitDeps FullDependencyConsumer::takeTranslationUnitDeps() {
710+
TranslationUnitDeps TU;
701711

702-
FD.ID.ContextHash = std::move(ContextHash);
703-
FD.FileDeps = std::move(Dependencies);
704-
FD.PrebuiltModuleDeps = std::move(PrebuiltModuleDeps);
705-
FD.Commands = std::move(Commands);
706-
FD.CASFileSystemRootID = CASFileSystemRootID;
712+
TU.ID.ContextHash = std::move(ContextHash);
713+
TU.FileDeps = std::move(Dependencies);
714+
TU.PrebuiltModuleDeps = std::move(PrebuiltModuleDeps);
715+
TU.Commands = std::move(Commands);
716+
TU.CASFileSystemRootID = CASFileSystemRootID;
707717

708718
for (auto &&M : ClangModuleDeps) {
709719
auto &MD = M.second;
710720
if (MD.ImportedByMainFile)
711-
FD.ClangModuleDeps.push_back(MD.ID);
721+
TU.ClangModuleDeps.push_back(MD.ID);
722+
// TODO: Avoid handleModuleDependency even being called for modules
723+
// we've already seen.
724+
if (AlreadySeen.count(M.first))
725+
continue;
726+
TU.ModuleGraph.push_back(std::move(MD));
727+
}
728+
729+
return TU;
730+
}
731+
732+
ModuleDepsGraph FullDependencyConsumer::takeModuleGraphDeps() {
733+
ModuleDepsGraph ModuleGraph;
734+
735+
for (auto &&M : ClangModuleDeps) {
736+
auto &MD = M.second;
712737
// TODO: Avoid handleModuleDependency even being called for modules
713738
// we've already seen.
714739
if (AlreadySeen.count(M.first))
715740
continue;
716-
FDR.DiscoveredModules.push_back(std::move(MD));
741+
ModuleGraph.push_back(std::move(MD));
717742
}
718743

719-
return FDR;
744+
return ModuleGraph;
720745
}

‎clang/test/ClangScanDeps/modules-full-by-mod-name.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,3 +75,5 @@
7575
// CHECK-NEXT: "name": "header2"
7676
// CHECK-NEXT: }
7777
// CHECK-NEXT: ],
78+
// CHECK-NEXT: "translation-units": []
79+
// CHECK-NEXT: }

‎clang/tools/clang-scan-deps/ClangScanDeps.cpp

Lines changed: 49 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -523,30 +523,30 @@ static llvm::json::Array toJSONSorted(std::vector<ModuleID> V) {
523523
// Thread safe.
524524
class FullDeps {
525525
public:
526-
void mergeDeps(StringRef Input, FullDependenciesResult FDR,
526+
void mergeDeps(StringRef Input, TranslationUnitDeps TUDeps,
527527
size_t InputIndex) {
528-
FullDependencies &FD = FDR.FullDeps;
529-
530528
InputDeps ID;
531529
ID.FileName = std::string(Input);
532-
ID.ContextHash = std::move(FD.ID.ContextHash);
533-
ID.FileDeps = std::move(FD.FileDeps);
534-
ID.ModuleDeps = std::move(FD.ClangModuleDeps);
535-
ID.CASFileSystemRootID = FD.CASFileSystemRootID;
530+
ID.ContextHash = std::move(TUDeps.ID.ContextHash);
531+
ID.FileDeps = std::move(TUDeps.FileDeps);
532+
ID.ModuleDeps = std::move(TUDeps.ClangModuleDeps);
533+
ID.CASFileSystemRootID = TUDeps.CASFileSystemRootID;
534+
mergeDeps(std::move(TUDeps.ModuleGraph), InputIndex);
535+
ID.DriverCommandLine = std::move(TUDeps.DriverCommandLine);
536+
ID.Commands = std::move(TUDeps.Commands);
537+
Inputs.push_back(std::move(ID));
538+
}
536539

540+
void mergeDeps(ModuleDepsGraph Graph, size_t InputIndex) {
537541
std::unique_lock<std::mutex> ul(Lock);
538-
for (const ModuleDeps &MD : FDR.DiscoveredModules) {
542+
for (const ModuleDeps &MD : Graph) {
539543
auto I = Modules.find({MD.ID, 0});
540544
if (I != Modules.end()) {
541545
I->first.InputIndex = std::min(I->first.InputIndex, InputIndex);
542546
continue;
543547
}
544548
Modules.insert(I, {{MD.ID, InputIndex}, std::move(MD)});
545549
}
546-
547-
ID.DriverCommandLine = std::move(FD.DriverCommandLine);
548-
ID.Commands = std::move(FD.Commands);
549-
Inputs.push_back(std::move(ID));
550550
}
551551

552552
void printFullOutput(raw_ostream &OS) {
@@ -660,21 +660,38 @@ class FullDeps {
660660
std::vector<InputDeps> Inputs;
661661
};
662662

663-
static bool handleFullDependencyToolResult(
664-
const std::string &Input,
665-
llvm::Expected<FullDependenciesResult> &MaybeFullDeps, FullDeps &FD,
666-
size_t InputIndex, SharedStream &OS, SharedStream &Errs) {
667-
if (!MaybeFullDeps) {
663+
static bool handleTranslationUnitResult(
664+
StringRef Input, llvm::Expected<TranslationUnitDeps> &MaybeTUDeps,
665+
FullDeps &FD, size_t InputIndex, SharedStream &OS, SharedStream &Errs) {
666+
if (!MaybeTUDeps) {
668667
llvm::handleAllErrors(
669-
MaybeFullDeps.takeError(), [&Input, &Errs](llvm::StringError &Err) {
668+
MaybeTUDeps.takeError(), [&Input, &Errs](llvm::StringError &Err) {
670669
Errs.applyLocked([&](raw_ostream &OS) {
671670
OS << "Error while scanning dependencies for " << Input << ":\n";
672671
OS << Err.getMessage();
673672
});
674673
});
675674
return true;
676675
}
677-
FD.mergeDeps(Input, std::move(*MaybeFullDeps), InputIndex);
676+
FD.mergeDeps(Input, std::move(*MaybeTUDeps), InputIndex);
677+
return false;
678+
}
679+
680+
static bool handleModuleResult(
681+
StringRef ModuleName, llvm::Expected<ModuleDepsGraph> &MaybeModuleGraph,
682+
FullDeps &FD, size_t InputIndex, SharedStream &OS, SharedStream &Errs) {
683+
if (!MaybeModuleGraph) {
684+
llvm::handleAllErrors(MaybeModuleGraph.takeError(),
685+
[&ModuleName, &Errs](llvm::StringError &Err) {
686+
Errs.applyLocked([&](raw_ostream &OS) {
687+
OS << "Error while scanning dependencies for "
688+
<< ModuleName << ":\n";
689+
OS << Err.getMessage();
690+
});
691+
});
692+
return true;
693+
}
694+
FD.mergeDeps(std::move(*MaybeModuleGraph), InputIndex);
678695
return false;
679696
}
680697

@@ -923,8 +940,8 @@ int main(int argc, const char **argv) {
923940

924941
// Run the tool on it.
925942
if (Format == ScanningOutputFormat::Make) {
926-
auto MaybeFile = WorkerTools[I]->getDependencyFile(
927-
Input->CommandLine, CWD, MaybeModuleName);
943+
auto MaybeFile =
944+
WorkerTools[I]->getDependencyFile(Input->CommandLine, CWD);
928945
if (handleMakeDependencyToolResult(Filename, MaybeFile, DependencyOS,
929946
Errs))
930947
HadErrors = true;
@@ -940,12 +957,19 @@ int main(int argc, const char **argv) {
940957
std::unique_lock<std::mutex> LockGuard(Lock);
941958
TreeResults.emplace_back(LocalIndex, std::move(Filename),
942959
std::move(MaybeTree));
960+
} else if (MaybeModuleName) {
961+
auto MaybeFullDeps = WorkerTools[I]->getModuleDependencies(
962+
*MaybeModuleName, Input->CommandLine, CWD, AlreadySeenModules,
963+
LookupOutput, PrefixMapping);
964+
if (handleModuleResult(Filename, MaybeFullDeps, FD, LocalIndex,
965+
DependencyOS, Errs))
966+
HadErrors = true;
943967
} else {
944-
auto MaybeFullDeps = WorkerTools[I]->getFullDependencies(
968+
auto MaybeTUDeps = WorkerTools[I]->getTranslationUnitDependencies(
945969
Input->CommandLine, CWD, AlreadySeenModules, LookupOutput,
946-
MaybeModuleName, PrefixMapping);
947-
if (handleFullDependencyToolResult(Filename, MaybeFullDeps, FD,
948-
LocalIndex, DependencyOS, Errs))
970+
PrefixMapping);
971+
if (handleTranslationUnitResult(Filename, MaybeTUDeps, FD, LocalIndex,
972+
DependencyOS, Errs))
949973
HadErrors = true;
950974
}
951975
}

0 commit comments

Comments
 (0)