[PATCH] D63919: [clangd] Emit publishSemanticHighlighting in LSP if enabled

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom added inline comments.


================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:144
+      write32be(Token.R.start.character, OS);
+      write16be(Token.R.end.character - Token.R.start.character, OS);
+      write16be(static_cast<int>(Token.Kind), OS);
----------------
hokein wrote:
> if the token is across multiple line, we will emit an ill-formed results.
There's a FIXME above (which is where it should probably be handled). A bit unsure how to solve though. If a token is a block comment spanning multiple lines we would need to know the length of every line in the block comment. Probably something that can be solved with the ASTContext  or SourceManager but that can't be accessed in this function.


================
Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:104
+      HighlightingToken{HighlightingKind::Variable,
+                        Range{CreatePosition(3, 8), CreatePosition(3, 12)}},
+      HighlightingToken{HighlightingKind::Function,
----------------
hokein wrote:
> `Range{ /*start*/{3, 8}, /*end*/{3, 12} }` should be compilable.
Doesn't compile because of the default initialization of `line` and `character` in Position.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63919/new/

https://reviews.llvm.org/D63919