[PATCH] D34272: [Tooling] A new framework for executing clang frontend actions.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.


================
Comment at: include/clang/Tooling/Execution.h:51-53
+  virtual std::vector<std::pair<std::string, std::string>> AllKVResults() = 0;
+  virtual void forEachResult(
+      llvm::function_ref<void(StringRef Key, StringRef Value)> Callback) = 0;
----------------
ioeric wrote:
> klimek wrote:
> > Why do we need to get the results via the interface? For example, in a map/reduce style setup getting the results is infeasible.
> This would enable tools to access results regardless of the underlying representation of the results.
> 
> In a map/reduce style execution, we could have an implementation that deals with fetching and reading remote files; information about remote files can be fed into the implementation from the executor that performs such execution. It might make sense to add an interface that returns the metadata about the underlying data though (e.g. URI for remote files). WDYT?
After offline discussion, I now understand the concern about having tool callbacks be able to access results.

I've changed the `ExecutionContext` to only expose a result reporting interface instead of returning a reference of `ToolResults` - callbacks are expected to only have access to `ExecutionContext`. The `ToolResults` would only be available from the executor itself.


https://reviews.llvm.org/D34272