[PATCH] D45717: [clangd] Using index for GoToDefinition.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clangd/XRefs.cpp:209
 
+  llvm::DenseSet<SymbolID> QuerySyms;
+  llvm::DenseMap<SymbolID, Location> ResultCandidates;
----------------
sammccall wrote:
> the approach could some documentation - I find it hard to follow the code.
> 
> This function is probably too big by now and should be split up - but without understanding the goal I can't suggest how.
Having read the code in more detail, I'd suggest a structure like:
 - we identify the symbols being searched for
 - for each, we have a definition slot and a non-definition slot
 - initially we populate one of the slots with our preferred declaration from the AST (if location is available)
 - then we query the index and overwrite either/both slots with returned info
 - finally, we return populated slots for all symbols, definition first

(It's not clear how to order the returned symbols if there are multiple. When can this happen?)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45717