iannisdezwart accepted this revision.
iannisdezwart added a comment.
This revision is now accepted and ready to land.

In D119077#3300271 <https://reviews.llvm.org/D119077#3300271>, @nridge wrote:

> I haven't looked at the patch in detail, but one high level question: have 
> you considered the possibility of adding these highlightings during the 
> findExplicitReferences 
> <https://searchfox.org/llvm/rev/bad1b7fbb0fec532f097ac59805562020f895962/clang-tools-extra/clangd/SemanticHighlighting.cpp#794>
>  phase, rather than in `CollectExtraHighlightings`? (I haven't thought 
> through whether that would work, just wondering if you have. The reason this 
> is worth asking is that if we can get `findExplicitReferences` to handle 
> overloaded operator calls, other clangd features that use 
> `findExplicitReferences` would benefit from knowing about such calls as well.)

I have in fact looked at the possibility of adding the code into 
`findExplicitReferences`, but I figured it would be more suitable to add it 
into `CollectExtraHighlightings`, because it is easier to distinguish 
declarations from references. In `findExplicitReferences` the declaration will 
be traversed multiple times, so it would be a pain to check if it had already 
been traversed. I found it was relatively easy to write code that would work in 
`CollectExtraHighlightings`, and I also don't know for sure if it's even 
possible to handle all cases in `findExplicitReferences`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119077/new/

https://reviews.llvm.org/D119077

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to