HighCommander4 wrote:

> > Clangd does not send semantic tokens for any other preprocessor directives 
> > (`#define`, `#ifdef`, `#include`...), leaving those to client-side 
> > highlighting. I think it shouldn't be sending semantic tokens for `#embed` 
> > either.
> 
> I can definitely understand not doing just `embed` by itself, but is there a 
> reason we want to leave the preprocessor to the client side? [...] Is there a 
> reason we wouldn't want to handle all preprocessor directives at once?

Good question. Previous discussion on this topic can be found at 
https://github.com/clangd/clangd/issues/1115, but to summarize:

 * The Language Server Protocol distinguishes between "syntax tokens" (basic 
lexical grammar like literals and most keywords) and "semantic tokens" (tokens 
that require a full semantic model to resolve like identifiers and 
context-sensitive keywords).
 * The initial intention was for the `semanticTokens` request to only provide 
semantic tokens, with the syntax tokens computed on the client side.
 * In LSP 3.17, a parameter `augmentsSyntaxTokens` to the `semanticTokens` 
request was introduced, which allows a client to ask the server to send 
semantic **and** syntax tokens in response. **Clangd does not currently 
implement this.**

So, it could make sense for clangd to send tokens for all preprocessor 
directives but only (1) gated on `augmentsSyntaxTokens=false`, and (2) 
hopefully as part of a larger change to also send syntax tokens for other 
lexical syntax like literals and all keywords.

> I can imagine that gets awkward for things like `module` which may or may not 
> be handled by the preprocessor.

`module` is not a preprocessor directive but a context-sensitive keyword, 
making it a good fit for semantic (rather than syntax) tokens.

We do send semantic tokens already for other context-sensitive keywords 
(`override` and `final` 
[here](https://searchfox.org/llvm/rev/08c7b4203815b3502c04ecb9cd8b6dae21fc5661/clang-tools-extra/clangd/SemanticHighlighting.cpp#977),
 `import` 
[here](https://searchfox.org/llvm/rev/08c7b4203815b3502c04ecb9cd8b6dae21fc5661/clang-tools-extra/clangd/SemanticHighlighting.cpp#586)).
 I would accept a patch which adds one for `module`.

https://github.com/llvm/llvm-project/pull/212036
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to