[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

This revision was automatically updated to reflect the committed changes. Closed by commit rC361037: [LibTooling] Add support to Transformer for composing rules as an ordered… (authored by ymandel, committed by ). Changed prior to commit: https://reviews.llvm.org/D61335?vs=200042=200044#toc

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ymandel updated this revision to Diff 200042. ymandel marked an inline comment as done. ymandel added a comment. Synced to HEAD in preparation for committing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61335/new/ https://reviews.llvm.org/D61335

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ymandel updated this revision to Diff 200026. ymandel marked 2 inline comments as done. ymandel added a comment. updated comments; moved some decls. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61335/new/ https://reviews.llvm.org/D61335 Files:

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ymandel marked 2 inline comments as done. ymandel added inline comments. Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:278 +/// Builds the matcher needed for registration. +ast_matchers::internal::DynTypedMatcher buildMatcher(const RewriteRule ); +

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ilya-biryukov added inline comments. Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:278 +/// Builds the matcher needed for registration. +ast_matchers::internal::DynTypedMatcher buildMatcher(const RewriteRule ); + ymandel wrote: >

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ymandel marked 5 inline comments as done. ymandel added a comment. Thanks!! Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:278 +/// Builds the matcher needed for registration. +ast_matchers::internal::DynTypedMatcher buildMatcher(const RewriteRule ); +

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ymandel updated this revision to Diff 199869. ymandel added a comment. adjust some API comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61335/new/ https://reviews.llvm.org/D61335 Files:

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ilya-biryukov added a comment. LG, just a few NITs wrt to exposing implementation details in the header. Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:161 +// components are gathered as a `Case` and rules are defined as an ordered list +// of cases. //

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ymandel updated this revision to Diff 199476. ymandel marked 18 inline comments as done. ymandel added a comment. Herald added a subscriber: jfb. responded to comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61335/new/

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ymandel added inline comments. Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:196 +/// Joins multiple rules into a single rule that applies the first rule in +/// `Rules` whose pattern matches a given node. All of the rules must use the +/// same kind of

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ilya-biryukov added a comment. The implementation LG, thanks! Just a few NITs and comments about the public interface and the comments Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:196 +/// Joins multiple rules into a single rule that applies the first

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ymandel marked 9 inline comments as done and an inline comment as not done. ymandel added a comment. Thanks for the review. PTAL. Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:196 +/// Joins multiple rules into a single rule that applies the first rule in

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ymandel updated this revision to Diff 199465. ymandel edited the summary of this revision. ymandel added a comment. Response to comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61335/new/ https://reviews.llvm.org/D61335 Files:

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ymandel marked an inline comment as done. ymandel added a comment. Agreed on all the comments -- just one question: Comment at: clang/lib/Tooling/Refactoring/Transformer.cpp:250 + assert(!CommonKind.isNone() && "Cases must have compatible matchers."); + return

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ilya-biryukov added inline comments. Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:196 +/// Joins multiple rules into a single rule that applies the first rule in +/// `Rules` whose pattern matches a given node. All of the rules must use the +/// same kind of

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ymandel added a comment. In D61335#1495324 , @ilya-biryukov wrote: > Sorry for the delay. > I personally like the `RewriteRule::Action` best. Allows to use a rather > common term, while still avoiding any possible confusion. I tried going with

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ymandel updated this revision to Diff 198811. ymandel added a comment. Folded CompositeRewriteRule into RewriteRule and added examples to comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61335/new/ https://reviews.llvm.org/D61335 Files:

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ilya-biryukov added a comment. Sorry for the delay. I personally like the `RewriteRule::Action` best. Allows to use a rather common term, while still avoiding any possible confusion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61335/new/

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ymandel added a comment. Gentle ping... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61335/new/ https://reviews.llvm.org/D61335 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ymandel added a comment. In D61335#1489680 , @ilya-biryukov wrote: > > As for naming, agreed, but does that concern drop away once we have only a > > single RewriteRule definition? > > Sure, that won't be an issue. > > The use-cases make sense, thanks

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ilya-biryukov added a comment. > As for naming, agreed, but does that concern drop away once we have only a > single RewriteRule definition? Sure, that won't be an issue. The use-cases make sense, thanks for the examples. Could you add one or two to the code? Would probably make sense to

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ymandel added a comment. In D61335#1488212 , @ilya-biryukov wrote: > - Could you provide a few real-world motivating examples? Especially > interested in cases that were complicated before and are easy to do now? Sure, below are some examples based

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ilya-biryukov added a comment. - Could you provide a few real-world motivating examples? Especially interested in cases that were complicated before and are easy to do now? - Do we expect most the rules to be written using the new API or is this something that's gonna be used in `5-10%` of the

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ymandel created this revision. ymandel added a reviewer: ilya-biryukov. Herald added a project: clang. This revision adds a new kind of rewrite rule, `CompositeRewriteRule`, which composes multiple subrules into a new rule that allows ordered-choice among its subrules. With this feature, users