[PATCH] D41276: [clangd] Build in-memory index on symbols in files.

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 15 01:51:42 PST 2017


sammccall added inline comments.


================
Comment at: clangd/index/FileMemIndexManager.h:23
+/// all symbols.
+class FileMemIndexManager {
+public:
----------------
Discussed offline a bit:
 - FileIndex or PerFileIndex is a great name for this, if it implements `Index`. So I think it should (and forward to the MemIndex that it owns).
 - `FileSymbols` is mostly useful to implement this, so we could put them in the same header `FileIndex`and add a comment to that effect.
 - What this means for unittest organization - up to you


================
Comment at: unittests/clangd/IndexTests.cpp:120
+/// empty, all symbols in \p Path are removed.
+void updateFile(std::string Path, llvm::StringRef Code,
+                FileMemIndexManager *M) {
----------------
Can you make this `build()` returning a ParsedAST instead?

It adds a little duplication to the callsite:
   M.update("f1", build("f1", "...code..."));
but it makes this a much more "pure" function, and a good candidate for pulling out into a helper for other tests. 


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41276





More information about the cfe-commits mailing list