hokein updated this revision to Diff 162611.
hokein marked 7 inline comments as done.
hokein added a comment.
Update the patch based on our new discussion
- SymbolOccurrenceSlab for storing underlying occurrence data
- reuse SymbolCollector to collect symbol occurrences
Repository:
rCTE Clang
sammccall added inline comments.
Comment at: clangd/index/Index.h:310
+
+ llvm::ArrayRef findOccurrences(const SymbolID &ID) const {
+auto It = SymbolOccurrences.find(ID);
As discussed offline: the merge of occurrences into SymbolSlab seems
problematic to m
ioeric added inline comments.
Comment at: clangd/index/SymbolCollector.cpp:241
-SymbolCollector::SymbolCollector(Options Opts) : Opts(std::move(Opts)) {}
+class SymbolCollector::CollectOccurrence {
+public:
I don't see a strong reason for the separation of `Col
ioeric added inline comments.
Comment at: clangd/index/SymbolCollector.h:74
+// If not null, SymbolCollector will collect symbols.
+const CollectSymbolOptions *SymOpts;
+// If not null, SymbolCollector will collect symbol occurrences.
Use `llvm::Optio
ilya-biryukov added inline comments.
Comment at: clangd/index/Index.h:46
+
+inline bool operator==(const SymbolLocation::Position &L,
+ const SymbolLocation::Position &R) {
hokein wrote:
> ilya-biryukov wrote:
> > NIT: having friend decls in
hokein added inline comments.
Comment at: clangd/index/Index.h:46
+
+inline bool operator==(const SymbolLocation::Position &L,
+ const SymbolLocation::Position &R) {
ilya-biryukov wrote:
> NIT: having friend decls inside the classes themselv
hokein updated this revision to Diff 161972.
hokein marked an inline comment as done.
hokein added a comment.
Add one more comment.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50385
Files:
clangd/index/FileIndex.cpp
clangd/index/Index.cpp
clangd/index/Index.h
clangd/
hokein updated this revision to Diff 161962.
hokein marked 6 inline comments as done.
hokein added a comment.
Address review comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50385
Files:
clangd/index/FileIndex.cpp
clangd/index/Index.cpp
clangd/index/Index.h
clan
ilya-biryukov added inline comments.
Comment at: clangd/index/Index.cpp:139
+std::sort(Occurrences.begin(), Occurrences.end(),
+ [](const SymbolOccurrence &L, const SymbolOccurrence &R) {
+return L < R;
NIT: remove the lambda? usi
hokein updated this revision to Diff 161927.
hokein added a comment.
Herald added a subscriber: kadircet.
Update the patch based on our offline discussion
- only one single clang intefaces implementation, and move finding references
to current symbol collector;
- store references in SymbolSlab;
hokein added a comment.
> Hmm, I think this is the same for other symbol payload e.g. definition can be
> missing for a symbol. And it seems to me that the concern is on the
> SymbolSlab level: if a slab is for a single TU, users should expect missing
> information; if a slab is merged from all
ilya-biryukov added a comment.
In https://reviews.llvm.org/D50385#1193600, @ioeric wrote:
> In https://reviews.llvm.org/D50385#1193545, @hokein wrote:
>
> > In https://reviews.llvm.org/D50385#1191914, @ioeric wrote:
> >
> > > 2 high-level questions:
> > >
> > > 1. What's the reason for having a s
ioeric added a comment.
In https://reviews.llvm.org/D50385#1193545, @hokein wrote:
> In https://reviews.llvm.org/D50385#1191914, @ioeric wrote:
>
> > 2 high-level questions:
> >
> > 1. What's the reason for having a separate `SymbolOccurrenceSlab`? Could
> > store occurrences as extra payload of
hokein added a comment.
In https://reviews.llvm.org/D50385#1191914, @ioeric wrote:
> 2 high-level questions:
>
> 1. What's the reason for having a separate `SymbolOccurrenceSlab`? Could
> store occurrences as extra payload of `Symbol`?
Storing occurrences in `Symbol` structure is easy to misus
ioeric added a comment.
2 high-level questions:
1. What's the reason for having a separate `SymbolOccurrenceSlab`? Could store
occurrences as extra payload of `Symbol`?
2. Could we merge `SymbolOccurrenceCollector` into the existing
`SymbolCollector`? They look a lot alike. Having another inde
hokein created this revision.
hokein added reviewers: ilya-biryukov, ioeric.
Herald added subscribers: arphaman, mgrang, jkorous, MaskRay, mgorny.
This patch implements a SymbolOccurenceCollector, which will be used to:
- Find all occurrences in AST
- Find all occurrences in MemIndex
Repository
16 matches
Mail list logo
