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

Reply via email to