ymandel added a comment.

Thanks for the review!



================
Comment at: clang/include/clang/Tooling/Transformer/RewriteRule.h:388
+llvm::Expected<SmallVector<Edit, 1>>
+rewriteDescendants(const Decl &Node, RewriteRule Rule,
+                   const ast_matchers::MatchFinder::MatchResult &Result);
----------------
gribozavr2 wrote:
> I agree that these functions provide very useful functionality, however I 
> feel uneasy about putting a function that returns an `EditGenerator` and a 
> function that actually executes the refactoring into the same overload set. 
> The first is a part of a DSL, the second is a regular function. Do you think 
> this is a problem worth solving?
> 
> IDK what exactly to suggest though. A namespace for DSL functions like we 
> have in AST matchers?
That's a really good point. These new functions are definitely a part of an 
(implicit) lower-level library that improves manipulation of the AST directly.  
We don't have any appropriate library yet for this -- SourceCode is in this 
spirit but even simpler than RewriteRule, which in fact depends on it.

For now,  I'll put these in the `detail` which is (in some sense) the 
collection of low level functions for which we need to solve this same problem. 
WDYT? (I wasn't quite clear about the comparison with ast matchers, since both 
the matchers DSL and the `match` functions (on which this is loosely based) are 
in the same namespace).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87031/new/

https://reviews.llvm.org/D87031

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to