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
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
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
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
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)));
+}
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
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
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 is
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);
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
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
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
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
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
14 matches
Mail list logo
