[PATCH] D29621: Add ASTMatchRefactorer and ReplaceNodeWithTemplate to RefactoringCallbacks

[Samuel Benzaquen via Phabricator via cfe-commits]

sbenza added inline comments.


================
Comment at: include/clang/Tooling/RefactoringCallbacks.h:61
+    MatchFinder.addMatcher(Matcher, Callback);
+    Callbacks.emplace_back(Callback);
+  }
----------------
jbangert wrote:
> sbenza wrote:
> > Why emplace_back instead of push_back?
> Changed to push_back.  Is there ever an advantage to using push_back over emplace_back (the latter falls back to a copy constructor). 
push_back and emplace_back are equivalent when the value you are passing is already a T.
In that case, push_back should be used from the principle of using the least powerful tool that can do the job you want.
emplace_back allows for calls to explicit constructors, which makes it more powerful in ways that might be surprising to the reader.
Eg:

    std::vector<std::vector<int>> v;
    ... many lines below ...
    v.push_back(1);  // fails
    v.emplace_back(1);  // works, but it didn't add a 1 to the vector.


https://reviews.llvm.org/D29621