This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Prefer location from codegen files when merging symbols.
ClosedPublic

Authored by ioeric on Feb 11 2019, 3:28 AM.

Details

Summary

For example, if an index symbol has location in a .proto file and an AST symbol
has location in a generated .proto.h file, then we prefer location in .proto
which is more meaningful to users.

Also use mergeSymbols to get the preferred location between AST location and index location in go-to-def.

Event Timeline

ioeric created this revision.Feb 11 2019, 3:28 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 11 2019, 3:28 AM
sammccall accepted this revision.Feb 11 2019, 5:34 AM
sammccall added inline comments.
clangd/XRefs.cpp
261

nit: move this and toIndexLocation together?

364

a little unclear what "we only do this" refers to - maybe missing a first line of the comment here.

// Use merge logic to choose AST or index declaration.

clangd/index/Merge.cpp
155

I think it might be clearer what this is doing, and more similar with surrounding code, if the function returns a bool:

bool prefer(const SymbolLocation &Candidate, const SymbolLocation &Baseline);
...
if (!S.Definition || prefer(O.Definition, S.Definition))
  S.Definition = O.Definition;

up to you

This revision is now accepted and ready to land.Feb 11 2019, 5:34 AM
ioeric updated this revision to Diff 186245.Feb 11 2019, 6:44 AM
ioeric marked 3 inline comments as done.
  • Address review comments.
This revision was automatically updated to reflect the committed changes.