[PATCH] D59481: [clangd] Count number of references while merging RefSlabs inside FileIndex
Eric Liu via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 19 02:36:57 PDT 2019
ioeric added a comment.
> I don't follow why this can over-count, FileIndex keeps only one RefSlab per each file, so we can't over-count? Did you mean it will be more than #TUs ?
Yes. This is how `Symbol::References` is described in its documentation. If we want to change/expand the semantic, we need to make it clear in the documentation as well.
================
Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:170
I.first->second = mergeSymbol(I.first->second, Sym);
+ // We re-count number of references while merging refs from scratch.
+ I.first->getSecond().References = 0;
----------------
kadircet wrote:
> ioeric wrote:
> > note that references might not always exist. SymbolCollector can be set up to not collect references.
> I thought we wouldn't make any promises about `References` in such a case, do we?
Is this documented somewhere?
`References` and `RefSlab` have been two independent things. `References` existed before `RefSlab`. Now might be a good time to unify them, but I think we should think about how this is reflected in their definitions (documentation etc) and other producers and consumers of the structures.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59481/new/
https://reviews.llvm.org/D59481
More information about the cfe-commits
mailing list