hokein added a comment.
In D67901#1687308 <https://reviews.llvm.org/D67901#1687308>, @nridge wrote:
> I do feel strongly that types and non-types should be highlighted
> differently, so the updated patch adds two new dependent highlightings, one
> for types and one for variables/functions.
I see your point here, no objection.
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:156
+ bool VisitUnresolvedLookupExpr(UnresolvedLookupExpr *E) {
+ if (canHighlightName(E->getName())) {
+ addToken(E->getNameLoc(), HighlightingKind::DependentName);
----------------
nit: remove the `{}` and elsewhere, LLVM prefers not adding `{}` if the body
only contains a single statement.
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:163
+ bool VisitUnresolvedMemberExpr(UnresolvedMemberExpr *E) {
+ if (canHighlightName(E->getMemberName())) {
+ addToken(E->getNameLoc(), HighlightingKind::DependentName);
----------------
I think we should use `E->getName()` since we are highlighting the `NameLoc`
below.
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:219
+ bool VisitDependentNameTypeLoc(DependentNameTypeLoc L) {
+ addToken(L.getNameLoc(), HighlightingKind::DependentType);
+ return true;
----------------
nit: we have `kindForType` for hanlding all types, so I'd move the logic of
detecting the dependent type there.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67901/new/
https://reviews.llvm.org/D67901
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
