jvikstrom added inline comments.
================ Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:54 + if (Conflicting.size() > 1) { + Tokens.erase(Tokens.begin() + I, + Tokens.begin() + I + Conflicting.size()); ---------------- ilya-biryukov wrote: > This is potentially `O(n^2)`. Could we instead create a new vector and fill > it with the new items? > > The memory usage should not matter much - we have the AST stored in the > background anyway. I bet we would be looking at it first if we wanted to > minimize memory usage. > > If we really wanted to **not** waste any memory, we could do it > `std::erase_if`-style, i.e. move the items we want to remove to the end of > the vector, call `vector::erase` once at the end. Completely forgot that erase is linear. Will go for the "allocate another vector" approach. Rewrote the entire conflicting checking code to hopefully be simpler as well (it changed substantially, so you might want to take another look at it). ================ Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:332 + #define DEF_CLASS(T) class T {}; + DEF_MULTIPLE(XYZ); + DEF_MULTIPLE(XYZW); ---------------- ilya-biryukov wrote: > Unrelated to the change: do we plan to highlight macro names? > That seems very useful. Yes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64741/new/ https://reviews.llvm.org/D64741 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits