[clangd] Ensure lambda init-capture gets semantic token

[nridge]

Summary

Prior to this patch, CollectExtraHighlightings would incorrectly produce
a token for the init-capture's type which overlapped the name and
resulted in both being dropped.

Fixes https://github.com/clangd/clangd/issues/868

[nridge]

clang-tools-extra/clangd/SemanticHighlighting.cpp

522

Note, I initially tried D->getTypeSpecStartLoc() != D->getTypeSpecEndLoc(), but it turns out they are equal both in the init-capture case and in the regular auto case, so that check cannot be used to discriminate between the two.

[kadircet]

Naming of the patch is a little bit confusing. We're actually dropping the semantic highlighting for the type of lambdacaptures, which was showing up in the declarator names since there was no explicit type spelled in the source code. This turns on highlighting for the capture variables because we're left with a single token now.

Can you reword the description to reflect that?

clang-tools-extra/clangd/SemanticHighlighting.cpp

515

nit: while here do you mind turning this into an early exit as well? the nesting below seems a little distracting.

522

why not just check if D is implicit?

[nridge]

address review comments

[nridge]

In D110130#3012748, @kadircet wrote:

Naming of the patch is a little bit confusing. We're actually dropping the semantic highlighting for the type of lambdacaptures, which was showing up in the declarator names since there was no explicit type spelled in the source code. This turns on highlighting for the capture variables because we're left with a single token now.

Can you reword the description to reflect that?

Updated patch description. (Also, I forgot to link to https://github.com/clangd/clangd/issues/868 which contains additional context, sorry!)

clang-tools-extra/clangd/SemanticHighlighting.cpp

522

If you mean D->isImplicit(), that returns false for init-captures.

[kadircet]

thanks, LGTM!

clang-tools-extra/clangd/SemanticHighlighting.cpp

515

sorry I was also talking about also turning if(auto K = ...) to

auto K = ...
if (!K)
  return true;

517

i think the comment might be more explicit about doing this to ensure we are not attributing the highlighting to some closeby token.

522

ah nvm, I was looking at the fielddecl implicitly introduced into the lambda, not the vardecl that was created with the capture (https://github.com/llvm/llvm-project/tree/main/clang/lib/Sema/SemaLambda.cpp#L1739).

[nridge]

address review comments

clang-tools-extra/clangd/SemanticHighlighting.cpp

bool VisitDecltypeTypeLoc(DecltypeTypeLoc L) {
     }
     return true;
   }
 
   bool VisitDeclaratorDecl(DeclaratorDecl *D) {
     auto *AT = D->getType()->getContainedAutoType();
     if (!AT)
       return true;
     if (auto K = kindForType(AT->getDeducedType().getTypePtrOrNull(),
                              H.getResolver())) {
-      auto &Tok = H.addToken(D->getTypeSpecStartLoc(), *K)
-                      .addModifier(HighlightingModifier::Deduced);
+      SourceLocation StartLoc = D->getTypeSpecStartLoc();
+      // The AutoType may not have a corresponding token, e.g. in the case of
+      // init-captures, so there's nothing to color.
+      // Detect this case by checking if the type specifier's location
+      // is the same as the location of the declared name itself.
+      if (StartLoc != D->getLocation()) {
+        auto &Tok =
+            H.addToken(StartLoc, *K).addModifier(HighlightingModifier::Deduced);
         const Type *Deduced = AT->getDeducedType().getTypePtrOrNull();
         if (auto Mod = scopeModifier(Deduced))
           Tok.addModifier(*Mod);
         if (isDefaultLibrary(Deduced))
           Tok.addModifier(HighlightingModifier::DefaultLibrary);
       }
+    }
     return true;
   }
 
   // We handle objective-C selectors specially, because one reference can
   // cover several non-contiguous tokens.
   void highlightObjCSelector(const ArrayRef<SourceLocation> &Locs, bool Decl,
                              bool Class, bool DefaultLibrary) {
     HighlightingKind Kind =

clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

)cpp",
         struct $Class_decl[[Derived]] : $Class[[Base]]<$TemplateParameter[[T]]> {
           using $Class[[Base]]<$TemplateParameter[[T]]>::$Field_dependentName[[member]];
 
           void $Method_decl[[method]]() {
             (void)$Field_dependentName[[member]];
           }
         };
       )cpp",
+      // init-captures
+      R"cpp(
+        void $Function_decl[[foo]]() {
+          int $LocalVariable_decl[[a]], $LocalVariable_decl[[b]];
+          [ $LocalVariable_decl[[c]] = $LocalVariable[[a]],
+            $LocalVariable_decl[[d]]($LocalVariable[[b]]) ]() {}();
+        }
+      )cpp",
   };
   for (const auto &TestCase : TestCases)
     // Mask off scope modifiers to keep the tests manageable.
     // They're tested separately.
     checkHighlightings(TestCase, {}, ~ScopeModifierMask);
 
   checkHighlightings(R"cpp(
     class $Class_decl[[A]] {
const char *TestCases[] = {
       )cpp",
       R"cpp(
         void $Function_globalScope[[foo]](int $Parameter_functionScope[[y]]) {
           int $LocalVariable_functionScope[[z]];
         }
       )cpp",
       R"cpp(
         // Lambdas are considered functions, not classes.
-        auto $Variable_fileScope[[x]] = [m(42)] { // FIXME: annotate capture
+        auto $Variable_fileScope[[x]] = [$LocalVariable_functionScope[[m]](42)] {
           return $LocalVariable_functionScope[[m]];
         };
       )cpp",
       R"cpp(
         // Classes in functions are classes.
         void $Function_globalScope[[foo]]() {
           class $Class_functionScope[[X]] {
             int $Field_classScope[[x]];