[PATCH] D42499: [clangd] Modify the Span API so that Spans propagate with contexts.

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 2 inline comments as done.
sammccall added inline comments.


================
Comment at: clangd/JSONRPCDispatcher.cpp:31
+  RequestArgs(json::obj *Args) : Args(Args) {}
+  std::mutex Mu;
+  json::obj *Args;
----------------
ilya-biryukov wrote:
> We could make the fields private and expose only the function to modify args under lock (similar to `FillRequestInfo`, i.e. the last two line of it).
> That would make the class safer to use. But that's work and the class itself is an implementation detail, so we could also leave it as is.
Yeah, good point. Encapsulated the whole mechanism in this class.


================
Comment at: clangd/Trace.cpp:138
+                  Args)
+            : Ctx.derive(kSpanArgs, nullptr)) {}
 
----------------
ilya-biryukov wrote:
> Would be nice for `Span`s to have zero cost when no tracer is active. We could probably achieve that be not storing the `kSpanArgs` when `T` is null. WDYT?
> That's really not a big deal, so I don't think we should block the patch on this.
Good catch! This was left over from an older revision where SPAN_ATTACH looked up the args in the context.
Now this is for ownership only, so I skip derive() if there's no tracer, and made the key anonymous.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42499