sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land.
This feels like a bug in the underlying clang libraries, but since none of the lifetimes are documented and everyone just does it this way... ================ Comment at: clangd/CodeComplete.cpp:431 // It doesn't do scoring or conversion to CompletionItem yet, as we want to // merge with index results first. struct CompletionRecorder : public CodeCompleteConsumer { ---------------- maybe comment: 'generally the fields/methods should only be used from within the callback' ================ Comment at: clangd/CodeComplete.cpp:470 } + if (ResultsCallback) + ResultsCallback(); ---------------- I think this check is dead - remove? ================ Comment at: clangd/CodeComplete.cpp:660 // Invokes Sema code completion on a file. // Callback will be invoked once completion is done, but before cleaning up. bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer, ---------------- remove this comment ================ Comment at: clangd/CodeComplete.cpp:665 - llvm::function_ref<void()> Callback = nullptr) { + const SemaCompleteInput &Input) { auto Tracer = llvm::make_unique<trace::Span>("Sema completion"); std::vector<const char *> ArgStrs; ---------------- this can be stack-allocated, as we don't need to reset it ================ Comment at: clangd/CodeComplete.cpp:737 - Tracer = llvm::make_unique<trace::Span>("Sema completion cleanup"); Action.EndSourceFile(); ---------------- this tracer isn't needed anymore as discussed ================ Comment at: clangd/CodeComplete.cpp:816 // // So we start Sema completion first, but defer its cleanup until we're done. // We use the Sema context information to query the index. ---------------- this comment should probably be ", and do all our work in its callback" ================ Comment at: clangd/CodeComplete.cpp:852 // We run Sema code completion first. It builds an AST and calculates: // - completion results based on the AST. These are saved for merging. // - partial identifier and context. We need these for the index query. ---------------- remove "these are saved for merging" Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44000 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits