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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGcb29c33984bf: [clangd][ObjC] Improve xrefs for protocols and classes (authored by dgoldman). Repository: rG LLVM Github Monorepo CHANGES SINCE

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

dgoldman updated this revision to Diff 284788. dgoldman marked 3 inline comments as done. dgoldman added a comment. - Final touches Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83501/new/ https://reviews.llvm.org/D83501 Files:

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

dgoldman added inline comments. Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:614 + +TEST_F(SymbolCollectorTest, ObjCClassExtensions) { + Annotations Header(R"( sammccall wrote: > dgoldman wrote: > > sammccall wrote: > > > dgoldman

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

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Ugh, I forgot to hit submit and went on vacation :-\ Really sorry again. As much as we can simplify/unify the tests that helps, but let's not block on this anymore, up to you.

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

dgoldman updated this revision to Diff 284425. dgoldman added a comment. Rebase + lint fix Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83501/new/ https://reviews.llvm.org/D83501 Files: clang-tools-extra/clangd/FindTarget.cpp

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

dgoldman added a comment. In D83501#2173534 , @sammccall wrote: > (Sorry this has been pending a while - I think it's basically there. Only > things we really need to address to land this is have a consistent view of > what the canonical decl is for the

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

dgoldman updated this revision to Diff 280930. dgoldman marked 7 inline comments as done. dgoldman added a comment. - More fixes for review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83501/new/ https://reviews.llvm.org/D83501 Files:

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

sammccall added a comment. (Sorry this has been pending a while - I think it's basically there. Only things we really need to address to land this is have a consistent view of what the canonical decl is for the no-@interface case, and avoid too much duplication of mechanisms in the tests)

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

sammccall added inline comments. Comment at: clang-tools-extra/clangd/FindTarget.cpp:360 + dyn_cast(D)) { + // Objective-C implementation should map back to its interface. + D = IID->getClassInterface(); This just describes what the

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

dgoldman updated this revision to Diff 280086. dgoldman added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83501/new/ https://reviews.llvm.org/D83501 Files: clang-tools-extra/clangd/FindTarget.cpp

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

dgoldman updated this revision to Diff 278752. dgoldman marked an inline comment as done. dgoldman added a comment. - Minor test and formatting fixes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83501/new/ https://reviews.llvm.org/D83501 Files:

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

dgoldman marked 7 inline comments as done. dgoldman added a comment. In D83501#2157957 , @sammccall wrote: > In D83501#2154300 , @dgoldman wrote: > > > In D83501#2153605 ,

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

sammccall added a comment. In D83501#2154300 , @dgoldman wrote: > In D83501#2153605 , @sammccall wrote: > > > In D83501#2148671 , @dgoldman > > wrote: > > > > > I

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

dgoldman marked an inline comment as done. dgoldman added inline comments. Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:614 + +TEST_F(SymbolCollectorTest, ObjCClassExtensions) { + Annotations Header(R"( Here's the ClassExtension that

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

dgoldman updated this revision to Diff 278310. dgoldman marked an inline comment as done. dgoldman added a comment. SymbolCollector fixes + more tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83501/new/ https://reviews.llvm.org/D83501 Files:

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

dgoldman marked 7 inline comments as done. dgoldman added a comment. In D83501#2153605 , @sammccall wrote: > In D83501#2148671 , @dgoldman wrote: > > > I implemented goto-definition from Foo() --> Foo impl in

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

sammccall added a comment. Thanks, this looks pretty good, I think there are a couple of adjustments... Comment at: clang-tools-extra/clangd/XRefs.cpp:276 getDeclAtPosition(AST, CurLoc, Relations, NodeKind)) { // Special case: void foo() ^override: jump to the

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

sammccall added a comment. In D83501#2148671 , @dgoldman wrote: > I implemented goto-definition from Foo() --> Foo impl in Xrefs, on the > symbolcollector side I don't think there's anything to do? This can't be done purely in xrefs as the AST may not

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

dgoldman updated this revision to Diff 277814. dgoldman added a comment. Minor lint fixes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83501/new/ https://reviews.llvm.org/D83501 Files: clang-tools-extra/clangd/FindTarget.cpp

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

dgoldman updated this revision to Diff 277601. dgoldman added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83501/new/ https://reviews.llvm.org/D83501 Files: clang-tools-extra/clangd/FindTarget.cpp

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

dgoldman marked 4 inline comments as done. dgoldman added a comment. In D83501#2146892 , @sammccall wrote: > In D83501#2144283 , @dgoldman wrote: > > > Looking further into the indexing support, I've questions: > >

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

dgoldman updated this revision to Diff 277582. dgoldman marked an inline comment as done. dgoldman added a comment. Fixes for ObjC in SymbolCollector as well as XRefs - Make sure ObjC indexing doesn't use the forward decls and doesn't treat the impl as the canonical decl - Don't treat ObjC

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

sammccall added inline comments. 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(D)) {

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

dgoldman marked 2 inline comments as done. dgoldman 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: > sammccall

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

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

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

sammccall added a comment. In D83501#2144283 , @dgoldman wrote: > Looking further into the indexing support, I've questions: > > - How `targetDecl` should behave for these objc container types (Hmm, targetDecl doesn't seem to canonicalize decls

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

dgoldman updated this revision to Diff 277105. dgoldman added a comment. rebase, phabricator keeps getting confused Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83501/new/ https://reviews.llvm.org/D83501 Files:

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

dgoldman updated this revision to Diff 277103. dgoldman added a comment. Move up null check Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83501/new/ https://reviews.llvm.org/D83501 Files: clang-tools-extra/clangd/FindTarget.cpp Index:

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

dgoldman updated this revision to Diff 277100. dgoldman added a comment. Find target fixes + symbol collector test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83501/new/ https://reviews.llvm.org/D83501 Files:

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

dgoldman marked 5 inline comments as done and an inline comment as not done. dgoldman added inline comments. Comment at: clang-tools-extra/clangd/FindTarget.cpp:273 Flags |= Rel::Underlying; // continue with the underlying decl. +} else if (const

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

dgoldman updated this revision to Diff 277084. dgoldman added a comment. Swap to getPreferredDecl and improve targetDecl Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83501/new/ https://reviews.llvm.org/D83501 Files:

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

dgoldman marked an inline comment as done. dgoldman added a comment. In 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

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

dgoldman updated this revision to Diff 277054. dgoldman marked 2 inline comments as done. dgoldman added a comment. Add support for categories + fix tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83501/new/ https://reviews.llvm.org/D83501

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

sammccall added a comment. 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

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

dgoldman updated this revision to Diff 277026. dgoldman added a comment. Fixes for getDefinition/getCanonicalDecl for ObjC Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83501/new/ https://reviews.llvm.org/D83501 Files:

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

dgoldman marked an inline comment as done. dgoldman added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:88-92 + if (const auto *ID = dyn_cast(D)) { +if (const auto *IMD = ID->getImplementation()) + return IMD; +return ID->getDefinition(); + }

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

sammccall marked an inline comment as done. 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: + // This is a really useful comment,

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

dgoldman added inline comments. 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(D)) {

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

dgoldman added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:88-92 + if (const auto *ID = dyn_cast(D)) { +if (const auto *IMD = ID->getImplementation()) + return IMD; +return ID->getDefinition(); + } Let me know if there's a

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

dgoldman created this revision. dgoldman added a reviewer: sammccall. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Previously clangd would jump to forward declarations for protocols and classes instead of