[clangd] Attach more information about Sema completion to traces

[ilya-biryukov]

Technically this is NFC, but it has a huge toString helper and I'm not sure if I chose the appropriate place for logging the filename.

[sammccall]

This looks really useful! Main suggestion is to drop the added span and attach kind to the main span instead. (It's relevant to index too, not just to sema)

clangd/CodeComplete.cpp

429

hmm, this really looks like it belongs next to CodeCompletionContext::Kind?

453

I think this span is going to be tiny (hard to see in some UIs!) and generally not add a lot of value itself.
The nicest thing would be to just grab the kind out of the CCcontext and attach it to the span in CodeCompleteFlow::run()

My instinct is that the #items can be dropped entirely - we already log the number of items from Sema considered (before Limit) so all we'd be losing is the number hidden/ineligible/destructor completions we drop here, which seems uninteresting for tracing purposes.
(But exposing that from the recorder is another option)

454

sema_results_prefilter?
the filtering is the only difference between this and sema_results logged below

867

FWIW, I have a (delayed, sorry) patch to add a span to all actions run by TUscheduler, which would log the filename. If that lands, *maybe* we don't want it in both places (it'd be the parent span of this one)? But not sure.

[ilya-biryukov]

• Attach completion kind in CodeCompleteFlow::run().
• Move printCompletionKind closer to CompletionContext::Kind.
• Added FIXME to remove tracing of filename.

[ilya-biryukov]

clangd/CodeComplete.cpp

453

You're right, they shouldn't be too important, completion kind is quite useful information, though.
I've moved attaching of the completion kind to CodeCompleteFlow::run()

454

Removed this altogether.

867

LGTM to logging this in TUScheduler. I've added a FIXME to this patch.

[ilya-biryukov]

• Rename usage of printCompletionKind to getCompletionKindString
• Remove tracing of the filename, TUScheduler now provides those traces

clangd/CodeComplete.cpp

Info.UnresolvedQualifier =
                            SM, clang::LangOptions()).ltrim("::");
   // Sema excludes the trailing "::".
   if (!Info.UnresolvedQualifier->empty())
     *Info.UnresolvedQualifier += "::";
 
   return Info.scopesForIndexQuery();
 }
 
 // The CompletionRecorder captures Sema code-complete output, including context.
 // It filters out ignored results (but doesn't apply fuzzy-filtering yet).
 // It doesn't do scoring or conversion to CompletionItem yet, as we want to
 // merge with index results first.
 struct CompletionRecorder : public CodeCompleteConsumer {
   CompletionRecorder(const CodeCompleteOptions &Opts)
       : CodeCompleteConsumer(Opts.getClangCompleteOpts(),
                              /*OutputIsBinary=*/false),
         CCContext(CodeCompletionContext::CCC_Other), Opts(Opts),
         CCAllocator(std::make_shared<GlobalCodeCompletionAllocator>()),
         CCTUInfo(CCAllocator) {}
   std::vector<CodeCompletionResult> Results;
   CodeCompletionContext CCContext;
   Sema *CCSema = nullptr; // Sema that created the results.
   // FIXME: Sema is scary. Can we store ASTContext and Preprocessor, instead?
 
   void ProcessCodeCompleteResults(class Sema &S, CodeCompletionContext Context,
                                   CodeCompletionResult *InResults,
                                   unsigned NumResults) override final {
     // Record the completion context.
     assert(!CCSema && "ProcessCodeCompleteResults called multiple times!");
     CCSema = &S;
     CCContext = Context;
 
     // Retain the results we might want.
     for (unsigned I = 0; I < NumResults; ++I) {
       auto &Result = InResults[I];
       // Drop hidden items which cannot be found by lookup after completion.
       // Exception: some items can be named by using a qualifier.
       if (Result.Hidden && (!Result.Qualifier || Result.QualifierIsInformative))
         continue;
       if (!Opts.IncludeIneligibleResults &&
           (Result.Availability == CXAvailability_NotAvailable ||
            Result.Availability == CXAvailability_NotAccessible))
public:
   CompletionList run(const SemaCompleteInput &SemaCCInput) && {
     trace::Span Tracer("CodeCompleteFlow");
     // 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.
     CompletionList Output;
     semaCodeComplete(std::move(RecorderOwner), Opts.getClangCompleteOpts(),
                      SemaCCInput, [&] {
-                       if (Recorder.CCSema)
+                       if (Recorder.CCSema) {
                          Output = runWithSema();
-                       else
+                         SPAN_ATTACH(
+                             Tracer, "sema_completion_kind",
+                             getCompletionKindString(Recorder.CCContext.getKind()));
+                       } else
                          log("Code complete: no Sema callback, 0 results");
                      });
 
     SPAN_ATTACH(Tracer, "sema_results", NSema);
     SPAN_ATTACH(Tracer, "index_results", NIndex);
     SPAN_ATTACH(Tracer, "merged_results", NBoth);
     SPAN_ATTACH(Tracer, "returned_results", Output.items.size());
     SPAN_ATTACH(Tracer, "incomplete", Output.isIncomplete);
     log(llvm::formatv("Code complete: {0} results from Sema, {1} from Index, "
                       "{2} matched, {3} returned{4}.",
                       NSema, NIndex, NBoth, Output.items.size(),
                       Output.isIncomplete ? " (incomplete)" : ""));