sammccall added inline comments.
================ Comment at: clangd/index/Index.h:416 + // until the call returns (even if reset() is called). + bool fuzzyFind(const FuzzyFindRequest &, + llvm::function_ref<void(const Symbol &)>) const override; ---------------- kbobyrev wrote: > Do we want these functions to be `final`? Since `SwapIndex` is a wrapper > around an immutable index structure, I believe it would be unlikely that > anything would derive from it. Maybe unlikely but I don't see a strong reason to enforce one way or the other. (We rarely use final). One example of where we'll do it: FileIndex inherits SwapIndex, but it should override `estimateMemoryUsage` once that's fixed to incorporate backing symbols. ================ Comment at: clangd/index/MemIndex.h:30 + /// Builds an index from a slab. The shared_ptr manages the slab's lifetime. + static std::shared_ptr<SymbolIndex> build(SymbolSlab Slab); ---------------- ioeric wrote: > (It's a bit unfortunate that this has to return `shared_ptr` now) Since some of the resources it owns has a shared lifetime, this is really just reflecting reality I think. Whether that's visible or invisible seems like a wash to me. ================ Comment at: clangd/index/dex/DexIndex.h:42 + // All symbols must outlive this index. + template <typename Range> DexIndex(Range &&Symbols) { + for (auto &&Sym : Symbols) ---------------- ioeric wrote: > Why is this constructor needed? I think this could restrict the flexibility > of DexIndex initialization, in case we need to pass in extra parameters here. I'm not sure exactly what you mean here. We need a way to specify the symbols to be indexed. Because these are now immutable, doing that in the constructor if possible is best. Previously this was a vector<const Symbol*>, but that sometimes required us to construct that big vector, dereference all those pointers, and throw away the vector. This signature is strictly more general (if you have a vector of pointers, you can pass `make_pointee_range<const Symbol>`) > in case we need to pass in extra parameters here. What stops us adding more parameters? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51422 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits