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");
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"

  rCTE Clang Tools Extra

cfe-commits mailing list

Reply via email to