[PATCH] D54269: Introduce shard storage to auto-index.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clangd/index/Background.h:39
+
+  virtual llvm::Expected<IndexFileIn>
+  loadShard(llvm::StringRef ShardIdentifier) const = 0;
----------------
sammccall wrote:
> sammccall wrote:
> > docs
> Hmm, we're going to attempt to load the shard corresponding to every file that we have in the CDB, even if the index isn't built yet. So "file doesn't exist" is an expected case (where we don't want to log etc), vs e.g. "file was corrupted" is unexpected and should definitely be logged. How do we distinguish these with this API?
> 
> One defensible option would be to have implementations handle errors: return Optional<IndexFileIn>, and have the disk version log errors appropriately and return None.
> 
> Another would be Expected<Optional<IndexFileIn>> or so, which is a little messy.
used the former proposition, but with std::unique_ptr instead of llvm::Optional since IndexFileIn is move-only.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54269