This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE341242: [clangd] Implement findOccurrences interface in
dynamic index. (authored by hokein, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D51279?vs=163539=163580#toc
Repository:
hokein updated this revision to Diff 163539.
hokein marked 3 inline comments as done.
hokein added a comment.
address review comments:
- build memindex with symbol slab and occurrence slab
- remove withAllCode in TestTU
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51279
sammccall added inline comments.
Comment at: unittests/clangd/TestTU.h:41
+ static TestTU withAllCode(llvm::StringRef HeaderCode, llvm::StringRef Code,
+llvm::StringRef Filename = "") {
hokein wrote:
> sammccall wrote:
> > We've
hokein updated this revision to Diff 163532.
hokein added a comment.
Minor cleanup.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51279
Files:
clangd/index/FileIndex.cpp
clangd/index/FileIndex.h
clangd/index/Index.h
clangd/index/MemIndex.cpp
clangd/index/MemIndex.h
hokein added inline comments.
Comment at: clangd/index/MemIndex.cpp:35
std::unique_ptr MemIndex::build(SymbolSlab Slab) {
auto Idx = llvm::make_unique();
sammccall wrote:
> This is still implicitly creating an index with no occurrences. Did you mean
> to
hokein updated this revision to Diff 163531.
hokein marked 4 inline comments as done.
hokein added a comment.
- rebase
- address review comments
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51279
Files:
clangd/index/FileIndex.cpp
clangd/index/FileIndex.h
hokein marked 3 inline comments as done.
hokein added a comment.
Sorry! Just realised I messed up this patch with
https://reviews.llvm.org/D50385 (mostly SymbolCollector changes), all the
comments about `SymbolCollector` are fixed in https://reviews.llvm.org/D50385.
Repository:
rCTE Clang
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
This basically looks good to go (some fixes needed but they're pretty clear I
think let me know if not!)
Comment at: clangd/index/FileIndex.cpp:63
+ auto Occurrences
ilya-biryukov added inline comments.
Comment at: clangd/index/FileIndex.cpp:48
+ if (TopLevelDecls) { // index main AST, set occurrence flag.
+CollectorOpts.OccurrenceFilter = SymbolOccurrenceKind::Declaration |
hokein wrote:
> ilya-biryukov wrote:
> >
hokein updated this revision to Diff 163066.
hokein added a comment.
Minor cleanup.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51279
Files:
clangd/index/FileIndex.cpp
clangd/index/FileIndex.h
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/MemIndex.cpp
hokein added a comment.
Thanks for the comments.
Comment at: clangd/index/FileIndex.cpp:48
+ if (TopLevelDecls) { // index main AST, set occurrence flag.
+CollectorOpts.OccurrenceFilter = SymbolOccurrenceKind::Declaration |
ilya-biryukov wrote:
> There
hokein updated this revision to Diff 163064.
hokein marked 16 inline comments as done.
hokein added a comment.
Herald added a subscriber: mgrang.
Address review comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51279
Files:
clangd/index/FileIndex.cpp
sammccall added a comment.
In https://reviews.llvm.org/D51279#1214268, @ilya-biryukov wrote:
> Just noticed I'm not on the reviewers list, sorry for chiming in without
> invitation. Was really excited about the change :-)
fixed :-)
Comment at: clangd/index/FileIndex.cpp:58
hokein added a comment.
In https://reviews.llvm.org/D51279#1214268, @ilya-biryukov wrote:
> Just noticed I'm not on the reviewers list, sorry for chiming in without
> invitation. Was really excited about the change :-)
Comments are always welcome :)
Comment at:
ilya-biryukov added a comment.
Just noticed I'm not on the reviewers list, sorry for chiming in without
invitation. Was really excited about the change :-)
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51279
___
cfe-commits
ilya-biryukov added a comment.
The memory usage looks good. Some NITs and a major consideration wrt to the
implementation of merging occurences from dynamic and static index.
Comment at: clangd/index/FileIndex.cpp:48
+ if (TopLevelDecls) { // index main AST, set occurrence
hokein added a comment.
Some numbers of memory usage from running this on my machine:
| File | Preamble AST | Main AST |
| SemaDecl.cpp | 14.5MB | 108.1KB (symbols) + 16.5KB (occurrences) |
| CodeComplete.cpp | 15.4MB | 53.9KB (symbols)
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric,
ilya-biryukov.
Implement the interface in
- FileIndex
- MemIndex
- MergeIndex
Depends on https://reviews.llvm.org/D50385.
Repository:
rCTE Clang Tools
18 matches
Mail list logo
