[PATCH] D40489: [clangd] Changed tracing interfaces

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
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.
----------------
sammccall wrote:
> just call this begin?
> Otherwise style is `beginEvent` I think
The new name is `beginSpan`. Hope that sounds good.


================
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;
----------------
sammccall wrote:
> 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.
Done. As discussed offline, the interface you propose seems much nicer.
Used names `beginSpan` and `instant`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40489