sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land.
LGTM with optional mechanical replacement of `Keyword` with `Modifier`. ================ Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:52 Macro, + Keyword, ---------------- LSP provides both `keyword` and `modifier`, which obviously overlap. I think `modifier` is more specific and applies here, so would lean towards that. WDYT? ================ Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:868 + // override and final + R"cpp( + class $Class_def_abstract[[Base]] { virtual void $Method_decl_abstract_virtual[[m]]() = 0; }; ---------------- can you add something like `int override, final;` to check that these are *not* highlighted? ================ Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:872 + class $Class_def[[MoreDerived]] : public $Class[[Derived]] { void $Method_decl_virtual[[m]]() $Keyword[[final]]; }; // Issue 1222: readonly modifier for generic parameter + )cpp", ---------------- this comment should not be inside the string literal Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137943/new/ https://reviews.llvm.org/D137943 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits