[PATCH] D41232: [clangd] Add a FileSymbols container that manages symbols from multiple files.
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 14 06:13:46 PST 2017
sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clangd/index/FileSymbols.h:39
+
+ /// \brief Removes snapshots of \p Path.
+ void remove(PathRef Path);
----------------
Consider just accepting `update(Path, nullptr)` for this (or make callers pass an empty slab).
This can make callsites that might call one or the other conditionally, less awkward.
================
Comment at: unittests/clangd/IndexTests.cpp:124
+TEST(FileSymbolsTest, UpdateAndGet) {
+ FileSymbols FS;
----------------
Can you move these tests into a separate cpp file?
The code overlap is really minimal, and they test things from different headers.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41232
More information about the cfe-commits
mailing list