[PATCH] D38402: [clang-refactor] Apply source replacements

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 9 02:16:55 PDT 2017

hokein added inline comments.

Comment at: include/clang/Frontend/CommandLineSourceLoc.h:57
+  std::string FileName;
+  std::pair<unsigned, unsigned> Begin;
+  std::pair<unsigned, unsigned> End;
Add a comment documenting what the first element and second element of the pair represent for (<Line, Column>? If I understand correctly).

Comment at: include/clang/Frontend/CommandLineSourceLoc.h:60
+  /// Returns a parsed source range from a string or None if the string is
+  /// invalid.
Would be clearer to document the valid format of the `Str`. 

Comment at: test/Refactor/tool-apply-replacements.cpp:3
+// RUN: cp %s %t.cp.cpp
+// RUN: clang-refactor local-rename -selection=%t.cp.cpp:6:7 -new-name=test %t.cp.cpp --
+// RUN: grep -v CHECK %t.cp.cpp | FileCheck %t.cp.cpp
Maybe add one more case for testing `-selection=%t.cp.cpp:6:7-6:15`.

Comment at: tools/clang-refactor/ClangRefactor.cpp:309
   void handleError(llvm::Error Err) {
     llvm::errs() << llvm::toString(std::move(Err)) << "\n";
Add `override`.

Comment at: tools/clang-refactor/ClangRefactor.cpp:313
-  // FIXME: Consume atomic changes and apply them to files.
+  void handle(AtomicChanges Changes) {
+    SourceChanges.insert(SourceChanges.begin(), Changes.begin(), Changes.end());
The same, `override`.

Comment at: tools/clang-refactor/ClangRefactor.cpp:412
+      if (!BufferErr) {
+        llvm::errs() << "error: failed to open" << File << " for rewriting\n";
+        return true;
nit: missing a blank after `open`.



More information about the cfe-commits mailing list