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

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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 &Rule);
+
----------------
ilya-biryukov wrote:
> Can it be declared in `.cpp` file instead? Or is it used in `clang-tidy` integration? 
`buildMatcher` and `findSelectedCase` will be used in the clang-tidy integration and in the apply-rule-to-single-node function that I'm planning.


================
Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:282
+const RewriteRule::Case &
+findSelectedCase(const ast_matchers::MatchFinder::MatchResult &Result,
+                 const RewriteRule &Rule);
----------------
ilya-biryukov wrote:
> Same here, so far this looks like an implementation detail of `Transformer`, why not put it into `.cpp` file?
see above. I think these also make sense as part of the RewriteRule abstraction. that is, you can think of these as methods of RewriteRule and they make sense even without thinking about transformer.  That said, if you think there's a way to make this clearer, i"m happy to adjust the comments/name, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61335