[PATCH] D40132: [clangd] Tracing improvements

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clangd/JSONRPCDispatcher.h:61
+                 llvm::Optional<json::Expr> ID)
+      : Out(Out), ID(std::move(ID)), Tracer(new trace::Span(Method)) {
+    if (this->ID)
----------------
NIT: maybe replace `new` with `llvm::make_unique`?


================
Comment at: clangd/JSONRPCDispatcher.h:78
   llvm::Optional<json::Expr> ID;
+  std::unique_ptr<trace::Span> Tracer;
 };
----------------
Why do we need `unique_ptr`? Are `Span`s non-movable?


================
Comment at: clangd/Trace.cpp:50
   // Contents must be a list of the other JSON key/values.
-  template <typename T> void event(StringRef Phase, const T &Contents) {
+  void event(StringRef Phase, json::obj &&Contents) {
     uint64_t TID = get_threadid();
----------------
Any reason why we use rval-ref instead of accepting by-value?


https://reviews.llvm.org/D40132