[PATCH] D75603: [clangd] Add instrumentation mode in clangd for metrics collection.

[Sam McCall via Phabricator via cfe-commits]

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


================
Comment at: clang-tools-extra/clangd/CodeComplete.h:136
+
+  /// Allows capturing various internal structures used by clangd during code
+  /// completion. Eg: Symbol quality and relevance signals of all the candidates
----------------
First say what the behavior/API is (called once for each result...), Then justify it :)


================
Comment at: clang-tools-extra/clangd/CodeComplete.h:139
+  /// in the output.
+  std::function<void(const CodeCompletion &, const SymbolQualitySignals &,
+                     const SymbolRelevanceSignals &)> *RecordCCResult = nullptr;
----------------
I'd suggest including the final score in the signature rather than recompute it, just so the contract is really clear and simple (results yielded in arbitrary order, will be ranked by -score). Please spell this out.


================
Comment at: clang-tools-extra/clangd/CodeComplete.h:140
+  std::function<void(const CodeCompletion &, const SymbolQualitySignals &,
+                     const SymbolRelevanceSignals &)> *RecordCCResult = nullptr;
 };
----------------
This doesn't need to be a pointer, std::function is copy/movable and supports nullptr as a sentinel value.


================
Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:1048
+  std::vector<CodeCompletion> RecordedCompletions;
+  std::function<void(const CodeCompletion &, const SymbolQualitySignals &,
+                     const SymbolRelevanceSignals &)>
----------------
Nit: typically `auto` here (the anonymous lambda type) and let it convert to function implicitly when needed.

No need for `-> type` in trivial cases


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75603