arphaman added inline comments.
================ Comment at: include/clang/Tooling/Refactoring/RefactoringResultConsumer.h:39 + /// Handles the source replacements that are produced by a refactoring action. + virtual void handle(AtomicChanges SourceReplacements) = 0; +}; ---------------- ioeric wrote: > arphaman wrote: > > ioeric wrote: > > > I think this interface is specific to some refactoring rules and should > > > be pushed down to derived classes. > > Are you talking about derived classes of `RefactoringResultConsumer`? So > > something like > > > > ``` > > class RefactoringResultConsumer { > > virtual void handleInvocationError(llvm::Error Err) = 0; > > }; > > > > class RefactoringResultSourceReplacementConsumer: RefactoringResultConsumer > > { > > virtual void handle(AtomicChanges SourceReplacements) = 0; > > }; > > ``` > > > > If yes, can you please clarify how the rule can call `handle` if it's in a > > subclass of `RefactoringResultConsumer`? > > > Sorry, I thought the `handle` interface was dispatched by template. > > Maybe we can have default implementation of rule-specific handlers, e.g. > generate errors, in the base class? Derived classes can implement handlers > that they care about and ignore others. Sure, I will add default implementations for `handle(...)`. I'm not sure about `handle...Error(Error)` though because `llvm::Error` forces some form of handling, and I don't want to use a generic stub that will silence the `llvm::Error` as it won't be that useful. The errors will be handled differently by different clients anyway, like clang-refactor might output the error to stderr, and clangd might either try to notify the user or silently swallow the error. Repository: rL LLVM https://reviews.llvm.org/D37291 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits