[PATCH] D30777: Added `applyAtomicChanges` function.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.


================
Comment at: include/clang/Tooling/Refactoring/AtomicChange.h:147-149
+/// This completely ignores the file path in each change and replaces them with
+/// \p FilePath, i.e. callers are responsible for ensuring all changes are for
+/// the same file.
----------------
klimek wrote:
> Why?
Changes might have equivalent paths in different forms (e.g. relative or absolute), and checking canonical paths are dependent on file systems and the working directory, so I think this should be checked/handled in the user level.


================
Comment at: lib/Tooling/Refactoring/AtomicChange.cpp:106
+  llvm::SmallVector<llvm::StringRef, 8> Lines;
+  Code.substr(StartPos, EndPos - StartPos).split(Lines, '\n');
+  for (llvm::StringRef Line : Lines) {
----------------
klimek wrote:
> Won't this be incorrect if the last line (no \n) is 1 over the column limit (due to EndPos = Code.size()-1 above)?
Yeah, that is a bug. Should be `Code.size()` then.


https://reviews.llvm.org/D30777