[PATCH] D30777: Added `applyAtomicChanges` function.

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.


================
Comment at: include/clang/Tooling/Refactoring/AtomicChange.h:129
+struct ApplyChangesSpec {
+  // If true, cleans up redundant/erroneous around changed code with
+  // clang-format's cleanup functionality, e.g. redundant commas around deleted
----------------
That part of the sentence doesn't parse.


================
Comment at: include/clang/Tooling/Refactoring/AtomicChange.h:132
+  // parameter or empty namespaces introduced by deletions.
+  bool Cleanup;
+
----------------
I'd add default values for everything that will otherwise be uninitialized.


================
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.
----------------
Why?


================
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) {
----------------
Won't this be incorrect if the last line (no \n) is 1 over the column limit (due to EndPos = Code.size()-1 above)?


https://reviews.llvm.org/D30777