[PATCH] D75447: [clangd] Make use of token buffers in semantic highlighting

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:134
 public:
-  HighlightingsBuilder(const SourceManager &SourceMgr,
-                       const LangOptions &LangOpts)
-      : SourceMgr(SourceMgr), LangOpts(LangOpts) {}
+  HighlightingsBuilder(ParsedAST &AST)
+      : TB(AST.getTokens()), SourceMgr(AST.getSourceManager()),
----------------
nit: this can be const


================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:148
+    // though.
+    if (!SourceMgr.isMacroArgExpansion(Loc))
       return;
----------------
only if this is a macro!


================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:150
       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() ||
----------------
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 :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75447