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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
