[PATCH] D83501: [clangd][ObjC] Improve xrefs for protocols and classes

[David Goldman via Phabricator via cfe-commits]

dgoldman marked an inline comment as done.
dgoldman added a comment.

In D83501#2144144 <https://reviews.llvm.org/D83501#2144144>, @sammccall wrote:

> I think without index changes this will still give the wrong answer for go-to-definition if the @implementation is in a different file.
>  Do you want to include those too or will that work ok in a separate patch?
>  (I'm not 100% sure of the interactions here, possible it'll seem a bit glitchy)


I'll look into the index changes as well in this change, just wanted to get the base functionality down for xrefs first. If it seems large I'll probably send a follow up diff.



================
Comment at: clang-tools-extra/clangd/XRefs.cpp:276
        getDeclAtPosition(AST, CurLoc, Relations, NodeKind)) {
     // Special case: void foo() ^override: jump to the overridden method.
     if (const auto *CMD = llvm::dyn_cast<CXXMethodDecl>(D)) {
----------------
sammccall wrote:
> dgoldman wrote:
> > Think it would make sense to special case ObjCInterfaceDecl here to get at both the interface definition + implementation if available?
> Rather than returning both results, I think it's more consistent to return them as a declaration/definition pair.
> 
> (This means special-casing ObjCImplDecl in namedDecl or at least getDeclAsPosition, so you always end up with the ObjCInterfaceDecl instead)
Whoops, meant to comment here but it was lost. I'm not sure what you meant here. Should this be done here inside the for loop or in getDeclAtPosition?

Are you saying that given:

```
@interface Foo // A
@end
@implementation Foo // B
@end
```
B --> A here

and similarly

```
@interface Foo // A
@end
@interface Foo (Ext) // B
@end
@implementation Foo (Ext) // C
@end
```
B --> A
C --> B (and A? it's unclear how this should map over, e.g. maybe Foo loc --> A, Ext --> B)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83501