jvikstrom marked 4 inline comments as done.
jvikstrom added inline comments.
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:61
+ }
+ if(isa<FunctionDecl>(D)) {
+ addToken(Loc, HighlightingKind::Function);
----------------
hokein wrote:
> sammccall wrote:
> > jvikstrom wrote:
> > > sammccall wrote:
> > > > note that methods, constructors, and destructors inherit from
> > > > functiondecl, so if you want to exclude/distinguish those, order
> > > > matters here
> > > I'm aware of that, but thanks for the heads up. Although should I add it
> > > in a comment somewhere in the method? Also added an additional testcase
> > > for classes and FIXMEs to the skip if statement in VisitNamedDecl.
> > I don't think it needs a comment, especially if you're not actually
> > highlighting them (because they have weird DeclarationNames)
> >
> > > FIXMEs to the skip if statement in VisitNamedDecl
> > I'm not actually sure there's anything to fix here - it's a bit hard to
> > talk about constructor/destructor highlighting as distinct from type name
> > highlighting in C++. If you want them highlighted as classes, then that
> > should just start working when you start handling TypeLocs.
> I think constructor/destructor can be categorized in the `function` group,
> like `entity.name.function.constructor`, `entity.name.function.destructor`
I'll have a look at constructors/destructors at the same time as I look at types
================
Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:69
+ void $Function[[foo]]() {
+ int $Variable[[b]];
+ auto $Variable[[FN]] = [ $Variable[[b]]](int $Variable[[a]]) -> void {};
----------------
hokein wrote:
> nit: even for the test code, could we make the code style consistent (like
> follow the LLVM code style) here?
I think this should be consistent with LLVM code style.
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
