jansvoboda11 added inline comments.
================
Comment at:
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h:34
+/// \see FullDependencies::Commands.
+class Command {
+public:
----------------
Have you considered using the `Job`/`Command` classes the driver uses? What are
the downsides of doing that?
================
Comment at:
clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h:37
+ std::vector<std::string> Args) = 0;
+ virtual void handleInvocation(clang::CompilerInvocation CI) = 0;
+
----------------
Is `clang::` necessary here?
================
Comment at:
clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h:102
+std::vector<std::string>
+serializeCompilerInvocation(const CompilerInvocation &CI);
+
----------------
Maybe we could add this API directly to `CompilerInvocation`. I can see lots of
clients not caring about the (potentially more efficient)
`CompilerInvocation::generateCC1CommandLine()` with `StringAllocator` and
inventing their own implementation of this.
================
Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp:155
+void FullDependencyConsumer::handleInvocation(CompilerInvocation CI) {
+ clearImplicitModuleBuildOptions(CI);
+ Commands.push_back(std::make_unique<CC1Command>(std::move(CI)));
----------------
Shouldn't this be a responsibility of the dependency scanner?
================
Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp:197
+ for (const auto &ID : FD.ClangModuleDeps)
+ FrontendOpts.ModuleFiles.push_back(
+ LookupModuleOutput(ID, ModuleOutputKind::ModuleFile));
----------------
Just a heads-up: after I land D132066 today, you might need to update this to
respect the eager/lazy loading mode.
================
Comment at:
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:438
+
+void dependencies::clearImplicitModuleBuildOptions(CompilerInvocation &CI) {
+ CI.getLangOpts()->ImplicitModules = false;
----------------
Wondering if we could move this to `CompilerInvocation`, right besides
`resetNonModularOptions()`.
================
Comment at: clang/test/ClangScanDeps/diagnostics.c:31
// CHECK-NEXT: {
-// CHECK-NEXT: "clang-context-hash": "[[HASH_TU:.*]],
+// CHECK: "clang-context-hash": "[[HASH_TU:.*]],
// CHECK-NEXT: "clang-module-deps": [
----------------
benlangmuir wrote:
> I didn't want to radically change all the tests, so in most cases I just
> loosened the matching so it would match inside the "commands". The
> modules-full.c and multiple-commands.c cover the whole structure in detail.
That sounds good to me.
Repository:
rG LLVM Github Monorepo
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
