[PATCH] D67720: [clangd] Add semantic selection to ClangdLSPServer.

[UTKARSH SAXENA via Phabricator via cfe-commits]

usaxena95 added inline comments.


================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1169
+        Result.emplace_back(render(std::move(*Ranges)));
+        return Reply(std::move(Result));
+      });
----------------
hokein wrote:
> does `Reply({render(std::move(*Ranges))});` work?
List initialization of vector of move only types is painful 😃 
Even this does not work: `Reply(std::vector<SelectionRange>{render(std::move(*Ranges))});` because I think it vector tries to copy construct it because of initializer_lists 


================
Comment at: clang-tools-extra/clangd/Protocol.h:1251
+  SelectionRange() = default;
+  SelectionRange(SelectionRange &&) = default;
+};
----------------
hokein wrote:
> Are these constructors needed?
We don't need them if we use just unique_ptr. They were needed before in Optional<unique_ptr>. Somehow it was not able to deduce this is a trivially-constructible + move only class.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67720/new/

https://reviews.llvm.org/D67720