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



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

Reply via email to