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

[Ilya Biryukov via Phabricator via cfe-commits]

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.
 //
----------------
NIT: maybe clarify what "ordered" means? E.g. "The first rule that matches gets applied and the rest are ignored"


================
Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:278
+/// Builds the matcher needed for registration.
+ast_matchers::internal::DynTypedMatcher buildMatcher(const RewriteRule &Rule);
+
----------------
Can it be declared in `.cpp` file instead? Or is it used in `clang-tidy` integration? 


================
Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:282
+const RewriteRule::Case &
+findSelectedCase(const ast_matchers::MatchFinder::MatchResult &Result,
+                 const RewriteRule &Rule);
----------------
Same here, so far this looks like an implementation detail of `Transformer`, why not put it into `.cpp` file?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61335