[PATCH] D26853: Make llvm::Error generated from replacement interfaces more specific.
Benjamin Kramer via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 21 08:58:55 PST 2016
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
More information about the cfe-commits
mailing list