ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LG with a few NITs.

Comment at: clangd/Trace.cpp:133
+    std::atomic<double> EndTime; // Filled in by endSpan().
+    std::string Name;
+    const uint64_t TID;
Maybe make all fields except `EndTime` const?

Comment at: clangd/Trace.h:46
+  // The Context returned by beginSpan is active, but Args is not ready.
+  virtual void endSpan() {};
It feels awkward that `endSpan()` does not have any parameters explicitly 
connecting it to `beginSpan()`.
We could change `beginSpan()` to return a callback that would be called when 
`Span` ends instead, that would make the connection clear, albeit it's not very 
useful for `JSONTracer`.

Not sure we should change it, just thinking out loud :-)

  rCTE Clang Tools Extra

cfe-commits mailing list

Reply via email to