sammccall 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()))
----------------
ArcsinX wrote:
> 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?
I'm not following, because I think we can ignore *all* those tokens. We care
about macro names when the macros are *used* (tokens expanded from macro bodies
are associated from the macro name at the expansion point). But not where
they're defined.
> Also I found similar check in XRefs.cpp
Yes, there are a few restrictions there though:
- it's only used for identifiers, so there are fewer cases to consider
- performance doesn't matter at all, as it runs in response to an explicit
action and runs on ~2 tokens
> How to judge should we skip token or not if we do not have expanded tokens
> for it? Do you have any advice here?
I think we should just always skip in this case, let me try this out...
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83508/new/
https://reviews.llvm.org/D83508
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
