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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
