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

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexshap 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; }
+
----------------
ioeric wrote:
> 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.
i see, thanks for the explanation, probably not particularly important
i thought that if a caller wanted to get a copy / take ownership 
it would just **accept** it by value, i.e. 
   auto path = change.getFilePath() /* assuming getFilePath returns const ref */ 
At the same time if for example we need to check the filepath (or just print it),
not copies would be required:
   llvm::errs() << change.getFilePath();


https://reviews.llvm.org/D27054