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

[Eric Liu via cfe-commits]

ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.

Lg with a few comments.

(There are two empty comments that can't be deleted due to a phabricator bugs...)



================
Comment at: clang-move/ClangMove.cpp:407
+  auto MovedClass =
+      cxxRecordDecl(
+          InOldFiles, *InMovedClassNames, isDefinition(),
----------------
.


================
Comment at: clang-move/ClangMove.cpp:455
 
 void ClangMoveTool::run(const ast_matchers::MatchFinder::MatchResult &Result) {
   if (const auto *D =
----------------
-


================
Comment at: clang-move/ClangMove.cpp:151
+  bool MatchClassMethod(const MatchFinder::MatchResult &Result) {
+    if (const auto *CMD =
+            Result.Nodes.getNodeAs<clang::CXXMethodDecl>("class_method")) {
----------------
Not sure why you don't do type check in `run(...)`. The following structure looks a bit weird.
```
if (...) {
  ...
  return true;
}
return false;
```


================
Comment at: clang-move/ClangMove.cpp:414
       hasParent(decl(anyOf(namespaceDecl(), translationUnitDecl()))));
   Finder->addMatcher(AllDeclsInHeader.bind("decls_in_header"), this);
   // Match forward declarations in old header.
----------------
Maybe put all `addMatcher`s that bind `this` before those that bind to `ClassDeclarationMatch`.


https://reviews.llvm.org/D26515