This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Move buildStaticIndex() to SymbolYAML
ClosedPublic

Authored by kbobyrev on Sep 4 2018, 6:15 AM.

Details

Summary

buildStaticIndex() is used by two other tools that I'm building, now it's useful outside of tool/ClangdMain.cpp.

Diff Detail

Repository
rL LLVM

Event Timeline

kbobyrev created this revision.Sep 4 2018, 6:15 AM
kbobyrev updated this revision to Diff 163797.Sep 4 2018, 6:16 AM

Move function documentation to the header file.

sammccall accepted this revision.Sep 4 2018, 7:55 AM
sammccall added inline comments.
clang-tools-extra/clangd/index/SymbolYAML.cpp
193 ↗(On Diff #163797)

this is deep copying the symbols for no reason

just build(std::move(Slab)) below.
(I know you're just moving this code, but while you're here...)

clang-tools-extra/clangd/index/SymbolYAML.h
44 ↗(On Diff #163797)

nit: can you just say "index file", and call the parameter "SymbolFile"?
See D51585 which adds another format (it can be detected by content sniffing, so no parameter needed)

47 ↗(On Diff #163797)

I'd prefer loadIndex for this function, what do you think?

"load" over "build" because much of the indexing work has already been done in producing the file, and in future potentially even more (serializing posting lists).
"static" is coupled to this index's role in ClangdMain, and the point of putting the function here is to allow other uses, so I'd prefer to drop that word.

This revision is now accepted and ready to land.Sep 4 2018, 7:55 AM
kbobyrev updated this revision to Diff 163823.Sep 4 2018, 8:08 AM
kbobyrev marked 3 inline comments as done.

Rebase and resolve comments

This revision was automatically updated to reflect the committed changes.