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



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

Reply via email to