Skip to content

Commit 1841bcd

Browse files
authored
[clangd] Update XRefs to support overridden ObjC methods (#127109)
- Support finding implementors of a protocol and discovering subclasses for ObjC interfaces via the implementations call - Support jumping to the overridden method when you trigger goto definition on an override - Properly find references to overridden methods
1 parent d57479c commit 1841bcd

File tree

3 files changed

+247
-0
lines changed

3 files changed

+247
-0
lines changed

clang-tools-extra/clangd/XRefs.cpp

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,15 @@ void enhanceLocatedSymbolsFromIndex(llvm::MutableArrayRef<LocatedSymbol> Result,
372372
});
373373
}
374374

375+
bool objcMethodIsTouched(const SourceManager &SM, const ObjCMethodDecl *OMD,
376+
SourceLocation Loc) {
377+
unsigned NumSels = OMD->getNumSelectorLocs();
378+
for (unsigned I = 0; I < NumSels; ++I)
379+
if (SM.getSpellingLoc(OMD->getSelectorLoc(I)) == Loc)
380+
return true;
381+
return false;
382+
}
383+
375384
// Decls are more complicated.
376385
// The AST contains at least a declaration, maybe a definition.
377386
// These are up-to-date, and so generally preferred over index results.
@@ -430,6 +439,26 @@ locateASTReferent(SourceLocation CurLoc, const syntax::Token *TouchedIdentifier,
430439
continue;
431440
}
432441
}
442+
// Special case: - (void)^method {} should jump to overrides, but the decl
443+
// shouldn't, only the definition. Note that an Objective-C method can
444+
// override a parent class or protocol.
445+
//
446+
// FIXME: Support jumping from a protocol decl to overrides on go-to
447+
// definition.
448+
if (const auto *OMD = llvm::dyn_cast<ObjCMethodDecl>(D)) {
449+
if (OMD->isThisDeclarationADefinition() && TouchedIdentifier &&
450+
objcMethodIsTouched(SM, OMD, TouchedIdentifier->location())) {
451+
llvm::SmallVector<const ObjCMethodDecl *, 4> Overrides;
452+
OMD->getOverriddenMethods(Overrides);
453+
if (!Overrides.empty()) {
454+
for (const auto *Override : Overrides)
455+
AddResultDecl(Override);
456+
LocateASTReferentMetric.record(1, "objc-overriden-method");
457+
}
458+
AddResultDecl(OMD);
459+
continue;
460+
}
461+
}
433462

434463
// Special case: the cursor is on an alias, prefer other results.
435464
// This targets "using ns::^Foo", where the target is more interesting.
@@ -1283,6 +1312,12 @@ std::vector<LocatedSymbol> findImplementations(ParsedAST &AST, Position Pos,
12831312
} else if (const auto *RD = dyn_cast<CXXRecordDecl>(ND)) {
12841313
IDs.insert(getSymbolID(RD));
12851314
QueryKind = RelationKind::BaseOf;
1315+
} else if (const auto *OMD = dyn_cast<ObjCMethodDecl>(ND)) {
1316+
IDs.insert(getSymbolID(OMD));
1317+
QueryKind = RelationKind::OverriddenBy;
1318+
} else if (const auto *ID = dyn_cast<ObjCInterfaceDecl>(ND)) {
1319+
IDs.insert(getSymbolID(ID));
1320+
QueryKind = RelationKind::BaseOf;
12861321
}
12871322
}
12881323
return findImplementors(std::move(IDs), QueryKind, Index, AST.tuPath());
@@ -1302,6 +1337,21 @@ void getOverriddenMethods(const CXXMethodDecl *CMD,
13021337
}
13031338
}
13041339

1340+
// Recursively finds all the overridden methods of `OMD` in complete type
1341+
// hierarchy.
1342+
void getOverriddenMethods(const ObjCMethodDecl *OMD,
1343+
llvm::DenseSet<SymbolID> &OverriddenMethods) {
1344+
if (!OMD)
1345+
return;
1346+
llvm::SmallVector<const ObjCMethodDecl *, 4> Overrides;
1347+
OMD->getOverriddenMethods(Overrides);
1348+
for (const ObjCMethodDecl *Base : Overrides) {
1349+
if (auto ID = getSymbolID(Base))
1350+
OverriddenMethods.insert(ID);
1351+
getOverriddenMethods(Base, OverriddenMethods);
1352+
}
1353+
}
1354+
13051355
std::optional<std::string>
13061356
stringifyContainerForMainFileRef(const Decl *Container) {
13071357
// FIXME We might also want to display the signature here
@@ -1438,6 +1488,12 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
14381488
getOverriddenMethods(CMD, OverriddenMethods);
14391489
}
14401490
}
1491+
// Special case: Objective-C methods can override a parent class or
1492+
// protocol, we should be sure to report references to those.
1493+
if (const auto *OMD = llvm::dyn_cast<ObjCMethodDecl>(ND)) {
1494+
OverriddenBy.Subjects.insert(getSymbolID(OMD));
1495+
getOverriddenMethods(OMD, OverriddenMethods);
1496+
}
14411497
}
14421498
}
14431499

clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1335,6 +1335,42 @@ TEST_F(SymbolCollectorTest, OverrideRelationsMultipleInheritance) {
13351335
OverriddenBy(CBar, DBar), OverriddenBy(CBaz, DBaz)));
13361336
}
13371337

1338+
TEST_F(SymbolCollectorTest, ObjCOverrideRelationsSimpleInheritance) {
1339+
std::string Header = R"cpp(
1340+
@interface A
1341+
- (void)foo;
1342+
@end
1343+
@interface B : A
1344+
- (void)foo; // A::foo
1345+
- (void)bar;
1346+
@end
1347+
@interface C : B
1348+
- (void)bar; // B::bar
1349+
@end
1350+
@interface D : C
1351+
- (void)foo; // B::foo
1352+
- (void)bar; // C::bar
1353+
@end
1354+
)cpp";
1355+
runSymbolCollector(Header, /*Main=*/"",
1356+
{"-xobjective-c++", "-Wno-objc-root-class"});
1357+
const Symbol &AFoo = findSymbol(Symbols, "A::foo");
1358+
const Symbol &BFoo = findSymbol(Symbols, "B::foo");
1359+
const Symbol &DFoo = findSymbol(Symbols, "D::foo");
1360+
1361+
const Symbol &BBar = findSymbol(Symbols, "B::bar");
1362+
const Symbol &CBar = findSymbol(Symbols, "C::bar");
1363+
const Symbol &DBar = findSymbol(Symbols, "D::bar");
1364+
1365+
std::vector<Relation> Result;
1366+
for (const Relation &R : Relations)
1367+
if (R.Predicate == RelationKind::OverriddenBy)
1368+
Result.push_back(R);
1369+
EXPECT_THAT(Result, UnorderedElementsAre(
1370+
OverriddenBy(AFoo, BFoo), OverriddenBy(BBar, CBar),
1371+
OverriddenBy(BFoo, DFoo), OverriddenBy(CBar, DBar)));
1372+
}
1373+
13381374
TEST_F(SymbolCollectorTest, CountReferences) {
13391375
const std::string Header = R"(
13401376
class W;

clang-tools-extra/clangd/unittests/XRefsTests.cpp

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -411,6 +411,85 @@ TEST(LocateSymbol, FindOverrides) {
411411
sym("foo", Code.range("2"), std::nullopt)));
412412
}
413413

414+
TEST(LocateSymbol, FindOverridesFromDefObjC) {
415+
auto Code = Annotations(R"objc(
416+
@protocol Fooey
417+
- (void)foo;
418+
@end
419+
@interface Base
420+
- (void)foo;
421+
@end
422+
@interface Foo : Base<Fooey>
423+
- (void)$1[[foo]];
424+
@end
425+
426+
@interface Bar : Foo
427+
- (void)$2[[foo]];
428+
@end
429+
@implementation Bar
430+
- (void)$3[[fo^o]] {}
431+
@end
432+
)objc");
433+
TestTU TU = TestTU::withCode(Code.code());
434+
TU.ExtraArgs.push_back("-xobjective-c++");
435+
auto AST = TU.build();
436+
EXPECT_THAT(
437+
locateSymbolAt(AST, Code.point(), TU.index().get()),
438+
UnorderedElementsAre(sym("foo", Code.range("1"), std::nullopt),
439+
sym("foo", Code.range("2"), Code.range("3"))));
440+
}
441+
442+
TEST(LocateSymbol, NoOverridesFromDeclObjC) {
443+
auto Code = Annotations(R"objc(
444+
@protocol Fooey
445+
- (void)foo;
446+
@end
447+
@interface Base
448+
- (void)foo;
449+
@end
450+
@interface Foo : Base<Fooey>
451+
- (void)foo;
452+
@end
453+
454+
@interface Bar : Foo
455+
- (void)$2[[fo^o]];
456+
@end
457+
@implementation Bar
458+
- (void)$3[[foo]] {}
459+
@end
460+
)objc");
461+
TestTU TU = TestTU::withCode(Code.code());
462+
TU.ExtraArgs.push_back("-xobjective-c++");
463+
auto AST = TU.build();
464+
EXPECT_THAT(
465+
locateSymbolAt(AST, Code.point(), TU.index().get()),
466+
UnorderedElementsAre(sym("foo", Code.range("2"), Code.range("3"))));
467+
}
468+
469+
TEST(LocateSymbol, ObjCNoOverridesOnUsage) {
470+
auto Code = Annotations(R"objc(
471+
@interface Foo
472+
- (void)foo;
473+
@end
474+
475+
@interface Bar : Foo
476+
- (void)$1[[foo]];
477+
@end
478+
@implementation Bar
479+
- (void)$2[[foo]] {}
480+
@end
481+
void doSomething(Bar *bar) {
482+
[bar fo^o];
483+
}
484+
)objc");
485+
TestTU TU = TestTU::withCode(Code.code());
486+
TU.ExtraArgs.push_back("-xobjective-c++");
487+
auto AST = TU.build();
488+
EXPECT_THAT(
489+
locateSymbolAt(AST, Code.point(), TU.index().get()),
490+
UnorderedElementsAre(sym("foo", Code.range("1"), Code.range("2"))));
491+
}
492+
414493
TEST(LocateSymbol, WithIndexPreferredLocation) {
415494
Annotations SymbolHeader(R"cpp(
416495
class $p[[Proto]] {};
@@ -1834,6 +1913,41 @@ TEST(FindImplementations, Inheritance) {
18341913
}
18351914
}
18361915

1916+
TEST(FindImplementations, InheritanceObjC) {
1917+
llvm::StringRef Test = R"objc(
1918+
@interface $base^Base
1919+
- (void)fo$foo^o;
1920+
@end
1921+
@protocol Protocol
1922+
- (void)$protocol^protocol;
1923+
@end
1924+
@interface $ChildDecl[[Child]] : Base <Protocol>
1925+
- (void)concrete;
1926+
- (void)$fooDecl[[foo]];
1927+
@end
1928+
@implementation $ChildDef[[Child]]
1929+
- (void)concrete {}
1930+
- (void)$fooDef[[foo]] {}
1931+
- (void)$protocolDef[[protocol]] {}
1932+
@end
1933+
)objc";
1934+
1935+
Annotations Code(Test);
1936+
auto TU = TestTU::withCode(Code.code());
1937+
TU.ExtraArgs.push_back("-xobjective-c++");
1938+
auto AST = TU.build();
1939+
auto Index = TU.index();
1940+
EXPECT_THAT(findImplementations(AST, Code.point("base"), Index.get()),
1941+
UnorderedElementsAre(sym("Child", Code.range("ChildDecl"),
1942+
Code.range("ChildDef"))));
1943+
EXPECT_THAT(findImplementations(AST, Code.point("foo"), Index.get()),
1944+
UnorderedElementsAre(
1945+
sym("foo", Code.range("fooDecl"), Code.range("fooDef"))));
1946+
EXPECT_THAT(findImplementations(AST, Code.point("protocol"), Index.get()),
1947+
UnorderedElementsAre(sym("protocol", Code.range("protocolDef"),
1948+
Code.range("protocolDef"))));
1949+
}
1950+
18371951
TEST(FindImplementations, CaptureDefinition) {
18381952
llvm::StringRef Test = R"cpp(
18391953
struct Base {
@@ -1963,6 +2077,7 @@ void checkFindRefs(llvm::StringRef Test, bool UseIndex = false) {
19632077
Annotations T(Test);
19642078
auto TU = TestTU::withCode(T.code());
19652079
TU.ExtraArgs.push_back("-std=c++20");
2080+
TU.ExtraArgs.push_back("-xobjective-c++");
19662081

19672082
auto AST = TU.build();
19682083
std::vector<Matcher<ReferencesResult::Reference>> ExpectedLocations;
@@ -2260,6 +2375,25 @@ TEST(FindReferences, IncludeOverrides) {
22602375
checkFindRefs(Test, /*UseIndex=*/true);
22612376
}
22622377

2378+
TEST(FindReferences, IncludeOverridesObjC) {
2379+
llvm::StringRef Test =
2380+
R"objc(
2381+
@interface Base
2382+
- (void)$decl(Base)[[f^unc]];
2383+
@end
2384+
@interface Derived : Base
2385+
- (void)$overridedecl(Derived::func)[[func]];
2386+
@end
2387+
@implementation Derived
2388+
- (void)$overridedef[[func]] {}
2389+
@end
2390+
void test(Derived *derived, Base *base) {
2391+
[derived func]; // No references to the overrides.
2392+
[base $(test)[[func]]];
2393+
})objc";
2394+
checkFindRefs(Test, /*UseIndex=*/true);
2395+
}
2396+
22632397
TEST(FindReferences, RefsToBaseMethod) {
22642398
llvm::StringRef Test =
22652399
R"cpp(
@@ -2284,6 +2418,27 @@ TEST(FindReferences, RefsToBaseMethod) {
22842418
checkFindRefs(Test, /*UseIndex=*/true);
22852419
}
22862420

2421+
TEST(FindReferences, RefsToBaseMethodObjC) {
2422+
llvm::StringRef Test =
2423+
R"objc(
2424+
@interface BaseBase
2425+
- (void)$(BaseBase)[[func]];
2426+
@end
2427+
@interface Base : BaseBase
2428+
- (void)$(Base)[[func]];
2429+
@end
2430+
@interface Derived : Base
2431+
- (void)$decl(Derived)[[fu^nc]];
2432+
@end
2433+
void test(BaseBase *bb, Base *b, Derived *d) {
2434+
// refs to overridden methods in complete type hierarchy are reported.
2435+
[bb $(test)[[func]]];
2436+
[b $(test)[[func]]];
2437+
[d $(test)[[fu^nc]]];
2438+
})objc";
2439+
checkFindRefs(Test, /*UseIndex=*/true);
2440+
}
2441+
22872442
TEST(FindReferences, MainFileReferencesOnly) {
22882443
llvm::StringRef Test =
22892444
R"cpp(

0 commit comments

Comments
 (0)