[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