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

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.


================
Comment at: clangd/JSONRPCDispatcher.h:25-32
+  virtual ~Handler() = default;
+
+  /// Called when the server receives a method call. This is supposed to return
+  /// a result on Outs.
+  virtual void handleMethod(llvm::yaml::MappingNode *Params, StringRef ID);
+  /// Called when the server receives a notification. No result should be
+  /// written to Outs.
----------------
Adi wrote:
> To avoid virtual dispatch one may rewrite this class in terms of CRTP. E.g.
> 
> ```
> template <typename T>
> class Handler {
> public:
>     Handler(llvm::raw_ostream &Outs, llvm::raw_ostream &Logs)
>       : Outs(Outs), Logs(Logs) {}
>     virtual ~Handler() = default;
> 
>     void handleMethod(const llvm::yaml::MappingMode *Params, StringRef ID) { 
>         static_cast<T*>(this)->handleMethod(Params, ID); 
>     }
>     void handleNotification(const llvm::yaml::MappingMode *Params) { 
>         static_cast<T*>(this)->handleNotification(Params); 
>     }
> 
> protected:
>     llvm::raw_ostream &Outs;
>     llvm::raw_ostream &Logs;
> 
>     void writeMessage(const Twine &Message);    
> };
> ```
> And then use it as:
> 
> ```
> struct MyConcreteHandler : public Handler<MyConcreteHandler> {
>     MyConcreteHandler(llvm::raw_ostream &Outs, llvm::raw_ostream &Logs /* other params if necessary */) 
>       : Handler(Outs, Logs) /* init other members */
>     {}
>  
>     void handleMethod(const llvm::yaml::MappingMode *Params, StringRef ID) { 
>         // impl
>     }
>     void handleNotification(const llvm::yaml::MappingMode *Params) { 
>         // impl 
>     }
> };
> ```
> To avoid virtual dispatch one may rewrite this class in terms of CRTP. E.g.

Why would virtual dispatch be a problem here? It seems strictly simpler this way.



https://reviews.llvm.org/D29451