[clangd] Merge ClangdServer::DynamicIndex into FileIndex. NFC.

[ioeric]

Summary

FileIndex now provides explicit interfaces for preamble and main file updates.
This avoids growing parameter list when preamble and main symbols diverge
further (e.g. D52078). This also gets rid of the hack in indexAST that
inferred main file index based on TopLevelDecls.

Also separate indexMainDecls from indexAST.

[sammccall]

Glorious! Think the APIs could be simplified/clarified a little further, but up to you.

clangd/index/FileIndex.cpp

24 ↗

(On Diff #165901)

assert DeclsToIndex is empty if !IsIndexMainAST?

clangd/index/FileIndex.h

77 ↗

(On Diff #165901)

can't this just take PathRef + ParsedAST?
(same data, but clearer semantics)

114 ↗

(On Diff #165901)

again, take ParsedAST here?

118 ↗

(On Diff #165901)

This isn't quite right I think - we don't index the symbols that only occur in the main file.

120 ↗

(On Diff #165901)

this shouldn't return any refs, just return SymbolSlab here

121 ↗

(On Diff #165901)

indexPreamble would be clearer I think.

unittests/clangd/TestTU.cpp

51 ↗

(On Diff #165901)

this should return FileIndex instead I think, with both preamble and main index.
But this requires fixing callers (since FileIndex isn't an index).
Leave a fixme?

(Maybe I should just bite the bullet and expose the MergedIndex class so we don't have to deal with these indirections...)

[ioeric]

• addressed review comments

clangd/index/FileIndex.h

121 ↗

(On Diff #165901)

What about indexHeaderSymbols? indexPreamble seems to require understanding of how preamble is built, but it doesn't seem to matter here if AST was built from preamble or not.

[sammccall]

clangd/index/FileIndex.h

121 ↗

(On Diff #165901)

indexHeaderSymbols is better, SGTM.

I kind of meant the "conceptual" preamble here - but headers is more accurate (mid-file #includes are possible), and has no risk of confusion with the concrete cacheable preamble.

[sammccall]

clangd/index/FileIndex.h

77 ↗

(On Diff #165901)

You're still taking the top-level decls, these can just come from the ParsedAST though?

[ioeric]

clangd/index/FileIndex.h

77 ↗

(On Diff #165901)

Ah right. Sorry! Sent rL342473.

clang-tools-extra/trunk/clangd/ClangdServer.h

public:
   /// if compilation arguments changed on calls to forceReparse().
   ///
   /// After each parsing request finishes, ClangdServer reports diagnostics to
   /// \p DiagConsumer. Note that a callback to \p DiagConsumer happens on a
   /// worker thread. Therefore, instances of \p DiagConsumer must properly
   /// synchronize access to shared state.
   ClangdServer(GlobalCompilationDatabase &CDB, FileSystemProvider &FSProvider,
                DiagnosticsConsumer &DiagConsumer, const Options &Opts);
-  ~ClangdServer();
 
   /// Set the root path of the workspace.
   void setRootPath(PathRef RootPath);
 
   /// Add a \p File to the list of tracked C++ files or update the contents if
   /// \p File is already tracked. Also schedules parsing of the AST for it on a
   /// separate thread. When the parsing is complete, DiagConsumer passed in
   /// constructor will receive onDiagnosticsReady callback.

 
 private:
   /// FIXME: This stats several files to find a .clang-format file. I/O can be
   /// slow. Think of a way to cache this.
   llvm::Expected<tooling::Replacements>
   formatCode(llvm::StringRef Code, PathRef File,
              ArrayRef<tooling::Range> Ranges);
 
-  class DynamicIndex;
   typedef uint64_t DocVersion;
 
   void consumeDiagnostics(PathRef File, DocVersion Version,
                           std::vector<Diag> Diags);
 
   tooling::CompileCommand getCompileCommand(PathRef File);
 
   GlobalCompilationDatabase &CDB;
   DiagnosticsConsumer &DiagConsumer;
   FileSystemProvider &FSProvider;
 
   /// Used to synchronize diagnostic responses for added and removed files.
   llvm::StringMap<DocVersion> InternalVersion;
 
   Path ResourceDir;
   // The index used to look up symbols. This could be:
   //   - null (all index functionality is optional)
   //   - the dynamic index owned by ClangdServer (DynamicIdx)
   //   - the static index passed to the constructor
   //   - a merged view of a static and dynamic index (MergedIndex)
   const SymbolIndex *Index;
   // If present, an index of symbols in open files. Read via *Index.
-  std::unique_ptr<DynamicIndex> DynamicIdx;
+  std::unique_ptr<FileIndex> DynamicIdx;
   // If present, storage for the merged static/dynamic index. Read via *Index.
   std::unique_ptr<SymbolIndex> MergedIndex;
 
   // GUARDED_BY(CachedCompletionFuzzyFindRequestMutex)
   llvm::StringMap<llvm::Optional<FuzzyFindRequest>>
       CachedCompletionFuzzyFindRequestByFile;
   mutable std::mutex CachedCompletionFuzzyFindRequestMutex;
 

clang-tools-extra/trunk/clangd/ClangdServer.cpp

 //===--- ClangdServer.cpp - Main clangd server code --------------*- C++-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===-------------------------------------------------------------------===//
 
 #include "ClangdServer.h"
 #include "CodeComplete.h"
 #include "FindSymbols.h"
 #include "Headers.h"
 #include "SourceCode.h"
 #include "Trace.h"
 #include "XRefs.h"
+#include "index/FileIndex.h"
 #include "index/Merge.h"
 #include "clang/Format/Format.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Refactoring/RefactoringResultConsumer.h"
 #include "clang/Tooling/Refactoring/Rename/RenamingAction.h"
void handle(tooling::AtomicChanges SourceReplacements) override {
     assert(!Result.hasValue());
     Result = std::move(SourceReplacements);
   }
 
   Optional<Expected<tooling::AtomicChanges>> Result;
 };
 } // namespace
 
-/// The dynamic index tracks symbols visible in open files.
-/// For boring reasons, it doesn't implement SymbolIndex directly - use index().
-class ClangdServer::DynamicIndex {
-public:
-  DynamicIndex(std::vector<std::string> URISchemes)
-      : PreambleIdx(URISchemes), MainFileIdx(URISchemes),
-        MergedIndex(mergeIndex(&MainFileIdx, &PreambleIdx)) {}
-
-  const SymbolIndex &index() const { return *MergedIndex; }
-
-  // Returns callbacks that can be used to update the index with new ASTs.
-  // Index() presents a merged view of the supplied main-file and preamble ASTs.
-  std::unique_ptr<ParsingCallbacks> makeUpdateCallbacks() {
-    struct CB : public ParsingCallbacks {
-      CB(ClangdServer::DynamicIndex *This) : This(This) {}
-      DynamicIndex *This;
+// Returns callbacks that can be used to update the FileIndex with new ASTs.
+std::unique_ptr<ParsingCallbacks> makeUpdateCallbacks(FileIndex *FIndex) {
+    CB(FileIndex *FIndex) : FIndex(FIndex) {}
+    FileIndex *FIndex;
 
-      void onPreambleAST(PathRef Path, ASTContext &Ctx,
-                         std::shared_ptr<clang::Preprocessor> PP) override {
-        This->PreambleIdx.update(Path, &Ctx, std::move(PP));
-      }
+      FIndex->updatePreamble(Path, Ctx, std::move(PP));
 
-      void onMainAST(PathRef Path, ParsedAST &AST) override {
-        This->MainFileIdx.update(Path, &AST.getASTContext(),
-                                 AST.getPreprocessorPtr(),
-                                 AST.getLocalTopLevelDecls());
-      }
-    };
-    return llvm::make_unique<CB>(this);
-  };
-
-private:
-  // Contains information from each file's preamble only.
-  // These are large, but update fairly infrequently (preambles are stable).
-  // Missing information:
-  //  - symbol refs (these are always "from the main file")
-  //  - definition locations in the main file
-  //
-  // FIXME: Because the preambles for different TUs have large overlap and
-  // FileIndex doesn't deduplicate, this uses lots of extra RAM.
-  // The biggest obstacle in fixing this: the obvious approach of partitioning
-  // by declaring file (rather than main file) fails if headers provide
-  // different symbols based on preprocessor state.
-  FileIndex PreambleIdx;
-  // Contains information from each file's main AST.
-  // These are updated frequently (on file change), but are relatively small.
-  // Mostly contains:
-  //  - refs to symbols declared in the preamble and referenced from main
-  //  - symbols declared both in the main file and the preamble
-  // (Note that symbols *only* in the main file are not indexed).
-  FileIndex MainFileIdx;
-  std::unique_ptr<SymbolIndex> MergedIndex;
-};
+      FIndex->updateMain(Path, AST, AST.getLocalTopLevelDecls());
+  return llvm::make_unique<CB>(FIndex);
+}
 
 ClangdServer::Options ClangdServer::optsForTest() {
   ClangdServer::Options Opts;
   Opts.UpdateDebounce = std::chrono::steady_clock::duration::zero(); // Faster!
   Opts.StorePreamblesInMemory = true;
   Opts.AsyncThreadsCount = 4; // Consistent!
   return Opts;
 }
 
 ClangdServer::ClangdServer(GlobalCompilationDatabase &CDB,
                            FileSystemProvider &FSProvider,
                            DiagnosticsConsumer &DiagConsumer,
                            const Options &Opts)
     : CDB(CDB), DiagConsumer(DiagConsumer), FSProvider(FSProvider),
       ResourceDir(Opts.ResourceDir ? Opts.ResourceDir->str()
                                    : getStandardResourceDir()),
-      DynamicIdx(Opts.BuildDynamicSymbolIndex
-                     ? new DynamicIndex(Opts.URISchemes)
+      DynamicIdx(Opts.BuildDynamicSymbolIndex ? new FileIndex(Opts.URISchemes)
                                               : nullptr),
       PCHs(std::make_shared<PCHContainerOperations>()),
       // Pass a callback into `WorkScheduler` to extract symbols from a newly
       // parsed file and rebuild the file index synchronously each time an AST
       // is parsed.
       // FIXME(ioeric): this can be slow and we may be able to index on less
       // critical paths.
       WorkScheduler(Opts.AsyncThreadsCount, Opts.StorePreamblesInMemory,
-                    DynamicIdx ? DynamicIdx->makeUpdateCallbacks() : nullptr,
+                    DynamicIdx ? makeUpdateCallbacks(DynamicIdx.get())
+                               : nullptr,
                     Opts.UpdateDebounce, Opts.RetentionPolicy) {
   if (DynamicIdx && Opts.StaticIndex) {
     MergedIndex = mergeIndex(&DynamicIdx->index(), Opts.StaticIndex);
     Index = MergedIndex.get();
   } else if (DynamicIdx)
     Index = &DynamicIdx->index();
   else if (Opts.StaticIndex)
     Index = Opts.StaticIndex;
   else
     Index = nullptr;
 }
 
-// Destructor has to be in .cpp file to see the definition of
-// ClangdServer::DynamicIndex.
-ClangdServer::~ClangdServer() = default;
-
 const SymbolIndex *ClangdServer::dynamicIndex() const {
   return DynamicIdx ? &DynamicIdx->index() : nullptr;
 }
 
 void ClangdServer::setRootPath(PathRef RootPath) {
   auto FS = FSProvider.getFileSystem();
   auto Status = FS->status(RootPath);
   if (!Status)

clang-tools-extra/trunk/clangd/index/FileIndex.h

 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_FILEINDEX_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_FILEINDEX_H
 
 #include "ClangdUnit.h"
 #include "Index.h"
 #include "MemIndex.h"
 #include "clang/Lex/Preprocessor.h"
+#include <memory>
 
 namespace clang {
 namespace clangd {
 
 /// A container of Symbols from several source files. It can be updated
 /// at source-file granularity, replacing all symbols from one file with a new
 /// set.
 ///
private:
 
   /// Stores the latest symbol snapshots for all active files.
   llvm::StringMap<std::shared_ptr<SymbolSlab>> FileToSymbols;
   /// Stores the latest ref snapshots for all active files.
   llvm::StringMap<std::shared_ptr<RefSlab>> FileToRefs;
 };
 
 /// This manages symbols from files and an in-memory index on all symbols.
-class FileIndex : public SwapIndex {
+/// FIXME: Expose an interface to remove files that are closed.
+class FileIndex {
 public:
   /// If URISchemes is empty, the default schemes in SymbolCollector will be
   /// used.
   FileIndex(std::vector<std::string> URISchemes = {});
 
-  /// Update symbols in \p Path with symbols in \p AST. If \p AST is
-  /// nullptr, this removes all symbols in the file.
-  /// If \p AST is not null, \p PP cannot be null and it should be the
-  /// preprocessor that was used to build \p AST.
-  /// If \p TopLevelDecls is set, only these decls are indexed. Otherwise, all
-  /// top level decls obtained from \p AST are indexed.
-  void
-  update(PathRef Path, ASTContext *AST, std::shared_ptr<Preprocessor> PP,
-         llvm::Optional<llvm::ArrayRef<Decl *>> TopLevelDecls = llvm::None);
+  // Presents a merged view of the supplied main-file and preamble ASTs.
+  const SymbolIndex &index() const { return *MergedIndex; }
 
-private:
-  // Only update() should swap the index.
-  using SwapIndex::reset;
+  /// Update preamble symbols of file \p Path with all declarations in \p AST
+  /// and macros in \p PP.
+  void updatePreamble(PathRef Path, ASTContext &AST,
+                      std::shared_ptr<Preprocessor> PP);
+
+  /// Update symbols from main file \p Path with symbols in \p TopLevelDecls.
+  void updateMain(PathRef Path, ParsedAST &AST,
+                  llvm::ArrayRef<Decl *> TopLevelDecls);
 
-  FileSymbols FSymbols;
+private:
   std::vector<std::string> URISchemes;
+
+  // Contains information from each file's preamble only.
+  // These are large, but update fairly infrequently (preambles are stable).
+  // Missing information:
+  //  - symbol refs (these are always "from the main file")
+  //  - definition locations in the main file
+  //
+  // FIXME: Because the preambles for different TUs have large overlap and
+  // FileIndex doesn't deduplicate, this uses lots of extra RAM.
+  // The biggest obstacle in fixing this: the obvious approach of partitioning
+  // by declaring file (rather than main file) fails if headers provide
+  // different symbols based on preprocessor state.
+  FileSymbols PreambleSymbols;
+  SwapIndex PreambleIndex;
+
+  // Contains information from each file's main AST.
+  // These are updated frequently (on file change), but are relatively small.
+  // Mostly contains:
+  //  - refs to symbols declared in the preamble and referenced from main
+  //  - symbols declared both in the main file and the preamble
+  // (Note that symbols *only* in the main file are not indexed).
+  FileSymbols MainFileSymbols;
+  SwapIndex MainFileIndex;
+
+  std::unique_ptr<SymbolIndex> MergedIndex;  // Merge preamble and main index.
 };
 
-/// Retrieves symbols and refs in \p AST.
+/// Retrieves symbols and refs of \p Decls in \p AST.
 /// Exposed to assist in unit tests.
 /// If URISchemes is empty, the default schemes in SymbolCollector will be used.
-/// If \p TopLevelDecls is set, only these decls are indexed. Otherwise, all top
-/// level decls obtained from \p AST are indexed.
 std::pair<SymbolSlab, RefSlab>
-indexAST(ASTContext &AST, std::shared_ptr<Preprocessor> PP,
-         llvm::Optional<llvm::ArrayRef<Decl *>> TopLevelDecls = llvm::None,
+indexMainDecls(ParsedAST &AST, llvm::ArrayRef<Decl *> Decls,
+               llvm::ArrayRef<std::string> URISchemes = {});
+
+/// Idex declarations from \p AST and macros from \p PP that are declared in
+/// included headers.
+/// If URISchemes is empty, the default schemes in SymbolCollector will be used.
+SymbolSlab indexHeaderSymbols(ASTContext &AST, std::shared_ptr<Preprocessor> PP,
                               llvm::ArrayRef<std::string> URISchemes = {});
 
 } // namespace clangd
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_FILEINDEX_H

clang-tools-extra/trunk/clangd/index/FileIndex.cpp

 //===--- FileIndex.cpp - Indexes for files. ------------------------ C++-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "FileIndex.h"
+#include "ClangdUnit.h"
 #include "Logger.h"
 #include "SymbolCollector.h"
+#include "index/Index.h"
+#include "index/Merge.h"
 #include "clang/Index/IndexingAction.h"
 #include "clang/Lex/Preprocessor.h"
+#include <memory>
 
 namespace clang {
 namespace clangd {
 
-std::pair<SymbolSlab, RefSlab>
-indexAST(ASTContext &AST, std::shared_ptr<Preprocessor> PP,
-         llvm::Optional<llvm::ArrayRef<Decl *>> TopLevelDecls,
+static std::pair<SymbolSlab, RefSlab>
+indexSymbols(ASTContext &AST, std::shared_ptr<Preprocessor> PP,
+             llvm::ArrayRef<Decl *> DeclsToIndex, bool IsIndexMainAST,
              llvm::ArrayRef<std::string> URISchemes) {
   SymbolCollector::Options CollectorOpts;
   // FIXME(ioeric): we might also want to collect include headers. We would need
   // to make sure all includes are canonicalized (with CanonicalIncludes), which
   // is not trivial given the current way of collecting symbols: we only have
   // AST at this point, but we also need preprocessor callbacks (e.g.
   // CommentHandler for IWYU pragma) to canonicalize includes.
   CollectorOpts.CollectIncludePath = false;
   CollectorOpts.CountReferences = false;
+  CollectorOpts.Origin = SymbolOrigin::Dynamic;
   if (!URISchemes.empty())
     CollectorOpts.URISchemes = URISchemes;
-  CollectorOpts.Origin = SymbolOrigin::Dynamic;
 
   index::IndexingOptions IndexOpts;
   // We only need declarations, because we don't count references.
   IndexOpts.SystemSymbolFilter =
       index::IndexingOptions::SystemSymbolFilterKind::DeclarationsOnly;
   IndexOpts.IndexFunctionLocals = false;
 
-  std::vector<Decl *> DeclsToIndex;
-  if (TopLevelDecls)
-    DeclsToIndex.assign(TopLevelDecls->begin(), TopLevelDecls->end());
-  else
-    DeclsToIndex.assign(AST.getTranslationUnitDecl()->decls().begin(),
-                        AST.getTranslationUnitDecl()->decls().end());
-
-  // We only collect refs when indexing main AST.
-  // FIXME: this is a hacky way to detect whether we are indexing preamble AST
-  // or main AST, we should make it explicitly.
-  bool IsIndexMainAST = TopLevelDecls.hasValue();
   if (IsIndexMainAST)
+    // We only collect refs when indexing main AST.
     CollectorOpts.RefFilter = RefKind::All;
 
   SymbolCollector Collector(std::move(CollectorOpts));
   Collector.setPreprocessor(PP);
   index::indexTopLevelDecls(AST, *PP, DeclsToIndex, Collector, IndexOpts);
 
   const auto &SM = AST.getSourceManager();
   const auto *MainFileEntry = SM.getFileEntryForID(SM.getMainFileID());
   std::string FileName = MainFileEntry ? MainFileEntry->getName() : "";
 
   auto Syms = Collector.takeSymbols();
   auto Refs = Collector.takeRefs();
-  vlog("index {0}AST for {1}: \n"
+  vlog("index AST for {0} (main={1}): \n"
        "  symbol slab: {2} symbols, {3} bytes\n"
        "  ref slab: {4} symbols, {5} bytes",
-       IsIndexMainAST ? "Main" : "Preamble", FileName, Syms.size(),
-       Syms.bytes(), Refs.size(), Refs.bytes());
+       FileName, IsIndexMainAST, Syms.size(), Syms.bytes(), Refs.size(),
+       Refs.bytes());
   return {std::move(Syms), std::move(Refs)};
 }
 
-FileIndex::FileIndex(std::vector<std::string> URISchemes)
-    : URISchemes(std::move(URISchemes)) {
-  reset(FSymbols.buildMemIndex());
+std::pair<SymbolSlab, RefSlab>
+indexMainDecls(ParsedAST &AST, llvm::ArrayRef<Decl *> TopLevelDecls,
+               llvm::ArrayRef<std::string> URISchemes) {
+  return indexSymbols(AST.getASTContext(), AST.getPreprocessorPtr(),
+                      TopLevelDecls,
+                      /*IsIndexMainAST=*/true, URISchemes);
+}
+
+SymbolSlab indexHeaderSymbols(ASTContext &AST, std::shared_ptr<Preprocessor> PP,
+                              llvm::ArrayRef<std::string> URISchemes) {
+  std::vector<Decl *> DeclsToIndex(
+      AST.getTranslationUnitDecl()->decls().begin(),
+      AST.getTranslationUnitDecl()->decls().end());
+  return indexSymbols(AST, std::move(PP), DeclsToIndex,
+                      /*IsIndexMainAST=*/false, URISchemes)
+      .first;
 }
 
 void FileSymbols::update(PathRef Path, std::unique_ptr<SymbolSlab> Symbols,
                          std::unique_ptr<RefSlab> Refs) {
   std::lock_guard<std::mutex> Lock(Mutex);
   if (!Symbols)
     FileToSymbols.erase(Path);
   else
std::unique_ptr<SymbolIndex> FileSymbols::buildMemIndex() {
   // Index must keep the slabs and contiguous ranges alive.
   return llvm::make_unique<MemIndex>(
       llvm::make_pointee_range(AllSymbols), std::move(AllRefs),
       std::make_tuple(std::move(SymbolSlabs), std::move(RefSlabs),
                       std::move(RefsStorage)),
       StorageSize);
 }
 
-void FileIndex::update(PathRef Path, ASTContext *AST,
-                       std::shared_ptr<Preprocessor> PP,
-                       llvm::Optional<llvm::ArrayRef<Decl *>> TopLevelDecls) {
-  if (!AST) {
-    FSymbols.update(Path, nullptr, nullptr);
-  } else {
-    assert(PP);
-    auto Contents = indexAST(*AST, PP, TopLevelDecls, URISchemes);
-    FSymbols.update(Path,
-                    llvm::make_unique<SymbolSlab>(std::move(Contents.first)),
-                    llvm::make_unique<RefSlab>(std::move(Contents.second)));
-  }
-  reset(FSymbols.buildMemIndex());
+FileIndex::FileIndex(std::vector<std::string> URISchemes)
+    : URISchemes(std::move(URISchemes)),
+      PreambleIndex(PreambleSymbols.buildMemIndex()),
+      MainFileIndex(MainFileSymbols.buildMemIndex()),
+      MergedIndex(mergeIndex(&MainFileIndex, &PreambleIndex)) {}
+
+void FileIndex::updatePreamble(PathRef Path, ASTContext &AST,
+                               std::shared_ptr<Preprocessor> PP) {
+  auto Symbols = indexHeaderSymbols(AST, std::move(PP), URISchemes);
+  PreambleSymbols.update(Path,
+                         llvm::make_unique<SymbolSlab>(std::move(Symbols)),
+                         llvm::make_unique<RefSlab>());
+  PreambleIndex.reset(PreambleSymbols.buildMemIndex());
+}
+
+void FileIndex::updateMain(PathRef Path, ParsedAST &AST,
+                           llvm::ArrayRef<Decl *> TopLevelDecls) {
+  auto Contents = indexMainDecls(AST, TopLevelDecls, URISchemes);
+  MainFileSymbols.update(
+      Path, llvm::make_unique<SymbolSlab>(std::move(Contents.first)),
+  MainFileIndex.reset(MainFileSymbols.buildMemIndex());
 }
 
 } // namespace clangd
 } // namespace clang

clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp

TEST(FileSymbolsTest, SnapshotAliveAfterRemove) {
   auto Empty = FS.buildMemIndex();
   EXPECT_THAT(getSymbolNames(*Empty), UnorderedElementsAre());
   EXPECT_THAT(getRefs(*Empty, ID), ElementsAre());
 
   EXPECT_THAT(getSymbolNames(*Symbols), UnorderedElementsAre("1", "2", "3"));
   EXPECT_THAT(getRefs(*Symbols, ID), RefsAre({FileURI("f1.cc")}));
 }
 
-std::vector<std::string> match(const SymbolIndex &I,
+std::vector<std::string> match(const FileIndex &I,
                                const FuzzyFindRequest &Req) {
   std::vector<std::string> Matches;
-  I.fuzzyFind(Req, [&](const Symbol &Sym) {
+  I.index().fuzzyFind(Req, [&](const Symbol &Sym) {
     Matches.push_back((Sym.Scope + Sym.Name).str());
   });
   return Matches;
 }
 
 // Adds Basename.cpp, which includes Basename.h, which contains Code.
 void update(FileIndex &M, llvm::StringRef Basename, llvm::StringRef Code) {
   TestTU File;
   File.Filename = (Basename + ".cpp").str();
   File.HeaderFilename = (Basename + ".h").str();
   File.HeaderCode = Code;
   auto AST = File.build();
-  M.update(File.Filename, &AST.getASTContext(), AST.getPreprocessorPtr());
+  M.updatePreamble(File.Filename, AST.getASTContext(),
+                   AST.getPreprocessorPtr());
 }
 
 TEST(FileIndexTest, CustomizedURIScheme) {
   FileIndex M({"unittest"});
   update(M, "f", "class string {};");
 
   FuzzyFindRequest Req;
   Req.Query = "";
   bool SeenSymbol = false;
-  M.fuzzyFind(Req, [&](const Symbol &Sym) {
+  M.index().fuzzyFind(Req, [&](const Symbol &Sym) {
     EXPECT_EQ(Sym.CanonicalDeclaration.FileURI, "unittest:///f.h");
     SeenSymbol = true;
   });
   EXPECT_TRUE(SeenSymbol);
 }
 
 TEST(FileIndexTest, IndexAST) {
   FileIndex M;
TEST(FileIndexTest, IndexMultiASTAndDeduplicate) {
   update(M, "f2", "namespace ns { void ff() {} class X {}; }");
 
   FuzzyFindRequest Req;
   Req.Query = "";
   Req.Scopes = {"ns::"};
   EXPECT_THAT(match(M, Req), UnorderedElementsAre("ns::f", "ns::X", "ns::ff"));
 }
 
-TEST(FileIndexTest, RemoveAST) {
-  FileIndex M;
-  update(M, "f1", "namespace ns { void f() {} class X {}; }");
-
-  FuzzyFindRequest Req;
-  Req.Query = "";
-  Req.Scopes = {"ns::"};
-  EXPECT_THAT(match(M, Req), UnorderedElementsAre("ns::f", "ns::X"));
-
-  M.update("f1.cpp", nullptr, nullptr);
-  EXPECT_THAT(match(M, Req), UnorderedElementsAre());
-}
-
-TEST(FileIndexTest, RemoveNonExisting) {
-  FileIndex M;
-  M.update("no.cpp", nullptr, nullptr);
-  EXPECT_THAT(match(M, FuzzyFindRequest()), UnorderedElementsAre());
-}
-
 TEST(FileIndexTest, ClassMembers) {
   FileIndex M;
   update(M, "f1", "class X { static int m1; int m2; static void f(); };");
 
   FuzzyFindRequest Req;
   Req.Query = "";
   EXPECT_THAT(match(M, Req),
               UnorderedElementsAre("X", "X::m1", "X::m2", "X::f"));
 }
 
 TEST(FileIndexTest, NoIncludeCollected) {
   FileIndex M;
   update(M, "f", "class string {};");
 
   FuzzyFindRequest Req;
   Req.Query = "";
   bool SeenSymbol = false;
-  M.fuzzyFind(Req, [&](const Symbol &Sym) {
+  M.index().fuzzyFind(Req, [&](const Symbol &Sym) {
     EXPECT_TRUE(Sym.IncludeHeaders.empty());
     SeenSymbol = true;
   });
   EXPECT_TRUE(SeenSymbol);
 }
 
 TEST(FileIndexTest, TemplateParamsInLabel) {
   auto Source = R"cpp(
 template <class Ty>
 class vector {
 };
 
 template <class Ty, class Arg>
 vector<Ty> make_vector(Arg A) {}
 )cpp";
 
   FileIndex M;
   update(M, "f", Source);
 
   FuzzyFindRequest Req;
   Req.Query = "";
   bool SeenVector = false;
   bool SeenMakeVector = false;
-  M.fuzzyFind(Req, [&](const Symbol &Sym) {
+  M.index().fuzzyFind(Req, [&](const Symbol &Sym) {
     if (Sym.Name == "vector") {
       EXPECT_EQ(Sym.Signature, "<class Ty>");
       EXPECT_EQ(Sym.CompletionSnippetSuffix, "<${1:class Ty}>");
       SeenVector = true;
       return;
     }
 
     if (Sym.Name == "make_vector") {
TEST(FileIndexTest, RebuildWithPreamble) {
   FileIndex Index;
   bool IndexUpdated = false;
   buildPreamble(
       FooCpp, *CI, /*OldPreamble=*/nullptr, tooling::CompileCommand(), PI,
       std::make_shared<PCHContainerOperations>(), /*StoreInMemory=*/true,
       [&](ASTContext &Ctx, std::shared_ptr<Preprocessor> PP) {
         EXPECT_FALSE(IndexUpdated) << "Expected only a single index update";
         IndexUpdated = true;
-        Index.update(FooCpp, &Ctx, std::move(PP));
+        Index.updatePreamble(FooCpp, Ctx, std::move(PP));
       });
   ASSERT_TRUE(IndexUpdated);
 
   // Check the index contains symbols from the preamble, but not from the main
   // file.
   FuzzyFindRequest Req;
   Req.Query = "";
   Req.Scopes = {"", "ns_in_header::"};
TEST(FileIndexTest, Refs) {
 
   FileIndex Index(/*URISchemes*/ {"unittest"});
   // Add test.cc
   TestTU Test;
   Test.HeaderCode = HeaderCode;
   Test.Code = MainCode.code();
   Test.Filename = "test.cc";
   auto AST = Test.build();
-  Index.update(Test.Filename, &AST.getASTContext(), AST.getPreprocessorPtr(),
-               AST.getLocalTopLevelDecls());
+  Index.updateMain(Test.Filename, AST, AST.getLocalTopLevelDecls());
   // Add test2.cc
   TestTU Test2;
   Test2.HeaderCode = HeaderCode;
   Test2.Code = MainCode.code();
   Test2.Filename = "test2.cc";
   AST = Test2.build();
-  Index.update(Test2.Filename, &AST.getASTContext(), AST.getPreprocessorPtr(),
-               AST.getLocalTopLevelDecls());
+  Index.updateMain(Test2.Filename, AST, AST.getLocalTopLevelDecls());
 
-  EXPECT_THAT(getRefs(Index, Foo.ID),
+  EXPECT_THAT(getRefs(Index.index(), Foo.ID),
               RefsAre({AllOf(RefRange(MainCode.range("foo")),
                              FileURI("unittest:///test.cc")),
                        AllOf(RefRange(MainCode.range("foo")),
                              FileURI("unittest:///test2.cc"))}));
 }
 
 } // namespace
 } // namespace clangd
 } // namespace clang

clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp

TEST(MergeTest, PreferSymbolWithDefn) {
   EXPECT_EQ(M.CanonicalDeclaration.FileURI, "file:/right.h");
   EXPECT_EQ(M.Definition.FileURI, "file:/right.cpp");
   EXPECT_EQ(M.Name, "right");
 }
 
 TEST(MergeIndexTest, Refs) {
   FileIndex Dyn({"unittest"});
   FileIndex StaticIndex({"unittest"});
-  auto MergedIndex = mergeIndex(&Dyn, &StaticIndex);
+  auto MergedIndex = mergeIndex(&Dyn.index(), &StaticIndex.index());
 
   const char *HeaderCode = "class Foo;";
   auto HeaderSymbols = TestTU::withHeaderCode("class Foo;").headerSymbols();
   auto Foo = findSymbol(HeaderSymbols, "Foo");
 
   // Build dynamic index for test.cc.
   Annotations Test1Code(R"(class $Foo[[Foo]];)");
   TestTU Test;
   Test.HeaderCode = HeaderCode;
   Test.Code = Test1Code.code();
   Test.Filename = "test.cc";
   auto AST = Test.build();
-  Dyn.update(Test.Filename, &AST.getASTContext(), AST.getPreprocessorPtr(),
-             AST.getLocalTopLevelDecls());
+  Dyn.updateMain(Test.Filename, AST, AST.getLocalTopLevelDecls());
 
   // Build static index for test.cc.
   Test.HeaderCode = HeaderCode;
   Test.Code = "// static\nclass Foo {};";
   Test.Filename = "test.cc";
   auto StaticAST = Test.build();
   // Add stale refs for test.cc.
-  StaticIndex.update(Test.Filename, &StaticAST.getASTContext(),
-                     StaticAST.getPreprocessorPtr(),
+  StaticIndex.updateMain(Test.Filename, StaticAST,
                          StaticAST.getLocalTopLevelDecls());
 
   // Add refs for test2.cc
   Annotations Test2Code(R"(class $Foo[[Foo]] {};)");
   TestTU Test2;
   Test2.HeaderCode = HeaderCode;
   Test2.Code = Test2Code.code();
   Test2.Filename = "test2.cc";
   StaticAST = Test2.build();
-  StaticIndex.update(Test2.Filename, &StaticAST.getASTContext(),
-                     StaticAST.getPreprocessorPtr(),
+  StaticIndex.updateMain(Test2.Filename, StaticAST,
                          StaticAST.getLocalTopLevelDecls());
 
   RefsRequest Request;
   Request.IDs = {Foo.ID};
   RefSlab::Builder Results;
   MergedIndex->refs(Request, [&](const Ref &O) { Results.insert(Foo.ID, O); });
 
   EXPECT_THAT(
       std::move(Results).build(),

clang-tools-extra/trunk/unittests/clangd/TestTU.cpp

if (!AST.hasValue()) {
     ADD_FAILURE() << "Failed to build code:\n" << Code;
     llvm_unreachable("Failed to build TestTU!");
   }
   return std::move(*AST);
 }
 
 SymbolSlab TestTU::headerSymbols() const {
   auto AST = build();
-  return indexAST(AST.getASTContext(), AST.getPreprocessorPtr()).first;
+  return indexHeaderSymbols(AST.getASTContext(), AST.getPreprocessorPtr());
 }
 
+// FIXME: This should return a FileIndex with both preamble and main index.
 std::unique_ptr<SymbolIndex> TestTU::index() const {
   auto AST = build();
-  auto Content = indexAST(AST.getASTContext(), AST.getPreprocessorPtr(),
-                          AST.getLocalTopLevelDecls());
+  auto Content = indexMainDecls(AST, AST.getLocalTopLevelDecls());
   return MemIndex::build(std::move(Content.first), std::move(Content.second));
 }
 
 const Symbol &findSymbol(const SymbolSlab &Slab, llvm::StringRef QName) {
   const Symbol *Result = nullptr;
   for (const Symbol &S : Slab) {
     if (QName != (S.Scope + S.Name).str())
       continue;