[PATCH] D40489: [clangd] Changed tracing interfaces

[Pavel Sychev via Phabricator via cfe-commits]

luckygeck added inline comments.


================
Comment at: clangd/Trace.h:41
+  /// Context.
   static PtrKey<EventTracer> CtxKey;
 
----------------
This is a (1)static non-pod member in an (2) interface. Is it really a good idea? If we plan to have only one ctxkey, then maybe let's make it not bound to EventTracer?


================
Comment at: clangd/Trace.h:45
+
+  /// Called when event with \p Name stars.
+  virtual void begin_event(Context &Ctx, llvm::StringRef Name) = 0;
----------------
s/stars/starts.


================
Comment at: clangd/Trace.h:49
+  virtual void end_event(Context &Ctx, llvm::StringRef Name,
+                         json::obj &&Args) = 0;
+  /// Called for instant events.
----------------
Maybe it is better to calculate these Args only if the tracing is enabled? This might be done at least this two ways:
1) Have `bool is_tracing_enabled()` method that we can check before calculating args.
2) Pass a function that will produce json::obj when called.


https://reviews.llvm.org/D40489