[clangd] Implement semanticTokens modifiers

[sammccall]

Summary

• Infrastructure to support modifiers (protocol etc)
• standard modifiers:
  • declaration (but no definition, yet)
  • deprecated
  • readonly (based on a fairly fuzzy const checking)
  • static (for class members and locals, but *not* file-scope things!)
  • abstract (for C++ classes, and pure-virtual methods)
• nonstandard modifier:
  • deduced (on "auto" whose Kind is Class etc) Happy to drop this if it's controversial at all.
• While here, update sample tweak to use our internal names, in anticipation of theia TM scopes going away.

This addresses some of the goals of D77702, but leaves some things undone.
Mostly because I think these will want some discussion.

• no split between dependent type/name. (We may want to model this as a modifier, type+dependent vs ???+dependent)
• no split between primitive/typedef. (Is introducing a nonstandard kind is worth this distinction?)
• no nonstandard local attribute This probably makes sense, I'm wondering if we want others and how they fit together.

There's one minor regression in explicit template specialization declarations
due to a latent bug in findExplicitReferences, but fixing it after seems OK.

[sammccall]

Rebase, polish, add tests.

[sammccall]

Update phab with commit message

[sammccall]

Tidy up, explain static

[sammccall]

Add tests for 'abstract' and fix bugs

[nridge]

Thanks a lot for working on this! I agree this is well aligned with the goals of D77702 (and in fact goes even further and meets the goals of D66990 via the declaration modifier). The other modifiers are neat as well; I like the expressiveness of the kind + modifier model in general.

This addresses some of the goals of D77702, but leaves some things undone.
Mostly because I think these will want some discussion.

• no split between dependent type/name. (We may want to model this as a modifier, type+dependent vs ???+dependent)

+1

• no split between primitive/typedef. (Is introducing a nonstandard kind is worth this distinction?)

Here's my general take on this:

• There is too much variety between programming languages to expect that a standard list of highlighting kinds is going to satisfactorily cover all languages.
• If LSP aims to be the basis for tools that are competitive with purpose-built language-specific tools, it's reasonable to allow for clients willing to go the extra mile in terms of customization (e.g. use a language-specific theme which provides colors for language-specific token kinds).
• Therefore, I would say yes, we should take the liberty of adding nonstandard highlighting kinds and modifiers where it makes sense from a language POV.

That said... for typedef specifically, I wonder if it actually makes more sense as a modifier than a kind. That is, have the kind be the target type (falling back to Unknown/Dependent if we don't know), and have a modifier flag for "the type is referred to via an alias" (as opposed to "the type is referred to directly"). WDYT?

• no nonstandard local attribute This probably makes sense, I'm wondering if we want others and how they fit together.

Are you referring to local-scope variables (i.e. FunctionScope in D95701)? If so, I think the approach in that patch is promising.

clang-tools-extra/clangd/SemanticHighlighting.cpp

130

It seems a bit unintuitive that the path which non-pointer types (e.g. unqualified class or builtin types) take is to return false in this recursive call because getPointeeType() returns null... but I guess that works.

134

Do you think in the future it might make sense to have the readonly modifier reflect, at least for variables, whether the particular reference to the variable is a readonly reference (e.g. binding the variable to a const X& vs. an X&)?

182

Will anything take this path (rather than being covered by VD->isStaticDataMember() above)?

376

This is a change of behaviour from before, in the case where the ReferenceLoc has multiple targets.

Before, we would produce at most one highlighting token for the ReferenceLoc. In the case where different target decls had different highlighting kinds, we wouldn't produce any.

Now, it looks like we produce a separate token for every target whose kind matches the kind of the first target (and skip targets with a conflicting kind).

Is that the intention?

It seems a bit strange: if we allow multiple tokens, why couldn't they have different kinds?

clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp

11

Maybe add // for llvm::to_string at it's not obvious

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

264 ↗

(On Diff #320218)

Presumably, the highlighting kinds StaticField and StaticMethod are going to be collapsed into Field and Method in a future change (after the removal of TheiaSemanticHighlighting, I guess)?

718 ↗

(On Diff #320218)

Should we add a test case for _deprecated as well?

[sammccall]

add missing test

[sammccall]

In D77811#2533237, @nridge wrote:

Thanks a lot for working on this! I agree this is well aligned with the goals of D77702 (and in fact goes even further and meets the goals of D66990 via the declaration modifier).

Ooh, I hadn't seen D66990, thanks for the pointer and also giving the upstream feedback to LSP folks! I'm also really happy about the multi-dimensional nature of highlightings in the new protocol, looks like you pushed it in that directyon.

• no split between primitive/typedef. (Is introducing a nonstandard kind is worth this distinction?)

Here's my general take on this:

• There is too much variety between programming languages to expect that a standard list of highlighting kinds is going to satisfactorily cover all languages.
• If LSP aims to be the basis for tools that are competitive with purpose-built language-specific tools, it's reasonable to allow for clients willing to go the extra mile in terms of customization (e.g. use a language-specific theme which provides colors for language-specific token kinds).
• Therefore, I would say yes, we should take the liberty of adding nonstandard highlighting kinds and modifiers where it makes sense from a language POV.

Totally agree, I should have been more explicit.
Introducing nonstandard kinds is backwards-incompatible. If the client doesn't understand primitiveType, then the token kind is now completely unknown. This could be a regression from the current state (type).
But for primitive type specifically, the information we're losing is "auto denotes a type" which the client's non-semantic highlighting should manage anyway. So I agree we should do this.

That said... for typedef specifically, I wonder if it actually makes more sense as a modifier than a kind. That is, have the kind be the target type (falling back to Unknown/Dependent if we don't know), and have a modifier flag for "the type is referred to via an alias" (as opposed to "the type is referred to directly"). WDYT?

Agree. Do you think it should be the *same* modifier as deduced which I included here?
(In a similar vein, there's an argument for pointer, ref, rvalue-ref as modifiers)

• no nonstandard local attribute This probably makes sense, I'm wondering if we want others and how they fit together.

Are you referring to local-scope variables (i.e. FunctionScope in D95701)? If so, I think the approach in that patch is promising.

Yes, exactly.

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

264 ↗

(On Diff #320218)

Yeah, merging any kinds that are exported with the same name should be NFC at that point.

Hmm, though currently StaticField --> Variable, not Field (similarly StaticMethod --> Method).
So we can have static fields be Variable+Static+ClassScope or Field+Static+ClassScope.

I can see arguments for either...

718 ↗

(On Diff #320218)

Oops, done!

[nridge]

In D77811#2533815, @sammccall wrote:

Introducing nonstandard kinds is backwards-incompatible. If the client doesn't understand primitiveType, then the token kind is now completely unknown. This could be a regression from the current state (type).

Good point. I guess, if we're aiming for backwards compatibility, we'll want to do most of our future customization via modifiers, since those will gracefully degrade to the highlighting for base kind for clients that don't recongize them.

That said... for typedef specifically, I wonder if it actually makes more sense as a modifier than a kind. That is, have the kind be the target type (falling back to Unknown/Dependent if we don't know), and have a modifier flag for "the type is referred to via an alias" (as opposed to "the type is referred to directly"). WDYT?

Agree. Do you think it should be the *same* modifier as deduced which I included here?

Conceptually, they seem distinct to me.

(In a similar vein, there's an argument for pointer, ref, rvalue-ref as modifiers)

Indeed. I'm definitely happy to defer type modifiers like these (including typedef) to a future patch.

[nridge]

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

264 ↗

(On Diff #320218)

I don't have a strong opinion on this one.

Maybe Field+Static+ClassScope, that way clients that use a generic theme (and thus do not recognize ClassScope) will color it as Field rather than Variable?

[nridge]

clang-tools-extra/clangd/SemanticHighlighting.cpp

376

Thinking more about this, the behaviour may actually be reasonable as written.

• Tokens with different kinds would get discarded via resolveConflict().
• Multiple tokens with the same kind are potentially useful because they may have different modifiers, and the modifiers are then merged in resolveConflict().

[nridge]

I think this patch is in pretty good shape, the outstanding comments are either minor or thoughts about future changes.

I am curious what you think of the extension idea for readonly, though obviously it wouldn't be something to do in this patch.

[sammccall]

Thanks for the excellent comments, and sorry for the radio silence. (Kids got quarantined recently so work time has been... scarce).

I think there are improvements to be had here but it's probably time to land this...

In D77811#2544328, @nridge wrote:

I am curious what you think of the extension idea for readonly, though obviously it wouldn't be something to do in this patch.

Longer comment below, but:

• I love it
• it's not trivial to implement, I think
• not obvious whether this should replace the simple readonly given here, augment it, or be a different attribute - I'm probably happy with any of these.

clang-tools-extra/clangd/SemanticHighlighting.cpp

134

Do I understand correctly?

std::vector<int> X; // X is not readonly
X.push_back(42); // X is not readonly
X.size(); // X is readonly

Distinguishing potentially-mutating from non-mutating uses seems really useful to me.
My only question is whether mapping this concept onto readonly is the right thing to do:

• there are really three sets: const access, mutable access, and non-access (e.g. declaration, or on RHS of using a=b). And I think maybe the most useful thing to distinguish is mutable access vs everything else, which is awkward with an attribute called "readonly".
• this also seems likely to diverge from how other languages use the attribute (most don't have this concept)
• on the other hand, standard attribute names will be better supported by clients

This also might be somewhat complicated to implement :-)
I'd like to leave in this simple decl-based thing as a placeholder, and either we can replace it and add an additional "mutation" attribute later (once we work out how to implement it!) I've left a comment about this...

(Related: a while ago Google's style guide dropped its requirement to use pointers rather than references for mutable function params. Having &x at the callsite rather than just x is a useful hint, but obviously diverging from common practice has a cost. We discussed how we could use semantic highlighting to highlight where a param was being passed by mutable reference, though didn't have client-side support for it yet)

376

Oops, I don't think the part where we treat the first kind specially was intended.
I think the simplest thing is to emit all the tokens and let conflict resolution sort them out.
In practice this means:

• if there's multiple kinds, discard all
• if there's different sets of modifiers, take the union

Which seems reasonable to me until proven otherwise

(This part of the patch is really old so I can't be 100% sure what my intention was...)

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

264 ↗

(On Diff #320218)

Field+Static is probably better than Variable, yeah.

In the back of my head I wonder if there will be significant clients that handle type but not modifiers, and if Variable is a better first-order description than Field. (I was surprised to find out that static members in clang are VarDecls not FieldDecls, but now I got used to the idea...)

[nridge]

clang-tools-extra/clangd/SemanticHighlighting.cpp

134

Do I understand correctly?

std::vector<int> X; // X is not readonly
X.push_back(42); // X is not readonly
X.size(); // X is readonly

Yup, that's what I had in mind.

Distinguishing potentially-mutating from non-mutating uses seems really useful to me.
My only question is whether mapping this concept onto readonly is the right thing to do:

• there are really three sets: const access, mutable access, and non-access (e.g. declaration, or on RHS of using a=b). And I think maybe the most useful thing to distinguish is mutable access vs everything else, which is awkward with an attribute called "readonly".
• this also seems likely to diverge from how other languages use the attribute (most don't have this concept)
• on the other hand, standard attribute names will be better supported by clients

This also might be somewhat complicated to implement :-)
I'd like to leave in this simple decl-based thing as a placeholder, and either we can replace it and add an additional "mutation" attribute later (once we work out how to implement it!) I've left a comment about this...

Sounds good!

(Related: a while ago Google's style guide dropped its requirement to use pointers rather than references for mutable function params. Having &x at the callsite rather than just x is a useful hint, but obviously diverging from common practice has a cost. We discussed how we could use semantic highlighting to highlight where a param was being passed by mutable reference, though didn't have client-side support for it yet)

Funny you mention this, because I implemented this exact highlighting in Eclipse a few years ago:

https://wiki.eclipse.org/CDT/User/NewIn92#Syntax_coloring_for_variables_passed_by_non-const_reference

(and it's my main motivation for the suggestion I've made here).

[nridge]

clang-tools-extra/clangd/SemanticHighlighting.cpp

134

I implemented this exact highlighting in Eclipse a few years ago

I will readily admit that classifying references as mutable or not can get tricky. For example:

int* array = new int[10];
array[0] = 42;

Is the reference to array on the second line mutable? Does the answer change if the declaration is changed to int array[10];?

Fun questions to figure out later :-)

clang-tools-extra/clangd/ClangdLSPServer.cpp

 //===--- ClangdLSPServer.cpp - LSP server ------------------------*- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 

 static std::vector<llvm::StringRef> semanticTokenTypes() {
   std::vector<llvm::StringRef> Types;
   for (unsigned I = 0; I <= static_cast<unsigned>(HighlightingKind::LastKind);
        ++I)
     Types.push_back(toSemanticTokenType(static_cast<HighlightingKind>(I)));
   return Types;
 }
 
+static std::vector<llvm::StringRef> semanticTokenModifiers() {
+  std::vector<llvm::StringRef> Modifiers;
+  for (unsigned I = 0;
+       I <= static_cast<unsigned>(HighlightingModifier::LastModifier); ++I)
+    Modifiers.push_back(
+        toSemanticTokenModifier(static_cast<HighlightingModifier>(I)));
+  return Modifiers;
+}
+
 void ClangdLSPServer::onInitialize(const InitializeParams &Params,
                                    Callback<llvm::json::Value> Reply) {
   // Determine character encoding first as it affects constructed ClangdServer.
   if (Params.capabilities.offsetEncoding && !NegotiatedOffsetEncoding) {
     NegotiatedOffsetEncoding = OffsetEncoding::UTF16; // fallback
     for (OffsetEncoding Supported : *Params.capabilities.offsetEncoding)
       if (Supported != OffsetEncoding::UnsupportedEncoding) {
         NegotiatedOffsetEncoding = Supported;
llvm::json::Object Result{
                  // trigger on '->' and '::'.
                  {"triggerCharacters", {".", ">", ":"}},
              }},
             {"semanticTokensProvider",
              llvm::json::Object{
                  {"documentProvider", true},
                  {"rangeProvider", false},
                  {"legend",
-                  llvm::json::Object{{"tokenTypes", semanticTokenTypes()},
-                                     {"tokenModifiers", llvm::json::Array()}}},
+                  llvm::json::Object{
+                      {"tokenTypes", semanticTokenTypes()},
+                      {"tokenModifiers", semanticTokenModifiers()}}},
              }},
             {"signatureHelpProvider",
              llvm::json::Object{
                  {"triggerCharacters", {"(", ","}},
              }},
             {"declarationProvider", true},
             {"definitionProvider", true},
             {"documentHighlightProvider", true},

clang-tools-extra/clangd/SemanticHighlighting.h

 //==-- SemanticHighlighting.h - Generating highlights from the AST-- C++ -*-==//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
enum class HighlightingKind {
   Macro,
 
   // This one is different from the other kinds as it's a line style
   // rather than a token style.
   InactiveCode,
 
   LastKind = InactiveCode
 };
+
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, HighlightingKind K);
 
+enum class HighlightingModifier {
+  Declaration,
+  Definition,
+  Deprecated,
+  Deduced,
+  Readonly,
+  StaticMember,
+
+  LastModifier = Deprecated
+};
+static_assert(static_cast<unsigned>(HighlightingModifier::LastModifier) < 32,
+              "Increase width of modifiers bitfield!");
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, HighlightingModifier K);
+
 // Contains all information needed for the highlighting a token.
 struct HighlightingToken {
   HighlightingKind Kind;
+  uint32_t Modifiers;
   Range R;
+
+  HighlightingToken &addModifier(HighlightingModifier M) {
+    Modifiers |= 1 << static_cast<unsigned>(M);
+    return *this;
+  }
 };
 
 bool operator==(const HighlightingToken &L, const HighlightingToken &R);
 bool operator<(const HighlightingToken &L, const HighlightingToken &R);
 
 /// Contains all information about highlightings on a single line.
 struct LineHighlightings {
   int Line;
   std::vector<HighlightingToken> Tokens;
   bool IsInactive;
 };
 
 bool operator==(const LineHighlightings &L, const LineHighlightings &R);
 
 // Returns all HighlightingTokens from an AST. Only generates highlights for the
 // main AST.
 std::vector<HighlightingToken> getSemanticHighlightings(ParsedAST &AST);
 
 std::vector<SemanticToken> toSemanticTokens(llvm::ArrayRef<HighlightingToken>);
 llvm::StringRef toSemanticTokenType(HighlightingKind Kind);
+llvm::StringRef toSemanticTokenModifier(HighlightingModifier Modifier);
 
 /// Converts a HighlightingKind to a corresponding TextMate scope
 /// (https://manual.macromates.com/en/language_grammars).
 llvm::StringRef toTextMateScope(HighlightingKind Kind);
 
 /// Convert to LSP's semantic highlighting information.
 std::vector<TheiaSemanticHighlightingInformation>
 toTheiaSemanticHighlightingInformation(

clang-tools-extra/clangd/SemanticHighlighting.cpp

 //===--- SemanticHighlighting.cpp - ------------------------- ---*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "SemanticHighlighting.h"
 #include "FindTarget.h"
 #include "Logger.h"
 #include "ParsedAST.h"
 #include "Protocol.h"
 #include "SourceCode.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclObjC.h"
+#include "clang/AST/DeclTemplate.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
if (TP->isBuiltinType()) // Builtins are special, they do not have decls.
     return HighlightingKind::Primitive;
   if (auto *TD = dyn_cast<TemplateTypeParmType>(TP))
     return kindForDecl(TD->getDecl());
   if (auto *TD = TP->getAsTagDecl())
     return kindForDecl(TD);
   return llvm::None;
 }
 
-llvm::Optional<HighlightingKind> kindForReference(const ReferenceLoc &R) {
-  llvm::Optional<HighlightingKind> Result;
-  for (const NamedDecl *Decl : R.Targets) {
-    if (!canHighlightName(Decl->getDeclName()))
-      return llvm::None;
-    auto Kind = kindForDecl(Decl);
-    if (!Kind || (Result && Kind != Result))
-      return llvm::None;
-    Result = Kind;
-  }
-  return Result;
+// Whether T is const in a loose sense - is a variable with this type readonly?
+bool isConst(QualType T) {
+  if (T.isNull() || T->isDependentType())
+    return false;
+  T = T.getNonReferenceType();
+  if (T.isConstQualified())
+    return true;
+  if (const auto *AT = T->getAsArrayTypeUnsafe())
+    return isConst(AT->getElementType());
+  return isConst(T->getPointeeType());
+
+// Whether D is const in a loose sense (should it be highlighted as such?)
+bool isConst(const Decl* D) {
+  if (llvm::isa<EnumConstantDecl>(D) || llvm::isa<NonTypeTemplateParmDecl>(D))
+    return true;
+  if (llvm::isa<FieldDecl>(D) || llvm::isa<VarDecl>(D) ||
+      llvm::isa<MSPropertyDecl>(D) || llvm::isa<BindingDecl>(D)) {
+    if (isConst(llvm::cast<ValueDecl>(D)->getType()))
+        return true;
+  }
+  if (const auto *OCPD = llvm::dyn_cast<ObjCPropertyDecl>(D)) {
+    if (OCPD->isReadOnly())
+      return true;
+  }
+  if (const auto *MPD = llvm::dyn_cast<MSPropertyDecl>(D)) {
+    if (!MPD->hasSetter())
+      return true;
+  }
+  if (const auto *CMD = llvm::dyn_cast<CXXMethodDecl>(D)) {
+    if (CMD->isConst())
+      return true;
+  }
+  return false;
+
+}
+
+bool isStaticMember(const Decl *D) {
+  if (const auto *CMD = llvm::dyn_cast<CXXMethodDecl>(D))
+    return CMD->isStatic();
+  if (const VarDecl *VD = llvm::dyn_cast<VarDecl>(D))
+    return VD->isStaticDataMember();
+  if (const auto *FD = llvm::dyn_cast<FieldDecl>(D))
+    return FD->isCXXInstanceMember();
+  if (const auto *OMD = llvm::dyn_cast<ObjCMethodDecl>(D))
+    return OMD->isClassMethod();
+  return false;
 }
 
 // For a macro usage `DUMP(foo)`, we want:
 //  - DUMP --> "macro"
 //  - foo --> "variable".
 SourceLocation getHighlightableSpellingToken(SourceLocation L,
                                              const SourceManager &SM) {
   if (L.isFileID())
     return SM.isWrittenInMainFile(L) ? L : SourceLocation{};
   // Tokens expanded from the macro body contribute no highlightings.
   if (!SM.isMacroArgExpansion(L))
     return {};
   // Tokens expanded from macro args are potentially highlightable.
   return getHighlightableSpellingToken(SM.getImmediateSpellingLoc(L), SM);
 }
 
 /// Consumes source locations and maps them to text ranges for highlightings.
 class HighlightingsBuilder {
 public:
   HighlightingsBuilder(const ParsedAST &AST)
       : TB(AST.getTokens()), SourceMgr(AST.getSourceManager()),
         LangOpts(AST.getLangOpts()) {}
 
-  void addToken(HighlightingToken T) { Tokens.push_back(T); }
-
-  void addToken(SourceLocation Loc, HighlightingKind Kind) {
+  HighlightingToken &addToken(SourceLocation Loc, HighlightingKind Kind) {
     Loc = getHighlightableSpellingToken(Loc, SourceMgr);
     if (Loc.isInvalid())
-      return;
+      return Dummy;
     const auto *Tok = TB.spelledTokenAt(Loc);
     assert(Tok);
+    return addToken(
+        halfOpenToRange(SourceMgr,
+                        Tok->range(SourceMgr).toCharRange(SourceMgr)),
+        Kind);
+  }
 
-    auto Range = halfOpenToRange(SourceMgr,
-                                 Tok->range(SourceMgr).toCharRange(SourceMgr));
-    Tokens.push_back(HighlightingToken{Kind, std::move(Range)});
+  HighlightingToken &addToken(Range R, HighlightingKind Kind) {
+    HighlightingToken HT;
+    HT.R = std::move(R);
+    HT.Kind = Kind;
+    Tokens.push_back(std::move(HT));
+    return Tokens.back();
   }
 
   std::vector<HighlightingToken> collect(ParsedAST &AST) && {
     // Initializer lists can give duplicates of tokens, therefore all tokens
     // must be deduplicated.
     llvm::sort(Tokens);
     auto Last = std::unique(Tokens.begin(), Tokens.end());
     Tokens.erase(Last, Tokens.end());
for (const Range &R : AST.getMacros().SkippedRanges) {
       // Create one token for each line in the skipped range, so it works
       // with line-based diffing.
       assert(R.start.line <= R.end.line);
       for (int Line = R.start.line; Line < R.end.line; ++Line) {
         // Don't bother computing the offset for the end of the line, just use
         // zero. The client will treat this highlighting kind specially, and
         // highlight the entire line visually (i.e. not just to where the text
         // on the line ends, but to the end of the screen).
-        NonConflicting.push_back({HighlightingKind::InactiveCode,
-                                  {Position{Line, 0}, Position{Line, 0}}});
+        HighlightingToken HT;
+        NonConflicting.emplace_back();
+        NonConflicting.back().Kind = HighlightingKind::InactiveCode;
+        NonConflicting.back().R.start.line = Line;
+        NonConflicting.back().R.end = NonConflicting.back().R.start;
       }
     }
     // Re-sort the tokens because that's what the diffing expects.
     llvm::sort(NonConflicting);
     return NonConflicting;
   }
 
 private:
   const syntax::TokenBuffer &TB;
   const SourceManager &SourceMgr;
   const LangOptions &LangOpts;
   std::vector<HighlightingToken> Tokens;
+  HighlightingToken Dummy; // returned from addToken(InvalidLoc)
 };
 
 /// Produces highlightings, which are not captured by findExplicitReferences,
 /// e.g. highlights dependent names and 'auto' as the underlying type.
 class CollectExtraHighlightings
     : public RecursiveASTVisitor<CollectExtraHighlightings> {
 public:
   CollectExtraHighlightings(HighlightingsBuilder &H) : H(H) {}
 
   bool VisitDecltypeTypeLoc(DecltypeTypeLoc L) {
     if (auto K = kindForType(L.getTypePtr()))
-      H.addToken(L.getBeginLoc(), *K);
+      H.addToken(L.getBeginLoc(), *K)
+          .addModifier(HighlightingModifier::Deduced);
     return true;
   }
 
   bool VisitDeclaratorDecl(DeclaratorDecl *D) {
     auto *AT = D->getType()->getContainedAutoType();
     if (!AT)
       return true;
     if (auto K = kindForType(AT->getDeducedType().getTypePtrOrNull()))
-      H.addToken(D->getTypeSpecStartLoc(), *K);
+      H.addToken(D->getTypeSpecStartLoc(), *K)
+          .addModifier(HighlightingModifier::Deduced);
     return true;
   }
 
   bool VisitOverloadExpr(OverloadExpr *E) {
     if (!E->decls().empty())
       return true; // handled by findExplicitReferences.
     H.addToken(E->getNameLoc(), HighlightingKind::DependentName);
     return true;

 std::vector<HighlightingToken> getSemanticHighlightings(ParsedAST &AST) {
   auto &C = AST.getASTContext();
   // Add highlightings for AST nodes.
   HighlightingsBuilder Builder(AST);
   // Highlight 'decltype' and 'auto' as their underlying types.
   CollectExtraHighlightings(Builder).TraverseAST(C);
   // Highlight all decls and references coming from the AST.
   findExplicitReferences(C, [&](ReferenceLoc R) {
-    if (auto Kind = kindForReference(R))
-      Builder.addToken(R.NameLoc, *Kind);
+    llvm::Optional<HighlightingKind> Result;
+    for (const NamedDecl *Decl : R.Targets) {
+      if (!canHighlightName(Decl->getDeclName()))
+        continue;
+      auto Kind = kindForDecl(Decl);
+      if (!Kind || (Result && Kind != Result))
+        continue;
+      Result = Kind;
+
+      auto &Tok = Builder.addToken(R.NameLoc, *Kind);
+      if (isConst(Decl))
+        Tok.addModifier(HighlightingModifier::Readonly);
+      if (isStaticMember(Decl))
+        Tok.addModifier(HighlightingModifier::StaticMember);
+      if (Decl->isDeprecated())
+        Tok.addModifier(HighlightingModifier::Deprecated);
+      if (R.IsDecl) // FIXME: also definition?
+        Tok.addModifier(HighlightingModifier::Declaration);
+    }
   });
   // Add highlightings for macro references.
   for (const auto &SIDToRefs : AST.getMacros().MacroRefs) {
     for (const auto &M : SIDToRefs.second)
-      Builder.addToken({HighlightingKind::Macro, M});
+      Builder.addToken(M, HighlightingKind::Macro);
   }
   for (const auto &M : AST.getMacros().UnknownMacros)
-    Builder.addToken({HighlightingKind::Macro, M});
+    Builder.addToken(M, HighlightingKind::Macro)
+        .addModifier(HighlightingModifier::Definition);
 
   return std::move(Builder).collect(AST);
 }
 
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, HighlightingKind K) {
   switch (K) {
   case HighlightingKind::Variable:
     return OS << "Variable";
case HighlightingKind::Primitive:
     return OS << "Primitive";
   case HighlightingKind::Macro:
     return OS << "Macro";
   case HighlightingKind::InactiveCode:
     return OS << "InactiveCode";
   }
   llvm_unreachable("invalid HighlightingKind");
 }
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, HighlightingModifier K) {
+  return OS << toSemanticTokenModifier(K);
+}
 
 std::vector<LineHighlightings>
 diffHighlightings(ArrayRef<HighlightingToken> New,
                   ArrayRef<HighlightingToken> Old) {
   assert(std::is_sorted(New.begin(), New.end()) &&
          "New must be a sorted vector");
   assert(std::is_sorted(Old.begin(), Old.end()) &&
          "Old must be a sorted vector");
if (NewLine != OldLine) {
       });
     }
   }
 
   return DiffedLines;
 }
 
 bool operator==(const HighlightingToken &L, const HighlightingToken &R) {
-  return std::tie(L.R, L.Kind) == std::tie(R.R, R.Kind);
+  return std::tie(L.R, L.Kind, L.Modifiers) ==
+         std::tie(R.R, R.Kind, R.Modifiers);
 }
 bool operator<(const HighlightingToken &L, const HighlightingToken &R) {
-  return std::tie(L.R, L.Kind) < std::tie(R.R, R.Kind);
+  return std::tie(L.R, L.Kind, R.Modifiers) <
+         std::tie(R.R, R.Kind, R.Modifiers);
 }
 bool operator==(const LineHighlightings &L, const LineHighlightings &R) {
   return std::tie(L.Line, L.Tokens) == std::tie(R.Line, R.Tokens);
 }
 
 std::vector<SemanticToken>
 toSemanticTokens(llvm::ArrayRef<HighlightingToken> Tokens) {
   assert(std::is_sorted(Tokens.begin(), Tokens.end()));
if (Last) {
       }
     } else {
       Out.deltaLine = Tok.R.start.line;
       Out.deltaStart = Tok.R.start.character;
     }
     assert(Tok.R.end.line == Tok.R.start.line);
     Out.length = Tok.R.end.character - Tok.R.start.character;
     Out.tokenType = static_cast<unsigned>(Tok.Kind);
+    Out.tokenModifiers = Tok.Modifiers;
 
     Last = &Tok;
   }
   return Result;
 }
 llvm::StringRef toSemanticTokenType(HighlightingKind Kind) {
   switch (Kind) {
   case HighlightingKind::Variable:
   case HighlightingKind::LocalVariable:
-  case HighlightingKind::StaticField:
     return "variable";
+  case HighlightingKind::Field:
+  case HighlightingKind::StaticField:
+    return "member";
+  case HighlightingKind::Method:
+  case HighlightingKind::StaticMethod:
+    return "member";
   case HighlightingKind::Parameter:
     return "parameter";
   case HighlightingKind::Function:
     return "function";
-  case HighlightingKind::Method:
-    return "member";
-  case HighlightingKind::StaticMethod:
-    // FIXME: better function/member with static modifier?
-    return "function";
-  case HighlightingKind::Field:
-    return "member";
   case HighlightingKind::Class:
     return "class";
   case HighlightingKind::Enum:
     return "enum";
   case HighlightingKind::EnumConstant:
     return "enumConstant"; // nonstandard
   case HighlightingKind::Typedef:
     return "type";
   case HighlightingKind::DependentType:
-    return "dependent"; // nonstandard
   case HighlightingKind::DependentName:
     return "dependent"; // nonstandard
   case HighlightingKind::Namespace:
     return "namespace";
   case HighlightingKind::TemplateParameter:
     return "typeParameter";
   case HighlightingKind::Concept:
     return "concept"; // nonstandard
   case HighlightingKind::Primitive:
     return "type";
   case HighlightingKind::Macro:
     return "macro";
   case HighlightingKind::InactiveCode:
     return "comment";
   }
   llvm_unreachable("unhandled HighlightingKind");
 }
 
+llvm::StringRef toSemanticTokenModifier(HighlightingModifier Modifier) {
+  switch (Modifier) {
+    case HighlightingModifier::Declaration:
+      return "declaration";
+    case HighlightingModifier::Definition:
+      return "definition";
+    case HighlightingModifier::Deprecated:
+      return "deprecated";
+    case HighlightingModifier::Readonly:
+      return "readonly";
+    case HighlightingModifier::StaticMember:
+      return "static";
+    case HighlightingModifier::Deduced:
+      return "deduced"; // nonstandard
+  }
+}
+
 std::vector<TheiaSemanticHighlightingInformation>
 toTheiaSemanticHighlightingInformation(
     llvm::ArrayRef<LineHighlightings> Tokens) {
   if (Tokens.size() == 0)
     return {};
 
   // FIXME: Tokens might be multiple lines long (block comments) in this case
   // this needs to add multiple lines for those tokens.

clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp

 //===--- AnnotateHighlightings.cpp -------------------------------*- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 #include "SemanticHighlighting.h"
 #include "refactor/Tweak.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/ScopedPrinter.h"
 
 namespace clang {
 namespace clangd {
 namespace {
 
 /// Annotate all highlighting tokens in the current file. This is a hidden tweak
 /// which is used to debug semantic highlightings.
 /// Before:
Expected<Tweak::Effect> AnnotateHighlightings::apply(const Selection &Inputs) {
   llvm::StringRef FilePath = SM.getFilename(Inputs.Cursor);
   for (const auto &Token : HighlightingTokens) {
     assert(Token.R.start.line == Token.R.end.line &&
            "Token must be at the same line");
     auto InsertOffset = positionToOffset(Inputs.Code, Token.R.start);
     if (!InsertOffset)
       return InsertOffset.takeError();
 
-    auto InsertReplacement = tooling::Replacement(
-        FilePath, *InsertOffset, 0,
-        ("/* " + toTextMateScope(Token.Kind) + " */").str());
+    std::string Comment = "/* ";
+    Comment.append(llvm::to_string(Token.Kind));
+    for (unsigned I = 0;
+         I <= static_cast<unsigned>(HighlightingModifier::LastModifier); ++I) {
+      if (Token.Modifiers & (1 <<I)) {
+        Comment.append(" [");
+        Comment.append(llvm::to_string(static_cast<HighlightingModifier>(I)));
+        Comment.push_back(']');
+      }
+    }
+    Comment.append(" */");
+    auto InsertReplacement =
+        tooling::Replacement(FilePath, *InsertOffset, 0, Comment);
     if (auto Err = Result.add(InsertReplacement))
       return std::move(Err);
   }
   return Effect::mainFileEdit(SM, std::move(Result));
 }
 
 } // namespace
 } // namespace clangd
 } // namespace clang