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

Reply via email to