nridge added inline comments.
================ Comment at: clang-tools-extra/clangd/index/Serialization.cpp:458 // data. Later we may want to support some backward compatibility. -constexpr static uint32_t Version = 17; +constexpr static uint32_t Version = 18; ---------------- I don't think indexing new symbols is a **breaking** change to the index format, in the sense that an older index cannot be read by a newer clangd. As such, I don't think it's necessary to increment this version. ================ Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:2966 {cls("nx::Clangd1"), cls("ny::Clangd2"), cls("Clangd3"), - cls("na::nb::Clangd4")}, + cls("na::nb::Clangd4"), enmConstant("na::C::Clangd5")}, Opts); ---------------- Hmm, I don't think this type of test actually exercises the `isIndexedForCodeCompletion()` codepath (since we're just mocking out the index contents, instead of running the actual indexer). Could we use [this type](https://searchfox.org/llvm/source/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp#3341-3359) instead? (Sorry, this is partly my fault for not looking more carefully at what sorts of tests are in CodeCompleteTests.cpp before my earlier comment.) ================ Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:1333 AllOf(qName("Color2::Yellow"), forCodeCompletion(false)), + AllOf(qName("Color3"), forCodeCompletion(true)), + AllOf(qName("Color3::Blue"), forCodeCompletion(true)), ---------------- nit: place after ns / ns::Black to match the order in the code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136925/new/ https://reviews.llvm.org/D136925 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits