[PATCH] D37291: [refactor] Use a RefactoringResultConsumer instead of tagged refactoring rule classes

Eric Liu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 31 07:17:01 PDT 2017

ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.


@klimek Manuel, would you like to take a look? This addresses your comment https://reviews.llvm.org/D36075#854075

Comment at: include/clang/Tooling/Refactoring/RefactoringResultConsumer.h:30
+  /// the source selection has no overlap at all with any relevant AST nodes.
+  virtual void handleInitiationFailure() = 0;
Just wondering if it is possible to provide more information about the initiation failure to the handler?

Comment at: unittests/Tooling/RefactoringActionRulesTest.cpp:39
+  class Consumer final : public RefactoringResultConsumer {
+    void handleInitiationFailure() {
+      Result = Expected<Optional<AtomicChanges>>(None);
arphaman wrote:
> ioeric wrote:
> > Can we probably have default error handling in the base class so that we don't need to re-implement these for every derived consumer. I would expect the error handling for initiation and invocation to be similar in different consumers.
> I've merge the two error functions into one, but I'm reluctant to add default implementation for them because of the reasons that I've mentioned in my previous inline comment.
Ok. Makes sense.


More information about the cfe-commits mailing list