[PATCH] D47846: [clangd] Implementation of textDocument/documentSymbol

This revision was automatically updated to reflect the committed changes. Closed by commit rL336386: [clangd] Implementation of textDocument/documentSymbol (authored by malaperle, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47846 File

[PATCH] D47846: [clangd] Implementation of textDocument/documentSymbol

malaperle added a comment. Thanks a lot for the great comments (as always)! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47846 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/

[PATCH] D47846: [clangd] Implementation of textDocument/documentSymbol

malaperle updated this revision to Diff 154280. malaperle marked 7 inline comments as done. malaperle added a comment. Address comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47846 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp

[PATCH] D47846: [clangd] Implementation of textDocument/documentSymbol

sammccall added a comment. (Reasoning for not using SymbolCollector totally makes sense, thanks for the breakdown!) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47846 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:/

[PATCH] D47846: [clangd] Implementation of textDocument/documentSymbol

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Sorry, I thought I'd accepted this already! Comment at: clangd/ClangdServer.cpp:467 + }; + WorkScheduler.runWithAST("Hover", File, Bind(Action, std::move(CB))); +} --

[PATCH] D47846: [clangd] Implementation of textDocument/documentSymbol

malaperle updated this revision to Diff 153742. malaperle added a comment. Rebased. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47846 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/FindSymbols.cpp c

[PATCH] D47846: [clangd] Implementation of textDocument/documentSymbol

malaperle updated this revision to Diff 153611. malaperle added a comment. Fix handling of externs, definition vs declaration and call more common code. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47846 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/

[PATCH] D47846: [clangd] Implementation of textDocument/documentSymbol

malaperle added inline comments. Comment at: clangd/FindSymbols.cpp:181 +/// Finds document symbols in the main file of the AST. +class DocumentSymbolsConsumer : public index::IndexDataConsumer { + ASTContext * sammccall wrote: > I guess the alternative here

[PATCH] D47846: [clangd] Implementation of textDocument/documentSymbol

sammccall added inline comments. Comment at: clangd/SourceCode.cpp:194 + if (!llvm::sys::path::is_absolute(FilePath)) { +if (!SourceMgr.getFileManager().makeAbsolutePath(FilePath)) { + log("Could not turn relative path to absolute: " + FilePath); sammcc

[PATCH] D47846: [clangd] Implementation of textDocument/documentSymbol

sammccall added a comment. This looks great! Would like to get your thoughts on reusing/not reusing SymbolCollector. Comment at: clangd/FindSymbols.cpp:181 +/// Finds document symbols in the main file of the AST. +class DocumentSymbolsConsumer : public index::IndexDataConsumer

[PATCH] D47846: [clangd] Implementation of textDocument/documentSymbol

malaperle planned changes to this revision. malaperle added a comment. I found some issues while testing, I will investigate before review. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47846 ___ cfe-commits mailing list cfe-commit

[PATCH] D47846: [clangd] Implementation of textDocument/documentSymbol

malaperle updated this revision to Diff 152953. malaperle added a comment. Rebased. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47846 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/FindSymbols.cpp c

[PATCH] D47846: [clangd] Implementation of textDocument/documentSymbol

malaperle added inline comments. Comment at: unittests/clangd/FindSymbolsTests.cpp:39 } +MATCHER_P(QName, Name, "") { + if (arg.containerName.empty()) I updated the other tests to use this in https://reviews.llvm.org/D47847 Repository: rCTE Clang Tools Extr

[PATCH] D47846: [clangd] Implementation of textDocument/documentSymbol

malaperle created this revision. Herald added subscribers: cfe-commits, jkorous, MaskRay, ioeric, ilya-biryukov. An AST-based approach is used to retrieve the document symbols rather than an in-memory index query. The index is not an ideal fit to achieve this because of the file-centric query bein