sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:83
+ return PD->getDefinition();
+ // Objective-C classes can have three types of declarations:
+ //
----------------
sammccall wrote:
> This is a really useful comment, thanks!
... and it's gone.
I think some comment is useful here, as this line is doing something subtly
different than all the other lines - returning a decl that isn't equivalent to
its input.
================
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)) {
----------------
dgoldman wrote:
> 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)
In the first example, selecting either A or B should yield one LocatedSymbol
with {Decl=A, Def=B}. This shouldn't require any special-casing.
The second example is very similar to template specialization, with exceptions:
- there's always a Decl/Def pair you may want to navigate between, whereas in
templates there rarely is, so we have ambiguity
- there's no AST like there is for template names and args, just a bag of
tokens
I'd suggest, given `@interface Foo (Ext)`:
- we produce a LocatedSymbol with {Decl=@interface Foo(Ext),
Def=@implementation Foo(Ext)} - this is the default behavior
- if the cursor is exactly on the token `Foo`, we also produce a LocatedSymbol
with {Decl=@interface Foo, Def=@implementation Foo} - this is similar to the
template special case
- if the cursor is exactly on the token Ext... are categories
explicitly/separately declared anywhere? I guess not. If they are, we could
special case this too.
And `@implementation Foo(Ext)` should behave in exactly the same way.
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
