sammccall added inline comments.
================ Comment at: clang-tools-extra/clangd/CodeComplete.h:141 + /// Ideally this must be disabled when being used by ClangdLSPServer. + bool EnableInstrumentationMode = false; + std::function<void(const CodeCompleteResult&)>* RecordCCResults; ---------------- I'm wondering if we can simplify this interface a bit. I'm not sure why we need another callback rather than just returning the CodeCompleteResult in the usual way. Another option: - if we invoke a callback for each completion instead of for the result set as a whole, we don't need to work out where to stash anything. - the work done after `addCandidate` is pretty trivial, so invoking a callback there provides basically all the information about the result set. The Top-N truncation is probably something you'd rather *not* have for analysis. - code completion always ends with the callback being invoked, so cross-result analysis can be done at that point. So I think this could just be a single `std::function<void(const CodeCompletion&, const SymbolQualitySignals &, const SymbolRelevanceSignals &)>`. If it's non-null, addCandidate would call toCodeCompletion on the bundle and pass it to the callback at the end. ================ Comment at: clang-tools-extra/clangd/CodeComplete.h:218 + // Signals captured when instrumentation is enabled during code completion. + std::shared_ptr<SymbolQualitySignals> QualitySignals; + std::shared_ptr<SymbolRelevanceSignals> RelevanceSignals; ---------------- why shared rather than unique? 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
