sammccall added a comment.
In D87891#2291838 <https://reviews.llvm.org/D87891#2291838>, @ArcsinX wrote:
> Thanks for your reply, I will rethink this patch.
FWIW I think if we drop most of the math in favor of using SourceManager's line
translation, this doesn't add much complexity and is probably a bit more direct
and efficient.
================
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:
> > 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.
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
