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
  • [PATCH] D137943: [clang... Christian Kandeler via Phabricator via cfe-commits
    • [PATCH] D137943: [... Sam McCall via Phabricator via cfe-commits

Reply via email to