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