[PATCH] D36075: [refactor] Initial support for refactoring action rules

Alex Lorenz via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 24 04:57:31 PDT 2017


arphaman added a comment.

In https://reviews.llvm.org/D36075#851278, @ioeric wrote:

> Thanks for the changes! The code is much clearer.
>
> I am wondering if the current design could be extended to support tools (or rules) that use AST matchers? Or is the selection expected to be powerful enough to replace AST matchers?
>
> We have a few tools in `clang-tools-extra` (e.g. `clang-move` and `change-namespace`) and many internal tools that are based on AST matchers, but we would really like to move them into `clang-refactor` in the future :)


The selection requirement is supposed to be orthogonal to AST matchers, not a replacement. It should be used when working with source selection in editors.

I did mess around with moving over `clang-reorder-fields` using this kind of model and didn't see any issues when using AST matchers. Essentially I used the `requiredOption` requirements and mapped them to run my matching code instead of using the selection requirement. Thus this requirement was satisfied only when the AST matchers were successful. It might be possible to simplify that pattern even further to make it simpler.

In https://reviews.llvm.org/D36075#851278, @ioeric wrote:

> Thanks for the changes! The code is much clearer.
>
> I am wondering if the current design could be extended to support tools (or rules) that use AST matchers? Or is the selection expected to be powerful enough to replace AST matchers?
>
> We have a few tools in `clang-tools-extra` (e.g. `clang-move` and `change-namespace`) and many internal tools that are based on AST matchers, but we would really like to move them into `clang-refactor` in the future :)





Repository:
  rL LLVM

https://reviews.llvm.org/D36075





More information about the cfe-commits mailing list