kadircet marked an inline comment as done.
kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:97
+/// paths used by \p FileSyms.
+void shardSlabToFiles(const SymbolSlab &Syms, const RelationSlab &Rels,
+ FileSymbols &FileSyms, llvm::StringRef HintPath) {
----------------
sammccall wrote:
> This screams out to be shared with BackgroundIndex::update(). Is it very hard
> to do?
>
> Things that are different:
> - no duplication of symbol into defining file (why?)
> - refs not gathered here (but I guess empty input -> empty output?)
> - include graph not gathered here (same)
> - this holds 3 copies of all the data at peak, other impl holds 2
>
> I imagine factoring out a function that returns a StringMap<File> like in
> BackgroundIndex::Update, but File has a build() function that returns an
> IndexFileIn or so.
> (Ooh, is depending on IndexFileIn a layering violation? Maybe we should move
> that into another header. Duplicating the struct is probably OK for now)
In backgroundindex we "prefilter" symbols and only duplicate the ones coming
from modified files, whereas in this one we duplicate every symbol no matter
what. I didn't want to change backgroundindex into post filtering mode (I
suppose that's what you mean by the last item).
as for the first bullet point, duplicating in case of preamble symbols wouldn't
matter since we don't merge(rather pick one) while building the index for
preamblesymbols. I suppose storing them twice shouldn't be a huge issue.
Depending to `IndexFileIn` in "Index.h" would be a violation, but depending on
it in "FileIndex.h" is fine.
I am happy to refactor a `StringMap<File> shardIndexToFiles(IndexFileIn,
HintPath)` into "FileIndex.h" that can be used by both backgroundindex and
this, if you are OK with above mentioned regressions.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77732/new/
https://reviews.llvm.org/D77732
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
