[PATCH] D104117: [clangd] Fix highlighting for implicit ObjC property refs

[David Goldman via Phabricator via cfe-commits]

dgoldman added a comment.

In D104117#2835599 <https://reviews.llvm.org/D104117#2835599>, @sammccall wrote:

> Drive by from vacation, sorry - can this be tackled "upstream" in findExplicitReferences rather than special cased in syntax highlighting?
>
> That would help with xrefs, rename etc. Features should really be only handled here if they're somehow not a simple token->decl reference, but this seems to be.

There's a couple of problems with that at the moment:

- Currently all ObjC method handling is done here since ObjC has selectors which need to be special cased
- FindExplicitRefs right now resolves these down to their ObjCMethodDecls, losing syntactic information that they were referenced by property expressions and not method expressions. We'd have to store that somehow to note these as `Field` and not `Method`.



================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:579
+      return true;
+    // A single property expr can reference both a getter and setter, but we can
+    // only provide a single semantic token, so prefer the getter.
----------------
kadircet wrote:
> is there any difference to using one or the other ? (i.e. can setter be static while getter isn't? I suppose not). maybe mention that in the comment and say that we are choosing whichever exists (and change the logic below to `if` followed by an `else if`?
Not like that - but it's technically possible you could have a getter in an SDK and a user-provided setter for it (but that would be really weird); both of them can exist.


================
Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:699
         @property(nonatomic, assign) int $Field_decl[[someProperty]];
+        @property(readonly, class) $Class[[Foo]] *$Field_decl_readonly_static[[sharedInstance]];
         @end
----------------
kadircet wrote:
> we have an explicit `@property` here, but comments in the implementation says otherwise. did you mean not having any explicit `getter/setter` or `@synthesize` statement ?
After some testing it seems like property refs to class properties always use the implicit form like `ObjCPropertyRefExpr 0x7f87cb8116f0 <col:11, col:15> '<pseudo-object type>' lvalue objcproperty Kind=MethodRef Getter="sharedInstance" Setter="(null)" Messaging=Getter` 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104117