[PATCH] D83157: [clangd] Extract BackgroundIndex::Options struct. NFC

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 6 12:53:38 PDT 2020


kadircet marked an inline comment as done.
kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/ClangdServer.cpp:184
+        TFS, CDB,
         BackgroundIndexStorage::createDiskBackedStorageFactory(
             [&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); }),
----------------
sammccall wrote:
> kadircet wrote:
> > I think we should move storage factory into options too ?
> The storage factory isn't optional, nor does it have a good default. So it seems mechanically awkward to put it in the struct, and I'm not sure there's much benefit... 
> 
> How would you see this being initialized?
I thought we were exposing `NullStorage` so I was planning for it to be the default for ignorant call sites. But seeing it is not exposed, and this only being used by ClangdServer (and tests), I suppose it is not that important.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83157/new/

https://reviews.llvm.org/D83157





More information about the cfe-commits mailing list