ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land.
LGTM. @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. https://reviews.llvm.org/D37291 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
