[PATCH] D26853: Make llvm::Error generated from replacement interfaces more specific.

[Benjamin Kramer via cfe-commits]

bkramer added inline comments.


================
Comment at: include/clang/Tooling/Core/Replacement.h:157
+  /// \brief Constructs an error related to an existing replacement.
+  ReplacementError(replacement_error Err, const Replacement &Existing)
+      : Err(Err), ExistingReplacement(Existing) {}
----------------
Can you use pass by value and std::move here?


================
Comment at: include/clang/Tooling/Core/Replacement.h:162
+  /// replacement in a set of replacements.
+  ReplacementError(replacement_error Err, const Replacement &New,
+                   const Replacement &Existing)
----------------
Here too.


================
Comment at: include/clang/Tooling/Core/Replacement.h:174
+
+  llvm::Optional<Replacement> getNewReplacement() const {
+    return NewReplacement;
----------------
This on the other hand should return a const ref.


================
Comment at: include/clang/Tooling/Core/Replacement.h:178
+
+  llvm::Optional<Replacement> getExistingReplacement() const {
+    return ExistingReplacement;
----------------
This too.


================
Comment at: lib/Tooling/Core/Replacement.cpp:157
+    return "The new insertion has the same insert location as an existing "
+           "replacement's.";
+  }
----------------
Drop the 's here.


https://reviews.llvm.org/D26853