[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

ckandeler added a comment. In D127403#3863713 , @nridge wrote: > In D127403#3863686 , @thakis wrote: > >> In D127403#3863641 , @nridge wrote: >> >>> Landed

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

nridge added a comment. In D127403#3863686 , @thakis wrote: > In D127403#3863641 , @nridge wrote: > >> Landed https://reviews.llvm.org/rGc93430bae4fc > > Still failing with that:

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

thakis added a comment. In D127403#3863641 , @nridge wrote: > Landed https://reviews.llvm.org/rGc93430bae4fc Still failing with that: http://45.33.8.238/linux/89240/step_9.txt Time to revert to green? Things have been broken for a few hours now. Maybe

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

nridge added a comment. Landed https://reviews.llvm.org/rGc93430bae4fc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127403/new/ https://reviews.llvm.org/D127403 ___ cfe-commits mailing list

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

nridge added a comment. Landing the fix now Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127403/new/ https://reviews.llvm.org/D127403 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

thakis added a comment. Looks like this breaks check-clangd everywhere, eg http://45.33.8.238/linux/89237/step_9.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127403/new/ https://reviews.llvm.org/D127403

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

nridge added a comment. Looks like an error introduced when rebasing the patch: a recently added test case (for https://github.com/clangd/clangd/issues/1222) needs to be updated to reflect the `_decl` to `_def` change that this patch makes to all semantic highlighting tests. I'll land a fix.

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

ormris added a comment. Looks like there are some test failures associated with this commit: https://lab.llvm.org/buildbot/#/builders/123/builds/13698 The change list is a bit confusing, as it's only showing LLVM commits. But the change list includes the commit before and the commit after this

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

nridge added a comment. I went ahead and landed this for you Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127403/new/ https://reviews.llvm.org/D127403 ___ cfe-commits mailing list

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

This revision was automatically updated to reflect the committed changes. Closed by commit rG4abc910a42e5: [clangd] Implement semantic token modifier definition (authored by ckandeler, committed by nridge). Changed prior to commit: https://reviews.llvm.org/D127403?vs=466731=468318#toc

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

Trass3r added a comment. https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127403/new/ https://reviews.llvm.org/D127403 ___ cfe-commits

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

ckandeler added a comment. Could you please land it for me? I don't have the privileges. (How does one attain those, by the way?) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127403/new/ https://reviews.llvm.org/D127403

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

sammccall added a comment. In D127403#3861619 , @ckandeler wrote: > Everything good now? Yes :-) (I also marked it as accepted above, so feel free to land!) Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

ckandeler added a comment. Everything good now? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127403/new/ https://reviews.llvm.org/D127403 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

ckandeler updated this revision to Diff 466731. ckandeler added a comment. Template parameters are definitions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127403/new/ https://reviews.llvm.org/D127403 Files:

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Just realized ParmVarDecls (and template args) are maybe more complicated than I thought: I suppose they're only really definitions if the function/template has a body (i.e. is itself a

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

ckandeler updated this revision to Diff 466510. ckandeler marked 2 inline comments as done. ckandeler added a comment. Use modifier also for parameters, Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127403/new/ https://reviews.llvm.org/D127403

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

sammccall accepted this revision. sammccall added a comment. Thanks! Sorry for letting this drop. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:79 + if (auto *Var = dyn_cast(Decl)) +return !isa(Var) && Var->isThisDeclarationADefinition(); + return

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

ckandeler added a comment. So, is this okay to merge now? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127403/new/ https://reviews.llvm.org/D127403 ___ cfe-commits mailing list

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

ckandeler added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:79 + if (auto *Var = dyn_cast(Decl)) +return !isa(Var) && Var->isThisDeclarationADefinition(); + return isa(Decl) || isa(Decl); I'm not 100% sure about this one,

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

ckandeler updated this revision to Diff 464170. ckandeler marked 3 inline comments as done. ckandeler added a comment. Adapted according to latest comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127403/new/ https://reviews.llvm.org/D127403

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. This is fantastic, I'm really not sure how I missed it, sorry :-( Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:857

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

ckandeler added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:863 + Tok.addModifier(HighlightingModifier::Declaration); +if (const auto Func = dyn_cast(Decl)) { + if (Func->isThisDeclarationADefinition())

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

dgoldman added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:863 + Tok.addModifier(HighlightingModifier::Declaration); +if (const auto Func = dyn_cast(Decl)) { + if (Func->isThisDeclarationADefinition())

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

ckandeler added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127403/new/ https://reviews.llvm.org/D127403 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

ckandeler updated this revision to Diff 448655. ckandeler added a comment. Addressed remaining Objective-C issues Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127403/new/ https://reviews.llvm.org/D127403 Files:

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

ckandeler updated this revision to Diff 436678. ckandeler added a comment. Objective-C improvements. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127403/new/ https://reviews.llvm.org/D127403 Files:

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

dgoldman added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:847-849 +} else if (const auto Iface = + dyn_cast(Decl)) { + if (Iface->isThisDeclarationADefinition()) ckandeler

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

ckandeler added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:847-849 +} else if (const auto Iface = + dyn_cast(Decl)) { + if (Iface->isThisDeclarationADefinition()) dgoldman

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

dgoldman added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:847-849 +} else if (const auto Iface = + dyn_cast(Decl)) { + if (Iface->isThisDeclarationADefinition()) Do we need

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

ckandeler updated this revision to Diff 435561. ckandeler added a comment. Addressed clang-format complaints. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127403/new/ https://reviews.llvm.org/D127403 Files:

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

ckandeler updated this revision to Diff 435547. ckandeler added a comment. Indentation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127403/new/ https://reviews.llvm.org/D127403 Files: clang-tools-extra/clangd/SemanticHighlighting.cpp

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

ckandeler created this revision. Herald added subscribers: usaxena95, kadircet, arphaman. Herald added a project: All. ckandeler requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Repository: rG LLVM