sammccall marked 5 inline comments as done. sammccall added inline comments.
================ Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:269 + + ++this->Version; + if (Version) ---------------- kadircet wrote: > Bumping the version in `update` and just exposing it in `buildIndex` feels > more natural, WDYT? > > Might even enable us to get rid of some redundant resets (not that it is > expensive ..), imagine a sequencing like: > > - update1 > - update2 > - buildIndex1 > - buildIndex2 > > even though both build the same index, they got different version numbers > hence both will reset the current index. Yeah, that makes sense. ================ Comment at: clang-tools-extra/clangd/index/Symbol.h:190 + using size_type = size_t; + size_type size() const { return Symbols.size(); } bool empty() const { return Symbols.empty(); } ---------------- kadircet wrote: > i suppose this is for `testing::SizeIs` right? Right, gtest is weird. Could live without it, seems harmless though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82891/new/ https://reviews.llvm.org/D82891 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits