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

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.


================
Comment at: include/clang/Tooling/Execution.h:76
+
+  void appendArgumentsAdjuster(ArgumentsAdjuster Adjuster);
+
----------------
I think the argument adjuster adjustment shouldn't be part of this interface, as the argument adjusters cannot be changed in the phase in which we want the ExecutionContext to be used.

I'd just make the argument adjusters a parameter on the constructor here (or alternatively, do not couple them in here, and just hand them around as a separate entity).


================
Comment at: include/clang/Tooling/Execution.h:130-131
+  /// context.
+  llvm::Error execute(ArgumentsAdjuster Adjuster,
+                      std::unique_ptr<FrontendActionFactory> Action);
+
----------------
I'd put the ArgumentAdjust second (as the action is the primary thing being acted on).



https://reviews.llvm.org/D34272