[PATCH] D29451: Add a prototype for clangd v0.1

[Manuel Klimek via Phabricator via cfe-commits]

klimek accepted this revision.
klimek added a comment.

LG. Couple of questions.



================
Comment at: clangd/ClangDMain.cpp:65
+    // Now read the JSON.
+    std::vector<char> JSON;
+    JSON.resize(Len);
----------------
Adi wrote:
> Avoid unnecessary JSON.resize(Len) & potential reallocation during JSON.push_back('\0') by allocating enough space in advance: std::vector<char> JSON(Len + 1);
Shouldn't that be unsigned char for raw bytes?


================
Comment at: clangd/JSONRPCDispatcher.h:29
+  /// a result on Outs.
+  virtual void handleMethod(llvm::yaml::MappingNode *Params, StringRef ID);
+  /// Called when the server receives a notification. No result should be
----------------
klimek wrote:
> Adi wrote:
> > const ptr/ref to Params?
> Can we make those Params pointers-to-const?
Here and below, document what the default implementations do.


================
Comment at: clangd/JSONRPCDispatcher.h:29-32
+  virtual void handleMethod(llvm::yaml::MappingNode *Params, StringRef ID);
+  /// Called when the server receives a notification. No result should be
+  /// written to Outs.
+  virtual void handleNotification(llvm::yaml::MappingNode *Params);
----------------
Adi wrote:
> const ptr/ref to Params?
Can we make those Params pointers-to-const?


https://reviews.llvm.org/D29451