[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