[PATCH] D110130: [clangd] Ensure lambda init-capture gets semantic token

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

thanks, LGTM!



================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:518
+      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.
----------------
i think the comment might be more explicit about doing this to ensure we are not attributing the highlighting to some closeby token.


================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:515
       return true;
     if (auto K = kindForType(AT->getDeducedType().getTypePtrOrNull(),
                              H.getResolver())) {
----------------
kadircet wrote:
> nit: while here do you mind turning this into an early exit as well? the nesting below seems a little distracting.
sorry I was also talking about also turning `if(auto K = ...)` to
```
auto K = ...
if (!K)
  return true;
```


================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:522
+      // is the same as the location of the declared name itself.
+      if (StartLoc != D->getLocation()) {
+        auto &Tok =
----------------
nridge wrote:
> kadircet wrote:
> > nridge wrote:
> > > 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.
> > why not just check if `D` is implicit?
> If you mean `D->isImplicit()`, that returns false for init-captures.
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).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110130/new/

https://reviews.llvm.org/D110130