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

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE331189: [clangd] Using index for GoToDefinition. (authored by hokein, committed by ). Changed prior to commit: https://reviews.llvm.org/D45717?vs=144561=144566#toc Repository: rCTE Clang Tools

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

hokein added a comment. Thanks for the review! Comment at: clangd/XRefs.cpp:277 +// it. +auto ToLSPLocation = []( +const SymbolLocation ) -> llvm::Optional { sammccall wrote: > hokein wrote: > >

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

hokein updated this revision to Diff 144561. hokein marked 7 inline comments as done. hokein added a comment. Address review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45717 Files: clangd/ClangdServer.cpp clangd/XRefs.cpp clangd/XRefs.h

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

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. A few more nits (and some ideas for further restructuring the merge logic). Otherwise LG, let's land this! Comment at: clangd/XRefs.cpp:215 -

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

hokein added a comment. Thanks for the useful comments! I refined the patch, and it becomes a bit larger (including some moving stuff). Comment at: clangd/XRefs.cpp:137 + +IdentifiedSymbol getSymbolAtPosition(ParsedAST , SourceLocation Pos) { + auto DeclMacrosFinder =

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

hokein updated this revision to Diff 143728. hokein marked 14 inline comments as done. hokein added a comment. Address review comments: - clarify the flow of go to definition. - simplify the test code. - some function move-out stuff. Repository: rCTE Clang Tools Extra

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

sammccall added a comment. Thanks for the changes! I still find the logic quite confusing. Happy to chat offline if useful. Comment at: clangd/XRefs.cpp:137 + +IdentifiedSymbol getSymbolAtPosition(ParsedAST , SourceLocation Pos) { + auto DeclMacrosFinder = std::make_shared(

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

hokein added a comment. I have updated the patch according to offline discussion -- for each symbol, we will return both decl and def locations (if available, def first) as they seems to be most sensible to users. We always prefer location from AST over Index when conflicts.

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

hokein updated this revision to Diff 143508. hokein marked 7 inline comments as done. hokein added a comment. Refine the patch and address all review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45717 Files: clangd/ClangdServer.cpp clangd/XRefs.cpp

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

sammccall added inline comments. Comment at: clangd/XRefs.cpp:209 + llvm::DenseSet QuerySyms; + llvm::DenseMap ResultCandidates; sammccall wrote: > the approach could some documentation - I find it hard to follow the code. > > This

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

sammccall added inline comments. Comment at: clangd/XRefs.cpp:209 + llvm::DenseSet QuerySyms; + llvm::DenseMap ResultCandidates; the approach could some documentation - I find it hard to follow the code. This function is probably too big

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

hokein created this revision. hokein added a reviewer: sammccall. Herald added subscribers: MaskRay, ioeric, jkorous-apple, ilya-biryukov, klimek. This patch adds index support for GoToDefinition -- when we don't get the definition from local AST, we query our index (Static) index to get it.