[PATCH] D89135: [clangd] Stop capturing trace args if the tracer doesn't need them.

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 12 02:31:00 PDT 2020


kadircet accepted this revision.
kadircet added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang-tools-extra/clangd/support/Trace.cpp:91
   public:
-    JSONSpan(JSONTracer *Tracer, llvm::StringRef Name, llvm::json::Object *Args)
+    llvm::json::Object Args;
+    JSONSpan(JSONTracer *Tracer, llvm::StringRef Name)
----------------
move this near other fileds?


================
Comment at: clang-tools-extra/clangd/support/Trace.h:86
+  beginSpan(llvm::StringRef Name,
+            llvm::function_ref<void(llvm::json::Object *)> RequestArgs);
   // Called when a Span is destroyed (it may still be active on other threads).
----------------
nit: at first i parsed request in `RequestArgs` as a verb (e.g. `i request arguments`), which was confusing a little bit. Hence the comment also didn't make much sense, I only managed to error correct after seeing the usage :D

Maybe something like `SetRequestArgs` to emphasize that one provides request arguments using this lambda rather than receiving them?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89135/new/

https://reviews.llvm.org/D89135



More information about the cfe-commits mailing list