[PATCH] D29621: Add ASTMatchRefactorer and ReplaceNodeWithTemplate to RefactoringCallbacks

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.


================
Comment at: lib/Tooling/RefactoringCallbacks.cpp:42
+  void HandleTranslationUnit(ASTContext &Context) override {
+    for (const auto &Callback : Refactoring.Callbacks) {
+      Callback->getReplacements().clear();
----------------
Could you add a comment here explaining why the replacements in callbacks need to be cleared?


================
Comment at: lib/Tooling/RefactoringCallbacks.cpp:165
+        continue;
+      } else if (ToTemplate.substr(Index, 2) == "${") {
+        size_t EndOfIdentifier = ToTemplate.find("}", Index);
----------------
Just use `if`  since there is a `continue`?


================
Comment at: lib/Tooling/RefactoringCallbacks.cpp:170
+                       << ToTemplate.substr(Index) << "\n";
+          assert(false);
+        }
----------------
You might want to use `llvm_unreachable` instead so that it also bails out in non-assertion build.


================
Comment at: lib/Tooling/RefactoringCallbacks.cpp:211
+
+}  // end namespace tooling
+}  // end namespace clang
----------------
Looks like you are using clang-format with Google-style. Please reformat your changes with LLVM style.


https://reviews.llvm.org/D29621