simark marked 10 inline comments as done.
simark added inline comments.
================
Comment at: clangd/DraftStore.cpp:75
+ return llvm::make_error<llvm::StringError>(
+ llvm::formatv("Range's end position (line={0}, character={1}) is "
+ "before start position (line={2}, character={3}).",
----------------
ilya-biryukov wrote:
> Could we add a format provide for `Position` to make this formatting code
> shorter?
> We'll need to define the corresponding specialization in `Protocol.h`:
>
> ```
> namespace llvm {
> template <>
> struct format_provider<clang::clangd::Position> {
> static void format(const clang::clangd::Position &Pos, raw_ostream &OS,
> StringRef Style) {
> assert(Style.empty() && "style modifiers for this type are not
> supported");
> OS << Pos;
> }
> };
> }
> ```
>
> This will allow to simplify this call site to:
> ```
> llvm::formatv("Range's end position ({0}) is before start position ({1})",
> End, Start)
> ```
Good idea, this is good for printing them in a consistent way too.
================
Comment at: clangd/DraftStore.cpp:100
+ EntryIt->second = Contents;
+ return std::move(Contents);
+}
----------------
ilya-biryukov wrote:
> Doing `return std::move(localvar)` is a pessimization. Use `return Contents;`
> instead.
> (for reference see
> [[https://stackoverflow.com/questions/14856344/when-should-stdmove-be-used-on-a-function-return-value|this
> SO post]], clang should give a warning for that case too)
Thanks, good catch.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44272
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
