[PATCH] D77811: [clangd] Implement semanticTokens modifiers

2021-02-09 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:136 +// Whether D is const in a loose sense (should it be highlighted as such?) +bool isConst(const Decl *D) { + if (llvm::isa(D) || llvm::isa(D)) nridge wrote: > sammcca

[PATCH] D77811: [clangd] Implement semanticTokens modifiers

2021-02-09 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:136 +// Whether D is const in a loose sense (should it be highlighted as such?) +bool isConst(const Decl *D) { + if (llvm::isa(D) || llvm::isa(D)) sammccall wrote: > nrid

[PATCH] D77811: [clangd] Implement semanticTokens modifiers

2021-02-09 Thread Sam McCall via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGdd8fb21227ce: [clangd] Implement semanticTokens modifiers (authored by sammccall). Changed prior to commit: https://reviews.llvm.org/D77811?vs=320

[PATCH] D77811: [clangd] Implement semanticTokens modifiers

2021-02-09 Thread Sam McCall via Phabricator via cfe-commits
sammccall marked 2 inline comments as done. sammccall added a comment. Thanks for the excellent comments, and sorry for the radio silence. (Kids got quarantined recently so work time has been... scarce). I think there are improvements to be had here but it's probably time to land this... In D7

[PATCH] D77811: [clangd] Implement semanticTokens modifiers

2021-02-05 Thread Nathan Ridge via Phabricator via cfe-commits
nridge accepted this revision. nridge added a comment. This revision is now accepted and ready to land. I think this patch is in pretty good shape, the outstanding comments are either minor or thoughts about future changes. I am curious what you think of the extension idea for readonly

[PATCH] D77811: [clangd] Implement semanticTokens modifiers

2021-02-05 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:476 + if (!Kind || (Result && Kind != Result)) +continue; + Result = Kind; nridge wrote: > This is a change of behaviour from before, in the case where t

[PATCH] D77811: [clangd] Implement semanticTokens modifiers

2021-02-02 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments. Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:264 +$Class[[D]] $Field_decl[[E]]; +static double $StaticField_decl_static[[S]]; +static void $StaticMethod_decl_static[[bar]]() {}

[PATCH] D77811: [clangd] Implement semanticTokens modifiers

2021-02-02 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. In D77811#2533815 , @sammccall wrote: > Introducing nonstandard kinds is **backwards-incompatible**. If the client > doesn't understand primitiveType, then the token kind is now completely > unknown. This could be a regression fro

[PATCH] D77811: [clangd] Implement semanticTokens modifiers

2021-02-01 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In D77811#2533237 , @nridge wrote: > Thanks a lot for working on this! I agree this is well aligned with the goals > of D77702 (and in fact goes even further > and meets the goals of D66990

[PATCH] D77811: [clangd] Implement semanticTokens modifiers

2021-02-01 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 320464. sammccall marked an inline comment as done. sammccall added a comment. add missing test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77811/new/ https://reviews.llvm.org/D77811 Files: clang-tools-e

[PATCH] D77811: [clangd] Implement semanticTokens modifiers

2021-01-31 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Thanks a lot for working on this! I agree this is well aligned with the goals of D77702 (and in fact goes even further and meets the goals of D66990 via the `declaration` modifier). The other modifiers a

[PATCH] D77811: [clangd] Implement semanticTokens modifiers

2021-01-29 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 320218. sammccall added a comment. Add tests for 'abstract' and fix bugs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77811/new/ https://reviews.llvm.org/D77811 Files: clang-tools-extra/clangd/ClangdLSPSe

[PATCH] D77811: [clangd] Implement semanticTokens modifiers

2021-01-27 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 319594. sammccall added a comment. Tidy up, explain static Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77811/new/ https://reviews.llvm.org/D77811 Files: clang-tools-extra/clangd/ClangdLSPServer.cpp cla

[PATCH] D77811: [clangd] Implement semanticTokens modifiers

2021-01-27 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 319592. sammccall retitled this revision from "[clangd] WIP playing with semantic highlighting modifiers" to "[clangd] Implement semanticTokens modifiers". sammccall edited the summary of this revision. sammccall added a comment. Update phab with commit mes