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) {
----------------
kadircet wrote:
> 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.
implemented a version of this that only copies data when requested. Hence the
pre/post filtering argument is gone.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77732/new/
https://reviews.llvm.org/D77732
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
