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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
