[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf80a0ea76072: [clang][deps] Split translation units into individual -cc1 or other commands (authored by benlangmuir). Repository: rG LLVM Github

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir marked 2 inline comments as done. benlangmuir added inline comments. Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:295 +if (MDC) + MDC->applyDiscoveredDependencies(CI); +LastCC1Arguments = CI.getCC1CommandLine();

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

jansvoboda11 accepted this revision. jansvoboda11 added a comment. This revision is now accepted and ready to land. LGTM. Up to you if you act on the last comment. Thanks for seeing this through! Comment at:

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir marked 6 inline comments as done. benlangmuir added inline comments. Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:165 +if (Scanned) { + // If we have already scanned an upstream command, just forward to the

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir updated this revision to Diff 455712. benlangmuir added a comment. - handleBuildCommand now takes a Command - Removed now-unused forward decl - Expanded comment about scan-once behaviour - Moved all calls to handleBuildCommand to a single place - Added comments to new tests CHANGES

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

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:

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir added inline comments. Comment at: clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h:204 + /// invocation, for example when there are multiple chained invocations. + void handleInvocation(CompilerInvocation CI); + jansvoboda11

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir updated this revision to Diff 455666. benlangmuir added a comment. - Rebased, picking up the changes that were previously split out. - Broke up applying changes to CI from passing to consumer. The MDC is no longer responsible for passing invocations to the consumer, and it is done at

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir added inline comments. Comment at: clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h:224 + /// a preprocessor. Storage owned by \c ModularDeps. + llvm::StringMap ModuleDepsByID; + /// Directly imported prebuilt deps.

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir added a comment. In D132405#3747232 , @jansvoboda11 wrote: > I'd like to see this split into multiple patches. I can see some formatting > changes, removal of `CompilerInvocation` from `ModuleDeps`, isolated changes > to `Tooling`, etc.

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

jansvoboda11 added a comment. I left a couple of smaller initial comments. I'd like to see this split into multiple patches. I can see some formatting changes, removal of `CompilerInvocation` from `ModuleDeps`, isolated changes to `Tooling`, etc. That'd make it much easier to review.

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir updated this revision to Diff 455338. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132405/new/ https://reviews.llvm.org/D132405 Files: clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir marked 11 inline comments as done. benlangmuir added inline comments. Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h:44 + + enum CommandKind { +CK_CC1, tschuett wrote: > Why is this not an enum class?

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir updated this revision to Diff 455315. benlangmuir added a comment. - Remove CompilerInvocation from Command and ModuleDeps. Only the arg strings are exposed now. - Simplify Command, which is now just a simple struct. - Move the logic for mutating the CompilerInvocation for the TU

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

jansvoboda11 added inline comments. Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h:34 +/// \see FullDependencies::Commands. +class Command { +public: benlangmuir wrote: > jansvoboda11 wrote: > > Have you considered using the

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir added inline comments. Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h:102 +std::vector +serializeCompilerInvocation(const CompilerInvocation ); + jansvoboda11 wrote: > Maybe we could add this API directly to

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir added inline comments. Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h:34 +/// \see FullDependencies::Commands. +class Command { +public: jansvoboda11 wrote: > Have you considered using the `Job`/`Command` classes

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

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?

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

tschuett added inline comments. Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h:44 + + enum CommandKind { +CK_CC1, Why is this not an enum class? Comment at:

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir added inline comments. Herald added a subscriber: ormris. Comment at: clang/test/ClangScanDeps/diagnostics.c:31 // CHECK-NEXT: { -// CHECK-NEXT: "clang-context-hash": "[[HASH_TU:.*]], +// CHECK:"clang-context-hash": "[[HASH_TU:.*]], //

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir created this revision. benlangmuir added reviewers: jansvoboda11, Bigcheese. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Instead of trying to "fix" the original driver invocation by