ArcsinX added inline comments.
================
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)
----------------
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).
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
