[PATCH] D26515: [clang-move] Abstract a ClassMather for matching class declarations.

[Eric Liu via cfe-commits]

ioeric added inline comments.


================
Comment at: clang-move/ClangMove.cpp:144
+  void run(const ast_matchers::MatchFinder::MatchResult &Result) override {
+    if (const auto *CMD =
+            Result.Nodes.getNodeAs<clang::CXXMethodDecl>("class_method")) {
----------------
It'd be more readable if you pull each case below into a helper function.


================
Comment at: clang-move/ClangMove.cpp:403
   //============================================================================
+  auto ClassMatchCallback = llvm::make_unique<ClassDeclarationMatcher>(this);
   // Match moved class declarations.
----------------
`push_back` and then use `back()` would save you a variable.


================
Comment at: clang-move/ClangMove.cpp:424
+  // Extend the MatchCallback lifetime.
+  MatchCallbacksHolder.push_back(std::move(ClassMatchCallback));
 
----------------
I'd first move `push_back` to the top, and then use `MatchCallbacksHolder.back()`.


================
Comment at: clang-move/ClangMove.h:104
+  std::vector<std::unique_ptr<ast_matchers::MatchFinder::MatchCallback>>
+      MatchCallbacksHolder;
   // The Key is file path, value is the replacements being applied to the file.
----------------
Simply `MatchCallbacks` would be a better name IMO.


https://reviews.llvm.org/D26515