sammccall added inline comments.
================
Comment at: clangd/Trace.cpp:138
return;
- if (!Args)
- Args = llvm::make_unique<json::obj>();
- T->event(Ctx, "E",
- Args ? json::obj{{"args", std::move(*Args)}} : json::obj{});
+ assert(Args && "Args can't be null at this point");
+ T->end_event(Ctx, Name, std::move(*Args));
----------------
why not?
================
Comment at: clangd/Trace.h:39
+ /// Called when event with \p Name starts.
+ virtual void begin_event(const ContextData &Ctx, llvm::StringRef Name) = 0;
+ /// Called when event with \p Name ends.
----------------
just call this begin?
Otherwise style is `beginEvent` I think
================
Comment at: clangd/Trace.h:41
+ /// Called when event with \p Name ends.
+ virtual void end_event(const ContextData &Ctx, llvm::StringRef Name,
+ json::obj &&Args) = 0;
----------------
How is identity represented between begin/end event? via Name doesn't seem
robust, so why the need to pass it twice? Does providing different contexts for
start/end make sense?
It might be cleaner/more flexible to have
std::function<void(json::obj&&)> begin()
and call the return value to signal end.
This seems likely to be pretty easy for different providers to implement, and
is easy to use from Span.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40489
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
