This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Clean-up XRefs.cpp from Lexer usages and unnecessary SourceLoc transformations
ClosedPublic

Authored by kadircet on Feb 26 2020, 4:40 AM.

Details

Summary

Get rid of calls to lexer and unnecessary source location
transformations.

Diff Detail

Event Timeline

kadircet created this revision.Feb 26 2020, 4:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 26 2020, 4:40 AM
kadircet updated this revision to Diff 246682.Feb 26 2020, 5:19 AM

Add more cleaning.

getDeclAtPosition builds a SelectionTree underneath and it only operates on
spelling locations inside main file. So it is invalid to pass any expansion
locations to it.

There is no need to call getBeginningOfIdentifier for offsets being thrown at
SelectionTrees.

kadircet updated this revision to Diff 246690.Feb 26 2020, 6:07 AM
  • Also clean usage in getSymbolInfo
kadircet retitled this revision from [clangd] Clean-up locateSymbolAt to [clangd] Clean-up XRefs.cpp from Lexer usages and unnecessary SourceLoc transformations.Feb 26 2020, 6:07 AM
kadircet updated this revision to Diff 246701.Feb 26 2020, 6:27 AM
  • Handle llvm::Expected failures
sammccall added inline comments.Feb 26 2020, 7:43 AM
clang-tools-extra/clangd/XRefs.cpp
224

used only once - inline?

228–245

covering->touching

or just TouchedIdentifier for brevity

429

apparently we don't have tests for non-identifier cases, but they did work. let's keep them working for now :) CurLoc here

498

again, CurLoc?

kadircet updated this revision to Diff 246734.Feb 26 2020, 7:56 AM
kadircet marked 4 inline comments as done.
  • Address comments
clang-tools-extra/clangd/XRefs.cpp
429

also get rid of the bail-out when no tokens were touched. (which means we can now possibly trigger on ^~Foo().)

498

ah thanks for catching this one especially, it also means we don't need to bail out when there are no identifiers touching cursor(it is only necessary for macros).
Updated it to do that, PTAL.

sammccall accepted this revision.Feb 26 2020, 8:09 AM
sammccall added inline comments.
clang-tools-extra/clangd/XRefs.cpp
429

can I bother you to add a test for this (or some non-identifier case, assuming any works)

This revision is now accepted and ready to land.Feb 26 2020, 8:09 AM
kadircet updated this revision to Diff 246750.Feb 26 2020, 8:50 AM
kadircet marked 3 inline comments as done.
  • Add tests for non-identifier cases.
This revision was automatically updated to reflect the committed changes.