[PATCH] D27054: Introducing clang::tooling::AtomicChange for refactoring tools.

[Eric Liu via Phabricator via cfe-commits]

ioeric marked an inline comment as done.
ioeric added inline comments.


================
Comment at: include/clang/Tooling/Refactoring/AtomicChange.h:59
+  /// \brief Returns the path of the file containing this atomic change.
+  std::string getFilePath() const { return FilePath; }
+
----------------
alexshap wrote:
> i assume i might be missing smth - why here and above (in getKey, getFilePath, getError) std::string is returned by value  ?
Otherwise, users would need to worry about the lifetimes of the object. And these methods are not expected to be called intensively, so performance is not an issue.


================
Comment at: lib/Tooling/Refactoring/AtomicChange.cpp:35
+        RemovedHeaders(E.getRemovedHeaders()) {
+    std::copy(E.getReplacements().begin(), E.getReplacements().end(),
+              std::back_inserter(Replaces));
----------------
alexshap wrote:
> if i am not mistaken this can be done in the intialization list:
>     Replaces(E.getReplacements().begin(), E.getReplacements().end())
>  
You are right. 


https://reviews.llvm.org/D27054