[PATCH] D83508: [clangd][Hover] Don't use Decl if it is not related with tokens under cursor.

Aleksandr Platonov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 16 14:02:58 PDT 2020


ArcsinX added inline comments.


================
Comment at: clang-tools-extra/clangd/Selection.cpp:227
         continue;
+      // Ignore tokens in disabled preprocessor sections.
+      if (Buf.expandedTokens(SM.getMacroArgExpandedLocation(T->location()))
----------------
sammccall wrote:
> I think this is more work than we'd like to do in this loop (the whole file could be selected!) and it's also not obviously correct - it relies on the assumption that any token that doesn't expand to something by itself, isn't a macro argument or macro name, is part of an ignored region. (That may or may not be correct, but it's a nontrivial assumption to make here).
> 
> I think the API we need is something like `vector<TokenBuffer::Expansion> TokenBuffer::expansionsOverlapping(ArrayRef<Token> Spelled)`.
> This requires a couple of binary searches on the TokenBuffer side to compute, and on this side we can just look at the spelled token ranges for empty expansions and mark them in a bitmap.
> 
> I'm happy to try to put together a TokenBuffer patch if this is too much of a yak-shave, though.
>  it relies on the assumption that any token that doesn't expand to something by itself, isn't a macro argument or macro name, is part of an ignored region. (That may or may not be correct, but it's a nontrivial assumption to make here).

Example `#define FOO BAR`.
Expanded tokens:
Spelled tokens: `#`, `define`, `FOO`, `BAR`

I think we could ignore all except `FOO` in this case. Also I found similar check in XRefs.cpp `bool tokenSpelledAt(SourceLocation SpellingLoc, const syntax::TokenBuffer &TB)`

How to judge should we skip token or not if we do not have expanded tokens for it? Do you have any advice here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83508





More information about the cfe-commits mailing list