[PATCH] D41729: Add a tool executor that runs actions on all TUs in the compilation database.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.


================
Comment at: include/clang/Tooling/AllTUsExecution.h:34
+  AllTUsToolExecutor(const CompilationDatabase &Compilations,
+                     unsigned ThreadCount,
+                     std::shared_ptr<PCHContainerOperations> PCHContainerOps =
----------------
hokein wrote:
> consider using a default parameter (= 0)? use the `llvm::hardware_concurrency` by default, and you don't have to pass the `0` in the unittest.
The constructors aren't usually used directly (outside of unit tests; interface design shouldn't be driven by tests), so I didn't use default parameters, since they are usually disliked...


================
Comment at: unittests/Tooling/ExecutionTest.cpp:264
+  std::vector<std::string> ExpectedSymbols;
+  for (unsigned i = 1; i <= NumFiles; i++) {
+    std::string File = "f" + std::to_string(i) + ".cc";
----------------
hokein wrote:
> nit: ++i.
Done. (I don't think this matters for simple types tho...)


Repository:
  rC Clang

https://reviews.llvm.org/D41729