Page MenuHomePhabricator

[clangd] findNearbyIdentifier(): fix the word search in the token stream.
ClosedPublic

Authored by ArcsinX on Jul 30 2020, 12:41 AM.

Details

Summary

Without this patch the word occurrence search always returns the first token of the file.
Despite of that, findNeardyIdentifier() returns the correct result (but inefficently) until there are several matched tokens with the same value floor(log2(<token line> - <word line>)) (e.g. several matched tokens on the same line).

Diff Detail

Event Timeline

ArcsinX created this revision.Jul 30 2020, 12:41 AM
ArcsinX requested review of this revision.Jul 30 2020, 12:41 AM
kadircet accepted this revision.Jul 30 2020, 2:31 AM

thanks for catching this, LGTM! This might still end up traversing the whole file in non-existent cases, but I suppose that's an adventure for another day.

I think we should also cherry-pick this into release branch. I've filed https://bugs.llvm.org/show_bug.cgi?id=46905 for cherry-picking please update the bug
with the commit hash once you land the fix (or let me know if you don't have an account in bugzilla)

This revision is now accepted and ready to land.Jul 30 2020, 2:31 AM

oh and also, how did you notice the discrepancy? was it a random encounter while reading the code or did you notice a misbehaviour?

ArcsinX added a comment.EditedJul 30 2020, 2:38 AM

oh and also, how did you notice the discrepancy? was it a random encounter while reading the code or did you notice a misbehaviour?

Case was like this

void f1(int [[paramValue]]){
}

void f2(int paramValue){
}

void f3(int paramValue){
}

void f4(int paramValue){
// param^Value
}

Because of
floor(log2(8)) = floor(log2(9)) ... = floor(log2(15)) = 3

I think we should also cherry-pick this into release branch. I've filed https://bugs.llvm.org/show_bug.cgi?id=46905 for cherry-picking please update the bug
with the commit hash once you land the fix (or let me know if you don't have an account in bugzilla)

I do not have bugzilla account, so could you please do this for me?