sammccall marked an inline comment as done.
sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/support/Trace.cpp:91
public:
- JSONSpan(JSONTracer *Tracer, llvm::StringRef Name, llvm::json::Object
*Args)
+ llvm::json::Object Args;
+ JSONSpan(JSONTracer *Tracer, llvm::StringRef Name)
----------------
kadircet wrote:
> move this near other fileds?
It's public unlike the other fields, but moved it to the bottom of the public
section
================
Comment at: clang-tools-extra/clangd/support/Trace.cpp:302
+ : llvm::StringRef(Name.str()),
+ [&](llvm::json::Object *A) { Args = A; });
+ return std::make_pair(std::move(Ctx), Args);
----------------
added assertions here (nonnull and empty) to guard against any API confusion
================
Comment at: clang-tools-extra/clangd/support/Trace.h:86
+ beginSpan(llvm::StringRef Name,
+ llvm::function_ref<void(llvm::json::Object *)> RequestArgs);
// Called when a Span is destroyed (it may still be active on other threads).
----------------
kadircet wrote:
> nit: at first i parsed request in `RequestArgs` as a verb (e.g. `i request
> arguments`), which was confusing a little bit. Hence the comment also didn't
> make much sense, I only managed to error correct after seeing the usage :D
>
> Maybe something like `SetRequestArgs` to emphasize that one provides request
> arguments using this lambda rather than receiving them?
This *is* actually the intention.
Changed to `AttachDetails` to avoid ambiguity, and rewrote the comment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89135/new/
https://reviews.llvm.org/D89135
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
