[PATCH] D43272: [clangd] Fix tracing now that spans lifetimes can overlap on a thread.

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 14 05:28:53 PST 2018


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 :-)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43272





More information about the cfe-commits mailing list