sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/Hover.cpp:542
+ CharSourceRange HighlightRange =
+ TokensTouchingCursor.front().range(SM).toCharRange(SM);
llvm::Optional<HoverInfo> HI;
----------------
nit: back() would fit better with the biases elsewhere
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:155
const SourceManager &SourceMgr = AST.getSourceManager();
- const FileEntry *F =
SourceMgr.getFileEntryForID(SourceMgr.getFileID(TokLoc));
+ auto Tok = syntax::tokenize(syntax::FileRange(SourceMgr, TokLoc, 1),
+ SourceMgr, AST.getLangOpts());
----------------
so this is just running the raw lexer...
both callers have a ParsedAST, can we call spelledTokenAt()? Either here,
passing in (TokenBuffer, Loc, TUPath) or in the caller, passing in
(syntax::Token*, SourceMgr, TUPath).
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:155
const SourceManager &SourceMgr = AST.getSourceManager();
- const FileEntry *F =
SourceMgr.getFileEntryForID(SourceMgr.getFileID(TokLoc));
+ auto Tok = syntax::tokenize(syntax::FileRange(SourceMgr, TokLoc, 1),
+ SourceMgr, AST.getLangOpts());
----------------
sammccall wrote:
> so this is just running the raw lexer...
>
> both callers have a ParsedAST, can we call spelledTokenAt()? Either here,
> passing in (TokenBuffer, Loc, TUPath) or in the caller, passing in
> (syntax::Token*, SourceMgr, TUPath).
It *seems* to always be the case that TokLoc is a spelled token, but this seems
slightly subtle and the previous code defended against marco locations. Worth a
comment
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:355
struct Reference {
- SourceLocation Loc;
+ syntax::Token Tok;
index::SymbolRoleSet Role;
----------------
document spelled or expanded.
It seems to be spelled, but I don't know why - expanded is more similar to the
old code, allows us to defer more work, and doesn't require the dubious front().
You could add a function here range(SourceMgr) --> Range
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:396
+ if (!Toks || Toks->empty() ||
+ !isInsideMainFile(Toks->front().location(), SM))
+ return true;
----------------
why is it safe to discard the rest of toks?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75474/new/
https://reviews.llvm.org/D75474
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
