jansvoboda11 added inline comments.
================
Comment at:
clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h:27
+namespace driver {
+class Command;
+}
----------------
Not needed anymore, I assume.
================
Comment at:
clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h:39
+ virtual void handleBuildCommand(std::string Executable,
+ std::vector<std::string> Args) = 0;
----------------
Would it make sense to accept the `clang::tooling::dependencies::Command`
struct here?
================
Comment at:
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:165
+ if (Scanned) {
+ // If we have already scanned an upstream command, just forward to the
----------------
This makes sure we only run scan once per driver invocation? Can you expand on
this a bit? Maybe even put the reasoning into a comment in the code.
================
Comment at:
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:444
+ Invocation.setDiagnosticOptions(&Diags->getDiagnosticOptions());
+ return Invocation.run();
+ });
----------------
I'm not particularly fond of the fact that `Consumer.handleBuildCommand()` is
called in this lambda directly in the non-clang case, but several objects deep
in the normal case (`ToolInvocation` -> `DependencyScanningAction`). I think a
clearer way to do this would be to somehow extract the `CompilerInvocation` (or
`Command`) result from `ToolInvocation` and report it in this lambda too.
================
Comment at: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp:177
+static bool needsModules(FrontendInputFile FIF) {
+ switch (FIF.getKind().getLanguage()) {
----------------
I think this could be useful for other tools too in the future. Do you think it
would make sense to put this in a more prominent header, so that other people
can find it and reuse it more easily?
================
Comment at: clang/test/ClangScanDeps/deprecated-driver-api.c:1
+// RUN: rm -rf %t
+// RUN: split-file %s %t
----------------
Please summarize what this test does.
================
Comment at: clang/test/ClangScanDeps/multiple-commands.c:1
+// RUN: rm -rf %t
+// RUN: split-file %s %t
----------------
Please summarize what this test does.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132405/new/
https://reviews.llvm.org/D132405
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
