ioeric added inline comments.

Comment at: clangd/ClangdLSPServer.cpp:194
+        if (!Replacements)
+          return replyError(ErrorCode::InternalError,
+                            llvm::toString(Replacements.takeError()));
nit: since we are not spelling out the return type here, it might be clearer if 
we do:

`return replyError(...)` makes me wonder what the return type is.

Comment at: clangd/ClangdServer.cpp:209
+  auto Action = [Contents, Pos, TaggedFS,
+                 PCHs](Path File, decltype(Callback) Callback,
+                       llvm::Expected<InputsAndPreamble> IP) {
nit: I'd probably use a different name than `Callback` for this parameter for 

Comment at: clangd/ClangdServer.cpp:441
-  using RetType = llvm::Expected<Tagged<std::vector<DocumentHighlight>>>;
-  auto Action = [=](llvm::Expected<InputsAndAST> InpAST) -> RetType {
+  auto Action = [=](decltype(Callback) Callback,
+                    llvm::Expected<InputsAndAST> InpAST) {
Consider spelling out the captured values, just in case new variables which are 
not expected to be captured are added in the future.

Comment at: unittests/clangd/SyncAPI.cpp:69
+  llvm::Expected<Tagged<SignatureHelp>> Result = Tagged<SignatureHelp>();
+  (void)(bool)Result; // Expected has to be checked.
+  Server.signatureHelp(File, Pos, capture(Result), OverridenContents);
I'd expect this to be checked by callers. Would `return std::move(Result);` 

  rCTE Clang Tools Extra

cfe-commits mailing list

Reply via email to