This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Mark "override" and "final" as modifiers
ClosedPublic

Authored by ckandeler on Nov 14 2022, 6:18 AM.

Details

Summary

... in semantic highlighting.
These specifiers cannot be identified by simple lexing (since e.g.
variables with these names can legally be declared), which means they
should be semantic tokens.

Diff Detail

Event Timeline

ckandeler created this revision.Nov 14 2022, 6:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 14 2022, 6:18 AM
ckandeler requested review of this revision.Nov 14 2022, 6:18 AM
nridge added a subscriber: nridge.Nov 19 2022, 5:01 PM
sammccall accepted this revision.Nov 21 2022, 2:25 AM

LGTM with optional mechanical replacement of Keyword with Modifier.

clang-tools-extra/clangd/SemanticHighlighting.h
52

LSP provides both keyword and modifier, which obviously overlap.
I think modifier is more specific and applies here, so would lean towards that. WDYT?

clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
868

can you add something like int override, final; to check that these are *not* highlighted?

872

this comment should not be inside the string literal

This revision is now accepted and ready to land.Nov 21 2022, 2:25 AM
ckandeler updated this revision to Diff 476843.Nov 21 2022, 3:29 AM
ckandeler marked 3 inline comments as done.
ckandeler retitled this revision from [clangd] Mark "override" and "final" as keywords to [clangd] Mark "override" and "final" as modifiers.
ckandeler edited the summary of this revision. (Show Details)

Incorporated review comments.

This revision was landed with ongoing or failed builds.Nov 21 2022, 1:01 PM
This revision was automatically updated to reflect the committed changes.