[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

608

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

603–604

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

608

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

608

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

[kadircet]

thanks, LGTM!

clang-tools-extra/clangd/SemanticHighlighting.cpp

603–604

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

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

604

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

608

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

if (auto K = kindForType(L.getTypePtr(), H.getResolver())) {
         Tok.addModifier(HighlightingModifier::DefaultLibrary);
     }
     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);
-      const Type *Deduced = AT->getDeducedType().getTypePtrOrNull();
-      if (auto Mod = scopeModifier(Deduced))
-        Tok.addModifier(*Mod);
-      if (isDefaultLibrary(Deduced))
-        Tok.addModifier(HighlightingModifier::DefaultLibrary);
-    }
+    auto K =
+        kindForType(AT->getDeducedType().getTypePtrOrNull(), H.getResolver());
+    if (!K)
+      return true;
+    SourceLocation StartLoc = D->getTypeSpecStartLoc();
+    // The AutoType may not have a corresponding token, e.g. in the case of
+    // init-captures. In this case, StartLoc overlaps with the location
+    // of the decl itself, and producing a token for the type here would result
+    // in both it and the token for the decl being dropped due to conflict.
+    if (StartLoc == D->getLocation())
+      return true;
+    auto &Tok =
+        H.addToken(StartLoc, *K).addModifier(HighlightingModifier::Deduced);
     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",
           static int $StaticField_decl_static[[staticField]];
         };
         template <typename $TemplateParameter_decl[[X]]>
         void $Function_decl[[foo]]($TemplateParameter[[X]]& $Parameter_decl[[x]]) {
           // We do not support dependent types, so this one should *not* get the modifier.
           $Function[[foo]]($Parameter[[x]]); 
         }
       )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]];