[clang-tools-extra] [clangd] Update XRefs to support overriden ObjC methods (PR #127109)
David Goldman via cfe-commits
cfe-commits at lists.llvm.org
Wed Feb 19 09:48:06 PST 2025
https://github.com/DavidGoldman updated https://github.com/llvm/llvm-project/pull/127109
>From d335931b8923005d2ffc580c5195b7fa997f3649 Mon Sep 17 00:00:00 2001
From: David Goldman <davg at google.com>
Date: Thu, 13 Feb 2025 13:51:05 -0500
Subject: [PATCH 1/7] [clangd] Update XRefs to support overriden ObjC methods
- Also support finding implementators of a protocol
- Also support discovering subclasses for ObjC interfaces
via the implementations call
- Add tests
---
clang-tools-extra/clangd/XRefs.cpp | 32 ++++++++
.../clangd/unittests/XRefsTests.cpp | 75 +++++++++++++++++++
2 files changed, 107 insertions(+)
diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp
index 1a23f6cca7756..095664a2391c6 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -430,6 +430,18 @@ locateASTReferent(SourceLocation CurLoc, const syntax::Token *TouchedIdentifier,
continue;
}
}
+ // Special case: an Objective-C method can override a parent class or
+ // protocol.
+ if (const auto *OMD = llvm::dyn_cast<ObjCMethodDecl>(D)) {
+ llvm::SmallVector<const ObjCMethodDecl *, 4> Overrides;
+ OMD->getOverriddenMethods(Overrides);
+ for (const auto *Override : Overrides)
+ AddResultDecl(Override);
+ if (!Overrides.empty())
+ LocateASTReferentMetric.record(1, "objc-overriden-method");
+ AddResultDecl(OMD);
+ continue;
+ }
// Special case: the cursor is on an alias, prefer other results.
// This targets "using ns::^Foo", where the target is more interesting.
@@ -1283,6 +1295,17 @@ std::vector<LocatedSymbol> findImplementations(ParsedAST &AST, Position Pos,
} else if (const auto *RD = dyn_cast<CXXRecordDecl>(ND)) {
IDs.insert(getSymbolID(RD));
QueryKind = RelationKind::BaseOf;
+ } else if (const auto *OMD = dyn_cast<ObjCMethodDecl>(ND)) {
+ IDs.insert(getSymbolID(OMD));
+ QueryKind = RelationKind::OverriddenBy;
+
+ llvm::SmallVector<const ObjCMethodDecl *, 4> Overrides;
+ OMD->getOverriddenMethods(Overrides);
+ for (const auto *Override : Overrides)
+ IDs.insert(getSymbolID(Override));
+ } else if (const auto *ID = dyn_cast<ObjCInterfaceDecl>(ND)) {
+ IDs.insert(getSymbolID(ID));
+ QueryKind = RelationKind::BaseOf;
}
}
return findImplementors(std::move(IDs), QueryKind, Index, AST.tuPath());
@@ -1438,6 +1461,15 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
getOverriddenMethods(CMD, OverriddenMethods);
}
}
+ // Special case: Objective-C methods can override a parent class or
+ // protocol, we should be sure to report references to those.
+ if (const auto *OMD = llvm::dyn_cast<ObjCMethodDecl>(ND)) {
+ OverriddenBy.Subjects.insert(getSymbolID(OMD));
+ llvm::SmallVector<const ObjCMethodDecl *, 4> Overrides;
+ OMD->getOverriddenMethods(Overrides);
+ for (const auto *Override : Overrides)
+ OverriddenMethods.insert(getSymbolID(Override));
+ }
}
}
diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index 7d824d659ad2c..a310a51cf4cfb 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -411,6 +411,26 @@ TEST(LocateSymbol, FindOverrides) {
sym("foo", Code.range("2"), std::nullopt)));
}
+TEST(LocateSymbol, FindOverridesObjC) {
+ auto Code = Annotations(R"objc(
+ @interface Foo
+ - (void)$1[[foo]];
+ @end
+
+ @interface Bar : Foo
+ @end
+ @implementation Bar
+ - (void)$2[[fo^o]] {}
+ @end
+ )objc");
+ TestTU TU = TestTU::withCode(Code.code());
+ TU.ExtraArgs.push_back("-xobjective-c++");
+ auto AST = TU.build();
+ EXPECT_THAT(locateSymbolAt(AST, Code.point(), TU.index().get()),
+ UnorderedElementsAre(sym("foo", Code.range("1"), std::nullopt),
+ sym("foo", Code.range("2"), Code.range("2"))));
+}
+
TEST(LocateSymbol, WithIndexPreferredLocation) {
Annotations SymbolHeader(R"cpp(
class $p[[Proto]] {};
@@ -1834,6 +1854,41 @@ TEST(FindImplementations, Inheritance) {
}
}
+TEST(FindImplementations, InheritanceObjC) {
+ llvm::StringRef Test = R"objc(
+ @interface $base^Base
+ - (void)fo$foo^o;
+ @end
+ @protocol Protocol
+ - (void)$protocol^protocol;
+ @end
+ @interface $ChildDecl[[Child]] : Base <Protocol>
+ - (void)concrete;
+ - (void)$fooDecl[[foo]];
+ @end
+ @implementation $ChildDef[[Child]]
+ - (void)concrete {}
+ - (void)$fooDef[[foo]] {}
+ - (void)$protocolDef[[protocol]] {}
+ @end
+ )objc";
+
+ Annotations Code(Test);
+ auto TU = TestTU::withCode(Code.code());
+ TU.ExtraArgs.push_back("-xobjective-c++");
+ auto AST = TU.build();
+ auto Index = TU.index();
+ EXPECT_THAT(
+ findImplementations(AST, Code.point("base"), Index.get()),
+ UnorderedElementsAre(sym("Child", Code.range("ChildDecl"), Code.range("ChildDef"))));
+ EXPECT_THAT(
+ findImplementations(AST, Code.point("foo"), Index.get()),
+ UnorderedElementsAre(sym("foo", Code.range("fooDecl"), Code.range("fooDef"))));
+ EXPECT_THAT(
+ findImplementations(AST, Code.point("protocol"), Index.get()),
+ UnorderedElementsAre(sym("protocol", Code.range("protocolDef"), Code.range("protocolDef"))));
+}
+
TEST(FindImplementations, CaptureDefinition) {
llvm::StringRef Test = R"cpp(
struct Base {
@@ -1963,6 +2018,7 @@ void checkFindRefs(llvm::StringRef Test, bool UseIndex = false) {
Annotations T(Test);
auto TU = TestTU::withCode(T.code());
TU.ExtraArgs.push_back("-std=c++20");
+ TU.ExtraArgs.push_back("-xobjective-c++");
auto AST = TU.build();
std::vector<Matcher<ReferencesResult::Reference>> ExpectedLocations;
@@ -2260,6 +2316,25 @@ TEST(FindReferences, IncludeOverrides) {
checkFindRefs(Test, /*UseIndex=*/true);
}
+TEST(FindReferences, IncludeOverridesObjC) {
+ llvm::StringRef Test =
+ R"objc(
+ @interface Base
+ - (void)$decl(Base)[[f^unc]];
+ @end
+ @interface Derived : Base
+ - (void)$overridedecl(Derived::func)[[func]];
+ @end
+ @implementation Derived
+ - (void)$overridedef[[func]] {}
+ @end
+ void test(Derived *derived, Base *base) {
+ [derived func]; // No references to the overrides.
+ [base $(test)[[func]]];
+ })objc";
+ checkFindRefs(Test, /*UseIndex=*/true);
+}
+
TEST(FindReferences, RefsToBaseMethod) {
llvm::StringRef Test =
R"cpp(
>From 65f19b760067233f75d5e8a66a8deb8419e1b47f Mon Sep 17 00:00:00 2001
From: David Goldman <davg at google.com>
Date: Thu, 13 Feb 2025 14:02:15 -0500
Subject: [PATCH 2/7] Run clang-format
---
.../clangd/unittests/XRefsTests.cpp | 25 ++++++++++---------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index a310a51cf4cfb..0f8d096e28c87 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -426,9 +426,10 @@ TEST(LocateSymbol, FindOverridesObjC) {
TestTU TU = TestTU::withCode(Code.code());
TU.ExtraArgs.push_back("-xobjective-c++");
auto AST = TU.build();
- EXPECT_THAT(locateSymbolAt(AST, Code.point(), TU.index().get()),
- UnorderedElementsAre(sym("foo", Code.range("1"), std::nullopt),
- sym("foo", Code.range("2"), Code.range("2"))));
+ EXPECT_THAT(
+ locateSymbolAt(AST, Code.point(), TU.index().get()),
+ UnorderedElementsAre(sym("foo", Code.range("1"), std::nullopt),
+ sym("foo", Code.range("2"), Code.range("2"))));
}
TEST(LocateSymbol, WithIndexPreferredLocation) {
@@ -1878,15 +1879,15 @@ TEST(FindImplementations, InheritanceObjC) {
TU.ExtraArgs.push_back("-xobjective-c++");
auto AST = TU.build();
auto Index = TU.index();
- EXPECT_THAT(
- findImplementations(AST, Code.point("base"), Index.get()),
- UnorderedElementsAre(sym("Child", Code.range("ChildDecl"), Code.range("ChildDef"))));
- EXPECT_THAT(
- findImplementations(AST, Code.point("foo"), Index.get()),
- UnorderedElementsAre(sym("foo", Code.range("fooDecl"), Code.range("fooDef"))));
- EXPECT_THAT(
- findImplementations(AST, Code.point("protocol"), Index.get()),
- UnorderedElementsAre(sym("protocol", Code.range("protocolDef"), Code.range("protocolDef"))));
+ EXPECT_THAT(findImplementations(AST, Code.point("base"), Index.get()),
+ UnorderedElementsAre(sym("Child", Code.range("ChildDecl"),
+ Code.range("ChildDef"))));
+ EXPECT_THAT(findImplementations(AST, Code.point("foo"), Index.get()),
+ UnorderedElementsAre(
+ sym("foo", Code.range("fooDecl"), Code.range("fooDef"))));
+ EXPECT_THAT(findImplementations(AST, Code.point("protocol"), Index.get()),
+ UnorderedElementsAre(sym("protocol", Code.range("protocolDef"),
+ Code.range("protocolDef"))));
}
TEST(FindImplementations, CaptureDefinition) {
>From 68fd05989c4a1ac9ae4f114b6b753167ed63bc36 Mon Sep 17 00:00:00 2001
From: David Goldman <davg at google.com>
Date: Fri, 14 Feb 2025 13:33:20 -0500
Subject: [PATCH 3/7] Only jump to overrides when selecting the method; fix
weird findImpl behavior
---
clang-tools-extra/clangd/XRefs.cpp | 38 ++++++++++++++++++------------
1 file changed, 23 insertions(+), 15 deletions(-)
diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp
index 095664a2391c6..e2fcc23bafdf6 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -372,6 +372,15 @@ void enhanceLocatedSymbolsFromIndex(llvm::MutableArrayRef<LocatedSymbol> Result,
});
}
+bool objcMethodIsTouched(const SourceManager &SM, const ObjCMethodDecl *OMD,
+ SourceLocation Loc) {
+ unsigned NumSels = OMD->getNumSelectorLocs();
+ for (unsigned I = 0; I < NumSels; ++I)
+ if (SM.getSpellingLoc(OMD->getSelectorLoc(I)) == Loc)
+ return true;
+ return false;
+}
+
// Decls are more complicated.
// The AST contains at least a declaration, maybe a definition.
// These are up-to-date, and so generally preferred over index results.
@@ -430,17 +439,21 @@ locateASTReferent(SourceLocation CurLoc, const syntax::Token *TouchedIdentifier,
continue;
}
}
- // Special case: an Objective-C method can override a parent class or
- // protocol.
+ // Special case: - (void)^method; should jump to all overrides. Note that an
+ // Objective-C method can override a parent class or protocol.
if (const auto *OMD = llvm::dyn_cast<ObjCMethodDecl>(D)) {
- llvm::SmallVector<const ObjCMethodDecl *, 4> Overrides;
- OMD->getOverriddenMethods(Overrides);
- for (const auto *Override : Overrides)
- AddResultDecl(Override);
- if (!Overrides.empty())
- LocateASTReferentMetric.record(1, "objc-overriden-method");
- AddResultDecl(OMD);
- continue;
+ if (TouchedIdentifier &&
+ objcMethodIsTouched(SM, OMD, TouchedIdentifier->location())) {
+ llvm::SmallVector<const ObjCMethodDecl *, 4> Overrides;
+ OMD->getOverriddenMethods(Overrides);
+ if (!Overrides.empty()) {
+ for (const auto *Override : Overrides)
+ AddResultDecl(Override);
+ LocateASTReferentMetric.record(1, "objc-overriden-method");
+ }
+ AddResultDecl(OMD);
+ continue;
+ }
}
// Special case: the cursor is on an alias, prefer other results.
@@ -1298,11 +1311,6 @@ std::vector<LocatedSymbol> findImplementations(ParsedAST &AST, Position Pos,
} else if (const auto *OMD = dyn_cast<ObjCMethodDecl>(ND)) {
IDs.insert(getSymbolID(OMD));
QueryKind = RelationKind::OverriddenBy;
-
- llvm::SmallVector<const ObjCMethodDecl *, 4> Overrides;
- OMD->getOverriddenMethods(Overrides);
- for (const auto *Override : Overrides)
- IDs.insert(getSymbolID(Override));
} else if (const auto *ID = dyn_cast<ObjCInterfaceDecl>(ND)) {
IDs.insert(getSymbolID(ID));
QueryKind = RelationKind::BaseOf;
>From e962112d956b6746e328ec7f50c21be27fd755ed Mon Sep 17 00:00:00 2001
From: David Goldman <davg at google.com>
Date: Fri, 14 Feb 2025 13:48:20 -0500
Subject: [PATCH 4/7] Add Objective-C inheritance test to SymbolCollector
---
.../clangd/unittests/SymbolCollectorTests.cpp | 36 +++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 7a9703c744e93..1ce28c91a420c 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -1335,6 +1335,42 @@ TEST_F(SymbolCollectorTest, OverrideRelationsMultipleInheritance) {
OverriddenBy(CBar, DBar), OverriddenBy(CBaz, DBaz)));
}
+TEST_F(SymbolCollectorTest, ObjCOverrideRelationsSimpleInheritance) {
+ std::string Header = R"cpp(
+ @interface A
+ - (void)foo;
+ @end
+ @interface B : A
+ - (void)foo; // A::foo
+ - (void)bar;
+ @end
+ @interface C : B
+ - (void)bar; // B::bar
+ @end
+ @interface D : C
+ - (void)foo; // B::foo
+ - (void)bar; // C::bar
+ @end
+ )cpp";
+ runSymbolCollector(Header, /*Main=*/"",
+ {"-xobjective-c++", "-Wno-objc-root-class"});
+ const Symbol &AFoo = findSymbol(Symbols, "A::foo");
+ const Symbol &BFoo = findSymbol(Symbols, "B::foo");
+ const Symbol &DFoo = findSymbol(Symbols, "D::foo");
+
+ const Symbol &BBar = findSymbol(Symbols, "B::bar");
+ const Symbol &CBar = findSymbol(Symbols, "C::bar");
+ const Symbol &DBar = findSymbol(Symbols, "D::bar");
+
+ std::vector<Relation> Result;
+ for (const Relation &R : Relations)
+ if (R.Predicate == RelationKind::OverriddenBy)
+ Result.push_back(R);
+ EXPECT_THAT(Result, UnorderedElementsAre(
+ OverriddenBy(AFoo, BFoo), OverriddenBy(BBar, CBar),
+ OverriddenBy(BFoo, DFoo), OverriddenBy(CBar, DBar)));
+}
+
TEST_F(SymbolCollectorTest, CountReferences) {
const std::string Header = R"(
class W;
>From 4f88a701d61eb9e77d99fed4d77ac70ec3116d28 Mon Sep 17 00:00:00 2001
From: David Goldman <davg at google.com>
Date: Tue, 18 Feb 2025 14:53:36 -0500
Subject: [PATCH 5/7] Recursively find all ObjC overrides + add test
---
clang-tools-extra/clangd/XRefs.cpp | 25 ++++++--
.../clangd/unittests/XRefsTests.cpp | 58 ++++++++++++++++++-
2 files changed, 75 insertions(+), 8 deletions(-)
diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp
index e2fcc23bafdf6..18fc35c0fac75 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -439,8 +439,11 @@ locateASTReferent(SourceLocation CurLoc, const syntax::Token *TouchedIdentifier,
continue;
}
}
- // Special case: - (void)^method; should jump to all overrides. Note that an
+ // Special case: - (void)^method; should jump to overrides. Note that an
// Objective-C method can override a parent class or protocol.
+ //
+ // FIXME: Support jumping from a protocol decl to overrides on go-to
+ // definition.
if (const auto *OMD = llvm::dyn_cast<ObjCMethodDecl>(D)) {
if (TouchedIdentifier &&
objcMethodIsTouched(SM, OMD, TouchedIdentifier->location())) {
@@ -1333,6 +1336,21 @@ void getOverriddenMethods(const CXXMethodDecl *CMD,
}
}
+// Recursively finds all the overridden methods of `OMD` in complete type
+// hierarchy.
+void getOverriddenMethods(const ObjCMethodDecl *OMD,
+ llvm::DenseSet<SymbolID> &OverriddenMethods) {
+ if (!OMD)
+ return;
+ llvm::SmallVector<const ObjCMethodDecl *, 4> Overrides;
+ OMD->getOverriddenMethods(Overrides);
+ for (const ObjCMethodDecl *Base : Overrides) {
+ if (auto ID = getSymbolID(Base))
+ OverriddenMethods.insert(ID);
+ getOverriddenMethods(Base, OverriddenMethods);
+ }
+}
+
std::optional<std::string>
stringifyContainerForMainFileRef(const Decl *Container) {
// FIXME We might also want to display the signature here
@@ -1473,10 +1491,7 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
// protocol, we should be sure to report references to those.
if (const auto *OMD = llvm::dyn_cast<ObjCMethodDecl>(ND)) {
OverriddenBy.Subjects.insert(getSymbolID(OMD));
- llvm::SmallVector<const ObjCMethodDecl *, 4> Overrides;
- OMD->getOverriddenMethods(Overrides);
- for (const auto *Override : Overrides)
- OverriddenMethods.insert(getSymbolID(Override));
+ getOverriddenMethods(OMD, OverriddenMethods);
}
}
}
diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index 0f8d096e28c87..1ce778e076730 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -413,14 +413,21 @@ TEST(LocateSymbol, FindOverrides) {
TEST(LocateSymbol, FindOverridesObjC) {
auto Code = Annotations(R"objc(
- @interface Foo
+ @protocol Fooey
+ - (void)foo;
+ @end
+ @interface Base
+ - (void)foo;
+ @end
+ @interface Foo : Base<Fooey>
- (void)$1[[foo]];
@end
@interface Bar : Foo
+ - (void)$2[[foo]];
@end
@implementation Bar
- - (void)$2[[fo^o]] {}
+ - (void)$3[[fo^o]] {}
@end
)objc");
TestTU TU = TestTU::withCode(Code.code());
@@ -429,7 +436,31 @@ TEST(LocateSymbol, FindOverridesObjC) {
EXPECT_THAT(
locateSymbolAt(AST, Code.point(), TU.index().get()),
UnorderedElementsAre(sym("foo", Code.range("1"), std::nullopt),
- sym("foo", Code.range("2"), Code.range("2"))));
+ sym("foo", Code.range("2"), Code.range("3"))));
+}
+
+TEST(LocateSymbol, ObjCNoOverridesOnUsage) {
+ auto Code = Annotations(R"objc(
+ @interface Foo
+ - (void)foo;
+ @end
+
+ @interface Bar : Foo
+ - (void)$1[[foo]];
+ @end
+ @implementation Bar
+ - (void)$2[[foo]] {}
+ @end
+ void doSomething(Bar *bar) {
+ [bar fo^o];
+ }
+ )objc");
+ TestTU TU = TestTU::withCode(Code.code());
+ TU.ExtraArgs.push_back("-xobjective-c++");
+ auto AST = TU.build();
+ EXPECT_THAT(
+ locateSymbolAt(AST, Code.point(), TU.index().get()),
+ UnorderedElementsAre(sym("foo", Code.range("1"), Code.range("2"))));
}
TEST(LocateSymbol, WithIndexPreferredLocation) {
@@ -2360,6 +2391,27 @@ TEST(FindReferences, RefsToBaseMethod) {
checkFindRefs(Test, /*UseIndex=*/true);
}
+TEST(FindReferences, RefsToBaseMethodObjC) {
+ llvm::StringRef Test =
+ R"objc(
+ @interface BaseBase
+ - (void)$(BaseBase)[[func]];
+ @end
+ @interface Base : BaseBase
+ - (void)$(Base)[[func]];
+ @end
+ @interface Derived : Base
+ - (void)$decl(Derived)[[fu^nc]];
+ @end
+ void test(BaseBase *bb, Base *b, Derived *d) {
+ // refs to overridden methods in complete type hierarchy are reported.
+ [bb $(test)[[func]]];
+ [b $(test)[[func]]];
+ [d $(test)[[fu^nc]]];
+ })objc";
+ checkFindRefs(Test, /*UseIndex=*/true);
+}
+
TEST(FindReferences, MainFileReferencesOnly) {
llvm::StringRef Test =
R"cpp(
>From 478cd5aec7462a667a3d5a3fcf2a5a27cd7d7d09 Mon Sep 17 00:00:00 2001
From: David Goldman <davg at google.com>
Date: Wed, 19 Feb 2025 10:41:59 -0500
Subject: [PATCH 6/7] Only give overrides on ObjC method defs
---
clang-tools-extra/clangd/XRefs.cpp | 8 +++--
.../clangd/unittests/XRefsTests.cpp | 29 ++++++++++++++++++-
2 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp
index 18fc35c0fac75..f26d5db9b2b2b 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -439,14 +439,16 @@ locateASTReferent(SourceLocation CurLoc, const syntax::Token *TouchedIdentifier,
continue;
}
}
- // Special case: - (void)^method; should jump to overrides. Note that an
- // Objective-C method can override a parent class or protocol.
+ // Special case: - (void)^method {} should jump to overrides, but the decl
+ // shouldn't, only the definition. Note that an Objective-C method can
+ // override a parent class or protocol.
//
// FIXME: Support jumping from a protocol decl to overrides on go-to
// definition.
if (const auto *OMD = llvm::dyn_cast<ObjCMethodDecl>(D)) {
if (TouchedIdentifier &&
- objcMethodIsTouched(SM, OMD, TouchedIdentifier->location())) {
+ objcMethodIsTouched(SM, OMD, TouchedIdentifier->location()) &&
+ OMD->isThisDeclarationADefinition()) {
llvm::SmallVector<const ObjCMethodDecl *, 4> Overrides;
OMD->getOverriddenMethods(Overrides);
if (!Overrides.empty()) {
diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index 1ce778e076730..475b56b1dc230 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -411,7 +411,7 @@ TEST(LocateSymbol, FindOverrides) {
sym("foo", Code.range("2"), std::nullopt)));
}
-TEST(LocateSymbol, FindOverridesObjC) {
+TEST(LocateSymbol, FindOverridesFromDefObjC) {
auto Code = Annotations(R"objc(
@protocol Fooey
- (void)foo;
@@ -439,6 +439,33 @@ TEST(LocateSymbol, FindOverridesObjC) {
sym("foo", Code.range("2"), Code.range("3"))));
}
+TEST(LocateSymbol, NoOverridesFromDeclObjC) {
+ auto Code = Annotations(R"objc(
+ @protocol Fooey
+ - (void)foo;
+ @end
+ @interface Base
+ - (void)foo;
+ @end
+ @interface Foo : Base<Fooey>
+ - (void)foo;
+ @end
+
+ @interface Bar : Foo
+ - (void)$2[[fo^o]];
+ @end
+ @implementation Bar
+ - (void)$3[[foo]] {}
+ @end
+ )objc");
+ TestTU TU = TestTU::withCode(Code.code());
+ TU.ExtraArgs.push_back("-xobjective-c++");
+ auto AST = TU.build();
+ EXPECT_THAT(
+ locateSymbolAt(AST, Code.point(), TU.index().get()),
+ UnorderedElementsAre(sym("foo", Code.range("2"), Code.range("3"))));
+}
+
TEST(LocateSymbol, ObjCNoOverridesOnUsage) {
auto Code = Annotations(R"objc(
@interface Foo
>From 13c701176b2854e3e3e4f81ae42742c869890972 Mon Sep 17 00:00:00 2001
From: David Goldman <davg at google.com>
Date: Wed, 19 Feb 2025 12:44:44 -0500
Subject: [PATCH 7/7] Check for definition first
---
clang-tools-extra/clangd/XRefs.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp
index f26d5db9b2b2b..8b9fffa3f64cd 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -446,9 +446,8 @@ locateASTReferent(SourceLocation CurLoc, const syntax::Token *TouchedIdentifier,
// FIXME: Support jumping from a protocol decl to overrides on go-to
// definition.
if (const auto *OMD = llvm::dyn_cast<ObjCMethodDecl>(D)) {
- if (TouchedIdentifier &&
- objcMethodIsTouched(SM, OMD, TouchedIdentifier->location()) &&
- OMD->isThisDeclarationADefinition()) {
+ if (OMD->isThisDeclarationADefinition() && TouchedIdentifier &&
+ objcMethodIsTouched(SM, OMD, TouchedIdentifier->location())) {
llvm::SmallVector<const ObjCMethodDecl *, 4> Overrides;
OMD->getOverriddenMethods(Overrides);
if (!Overrides.empty()) {
More information about the cfe-commits
mailing list