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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
