sammccall added inline comments.
================ Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:46 + + bool VisitDeclRefExpr(DeclRefExpr *Ref) { + if (Ref->getNameInfo().getName().getNameKind() == ---------------- hokein wrote: > The `DeclRefExpr` is a very general expression, which can reference a > variable, a function, an enum, etc. I think we want to distinguish with them > (rather than putting all into `Variable` type). > > And we are missing large majority of entities now, I think we could start > collecting more entities (class, method, enum, etc). IMO the way to ensure consistency is to make the highlight a function of the decl, regardless of whether you found it in the decl itself, a declrefexpr, etc. so something like: ``` // (overloaded) addToken(SourceLocation Loc, NamedDecl *D) { if (isa<VarDecl>(D)) return addToken(Loc, HighlightingKind::Variable); // ... etc } bool VisitDeclRefExpr(DeclRefExpr *Ref) { // bail out for operators here (if it's only *usages* you don't want to highlight) addToken(Ref->getLocation(), Ref->getDecl()); } bool VisitNamedDecl(NamedDecl *ND) { addToken(ND->getLocation(), ND); } ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64199/new/ https://reviews.llvm.org/D64199 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits