ArcsinX added inline comments.
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:589
// but not things like disabled preprocessor sections.
if (!(tokenSpelledAt(Tok.location(), TB) || TB.expansionStartingAt(&Tok)))
return false;
----------------
here
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:580
return false;
- // No point guessing the same location we started with.
- if (Tok.location() == Word.Location)
----------------
ArcsinX wrote:
> sammccall wrote:
> > ArcsinX wrote:
> > > sammccall wrote:
> > > > why can't this happen anymore?
> > > As I understand, we call `findNearbyIdentifier()` only when the word is
> > > not an identifier. And `Tok` is always identifier here.
> > >
> > > Btw, even if `findNearbyIdentifier()` could be called for an identifier,
> > > I could not get why it's bad to return current position here.
> > >
> > > Am I wrong?
> > > As I understand, we call findNearbyIdentifier() only when the word is not
> > > an identifier
> >
> > I'm not sure where you're getting that.
> > If you mean the bail-out inside the function itself:
> >
> > ```
> > // Don't use heuristics if this is a real identifier.
> > // Unlikely identifiers are OK if they were used as identifiers nearby.
> > if (Word.ExpandedToken)
> > return nullptr;
> > ```
> >
> > then "real identifier" here means it's an identifier after preprocessing.
> > We're still getting here if it's e.g. part of a macro body, or code that's
> > `#ifdef`'d out.
> > The spelled token in those cases is an identifier, there's just no
> > corresponding expanded token.
> >
> > (I'm surprised we don't have a test covering this though!)
> >
> > > Btw, even if findNearbyIdentifier() could be called for an identifier, I
> > > could not get why it's bad to return current position here.
> >
> > The point of this function is that *this* occurrence of the word can't be
> > resolved, so let's try another one nearby.
> > If we just return the same occurrence, then we're certainly not going to be
> > able to resolve it.
> Thanks for your clarification. I will revert this change (and will try to add
> a test as well).
I failed to create a test for this.
I can create a test for identifiers in a macro body or under `#ifdef`, but
such a test passes without ` if (Tok.location() == Word.Location)` because of
this check:
```
if (!(tokenSpelledAt(Tok.location(), TB) || TB.expansionStartingAt(&Tok)))
return false;
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87891/new/
https://reviews.llvm.org/D87891
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
