[PATCH] D108320: Add semantic token modifier for non-const reference parameter

nridge added a comment. Apologies, forgot about this. Commited now :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108320/new/ https://reviews.llvm.org/D108320 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

This revision was automatically updated to reflect the committed changes. Closed by commit rG5a85f9b1d48c: Add semantic token modifier for non-const reference parameter (authored by tom-anders, committed by nridge). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

tom-anders added a comment. I’d say commit as is and have a look at the SmallVector -> unit32_t thing together with the std::map issue afterwards Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108320/new/ https://reviews.llvm.org/D108320 _

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

nridge added a comment. I'm happy to commit it. Did you want to make the `SmallVector<1>` --> `uint32_t` change, or should I just commit it as is? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108320/new/ https://reviews.llvm.org/D108320

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

tom-anders added a comment. Yeah, I‘d need someone to commit this for me. About the std::map thing: let’s proceed as suggested and have @sammccall take look at it himself after :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108320/new/ https://

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Thanks! This looks good to me to land as is. I'd like to see the std::map eliminated, but if you don't want to do this I'm happy to take a stab at it after. Want someone to commit this f

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

nridge added a comment. Thanks for the updates! LGTM from my side. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:555 + // Iterate over the types of the function parameters. + // If any of them are non-const reference paramteres, add it as a + // h

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

tom-anders updated this revision to Diff 369249. tom-anders marked an inline comment as done. tom-anders added a comment. - Use llvm::SmallVector, add more FIXME, remove old commented out test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108320/new

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

tom-anders marked 5 inline comments as done. tom-anders added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:470 std::vector Tokens; + std::map> ExtraModifiers; const HeuristicResolver *Resolver; nridge wrote: > Looking at e

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

nridge added a comment. Thanks for the update! Looks pretty good, just a few minor comments from me. Will leave approval to Sam. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:470 std::vector Tokens; + std::map> ExtraModifiers; const HeuristicResolver *Re

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

tom-anders added a comment. @sammccall @nridge Thanks for the positive feedback, let me know if I missed any of your suggestions! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108320/new/ https://reviews.llvm.org/D108320 _

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

tom-anders updated this revision to Diff 368181. tom-anders added a comment. - clang-format once again Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108320/new/ https://reviews.llvm.org/D108320 Files: clang-tools-extra/clangd/SemanticHighlightin

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

tom-anders updated this revision to Diff 368179. tom-anders added a comment. - Remove accidentally added empty line Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108320/new/ https://reviews.llvm.org/D108320 Files: clang-tools-extra/clangd/Semant

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

tom-anders updated this revision to Diff 368177. tom-anders marked 3 inline comments as done. tom-anders added a comment. - Apply suggested renaming and fix nits - Use a map for extra modifiers instead of abusing conflict resolution - Use ArrayRef instead of callback - Add FIXME for handling CXXOp

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

tom-anders marked 12 inline comments as done. tom-anders added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:314 // (these tend to be vague, like Type or Unknown) +// - Resolved tokens (i.e. without the "dependent-name" modifier) with kind +//

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

nridge added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:538 +for (size_t I = 0; I < FD->getNumParams(); ++I) { + if (const auto *Param = FD->getParamDecl(I)) { +auto T = Param->getType(); tom-anders wrote: > sa

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

tom-anders added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:538 +for (size_t I = 0; I < FD->getNumParams(); ++I) { + if (const auto *Param = FD->getParamDecl(I)) { +auto T = Param->getType(); sammccall wrote: >

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

sammccall added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:547 +if (isa(Arg)) { + Location = Arg->getBeginLoc(); +} else if (auto *M = dyn_cast(Arg)) { tom-anders wrote: > sammccall wrote: >

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

tom-anders planned changes to this revision. tom-anders added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:547 +if (isa(Arg)) { + Location = Arg->getBeginLoc(); +} else if (auto *M = dyn_cast(Arg)) { -

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

nridge added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:314 // (these tend to be vague, like Type or Unknown) +// - Resolved tokens (i.e. without the "dependent-name" modifier) with kind +// "Unknown" are less reliable than resolved tokens

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

sammccall added a comment. Agree this is nice, well done! A few more notes for consideration... Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:314 // (these tend to be vague, like Type or Unknown) +// - Resolved tokens (i.e. without the "dependent-name" modifi

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

nridge added a comment. Thanks -- this patch is looking great so far! Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:314 // (these tend to be vague, like Type or Unknown) +// - Resolved tokens (i.e. without the "dependent-name" modifier) with kind +// "Unkno

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

tom-anders created this revision. Herald added subscribers: usaxena95, kadircet, arphaman. tom-anders requested review of this revision. Herald added subscribers: cfe-commits, ilya-biryukov. Herald added a project: clang-tools-extra. See https://github.com/clangd/clangd/issues/839 Repository: