[clangd] Make use of token buffers in semantic highlighting

[kadircet]

• Add forgetten macroid check, before looking for an macroargexpansion.

[sammccall]

clang-tools-extra/clangd/SemanticHighlighting.cpp

135–137

nit: this can be const

149

only if this is a macro!

151–153

I think using TokenBuffer to translate expanded -> spelled isn't actually an improvement here as the API exposes several possibilities that can't actually happen in our case.

The specialized translation from expanded to spelled location is *policy*, and it's pretty easy to implement I think:

// For a macro usage `DUMP(foo)`, we want:
//  - DUMP --> "macro"
//  - foo --> "variable".
SourceLocation getHighlightableSpellingToken(SourceLocation L) {
  if (L.isFileID()) return SM.isWrittenInMainFile(L) ? L : {};
  // 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.getImmediateSpellingLocation(L));
}

once you have the spelling location, getting the range from tokenbuffer is easy :-)

[kadircet]

• Address comments

[sammccall]

clang-tools-extra/clangd/SemanticHighlighting.cpp

159

&& Tok->location() == Loc, right?

(I think this would make a nice method TokenBuffer::spelledTokenAt(SourceLocation) -> const Token*)

[kadircet]

• Use spelledTokenAt

clang-tools-extra/clangd/SemanticHighlighting.cpp

 #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"
+#include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Base64.h"
 #include "llvm/Support/Casting.h"
 #include <algorithm>
 
 namespace clang {
for (const NamedDecl *Decl : R.Targets) {
     Result = Kind;
   }
   return Result;
 }
 
 /// Consumes source locations and maps them to text ranges for highlightings.
 class HighlightingsBuilder {
 public:
-  HighlightingsBuilder(const SourceManager &SourceMgr,
-                       const LangOptions &LangOpts)
-      : SourceMgr(SourceMgr), LangOpts(LangOpts) {}
+  HighlightingsBuilder(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) {
-    if (Loc.isInvalid())
+    // This enables highlighting tokens coming from macro expansions, e.g.:
+    // #define MACRO SOME_NAME
+    // int MACRO;
+    //     ~~~~~ -> this will get highlighted as a macro token.
+    // As `de-conflicting` logic inside `collect` would drop these otherwise.
+    // There were tests specifically checking for that, not sure if it is needed
+    // though.
+    if (Loc.isMacroID() && !SourceMgr.isMacroArgExpansion(Loc))
       return;
-    if (Loc.isMacroID()) {
-      // Only intereseted in highlighting arguments in macros (DEF_X(arg)).
-      if (!SourceMgr.isMacroArgExpansion(Loc))
-        return;
-      Loc = SourceMgr.getSpellingLoc(Loc);
-    }
-
-    // Non top level decls that are included from a header are not filtered by
-    // topLevelDecls. (example: method declarations being included from
-    // another file for a class from another file).
-    // There are also cases with macros where the spelling loc will not be in
-    // the main file and the highlighting would be incorrect.
-    if (!isInsideMainFile(Loc, SourceMgr))
+    auto Toks = TB.spelledForExpanded(TB.expandedTokens(Loc));
+    if (!Toks || Toks->empty() ||
+        !isInsideMainFile(Toks->front().location(), SourceMgr))
       return;
 
-    auto Range = getTokenRange(SourceMgr, LangOpts, Loc);
-    if (!Range) {
-      // R should always have a value, if it doesn't something is very wrong.
-      elog("Tried to add semantic token with an invalid range");
-      return;
-    }
-    Tokens.push_back(HighlightingToken{Kind, *Range});
+    auto Range = halfOpenToRange(
+        SourceMgr, Toks->front().range(SourceMgr).toCharRange(SourceMgr));
+    Tokens.push_back(HighlightingToken{Kind, std::move(Range)});
   }
 
   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) {
       }
     }
     // 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;
 };
 
 /// Produces highlightings, which are not captured by findExplicitReferences,
 /// e.g. highlights dependent names and 'auto' as the underlying type.
 class CollectExtraHighlightings
return ArrayRef<HighlightingToken>(StartLineIt, AllTokens.end())
         return Token.R.start.line == Line;
       });
 }
 } // namespace
 
 std::vector<HighlightingToken> getSemanticHighlightings(ParsedAST &AST) {
   auto &C = AST.getASTContext();
   // Add highlightings for AST nodes.
-  HighlightingsBuilder Builder(AST.getSourceManager(), C.getLangOpts());
+  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);
   });
   // Add highlightings for macro references.