diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -80,16 +80,8 @@ return FD->getDefinition(); if (const auto *PD = dyn_cast(D)) return PD->getDefinition(); - // Objective-C classes can have three types of declarations: - // - // - forward declaration: @class MyClass; - // - definition declaration: @interface MyClass ... @end - // - implementation: @implementation MyClass ... @end - if (const auto *ID = dyn_cast(D)) { - if (const auto *IMD = ID->getImplementation()) - return IMD; - return ID->getDefinition(); - } + if (const auto *ID = dyn_cast(D)) + return ID->getImplementation(); // Only a single declaration is allowed. if (isa(D) || isa(D) || isa(D)) // except cases above @@ -235,6 +227,30 @@ return llvm::None; } +// A wrapper around `Decl::getCanonicalDecl` to support cases where Clang's +// definition of a canonical declaration doesn't match up to what a programmer +// would expect. For example, Objective-C classes can have three types of +// declarations: +// +// - forward declaration(s): @class MyClass; +// - definition declaration: @interface MyClass ... @end +// - implementation: @implementation MyClass ... @end +// +// Clang will consider the forward declaration to be the canonical declaration +// because it is first, but we actually want the class definition if it is +// available since that is what a programmer would consider the real definition +// to be. +const NamedDecl *getNonStandardCanonicalDecl(const NamedDecl *D) { + D = llvm::cast(D->getCanonicalDecl()); + + // Prefer Objective-C class definitions over the forward declaration. + if (const auto *ID = dyn_cast(D)) + if (const auto *DefinitionID = ID->getDefinition()) + return DefinitionID; + + return D; +} + // 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. @@ -250,7 +266,7 @@ llvm::DenseMap ResultIndex; auto AddResultDecl = [&](const NamedDecl *D) { - D = llvm::cast(D->getCanonicalDecl()); + D = getNonStandardCanonicalDecl(D); auto Loc = makeLocation(AST.getASTContext(), nameLocation(*D, SM), MainFilePath); if (!Loc) diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp --- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp +++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp @@ -688,7 +688,14 @@ R"objc(//objc @class $decl[[Foo]]; - @interface $def[[Foo]] + Fo^o * getFoo() { + return 0; + } + )objc", + + R"objc(//objc + @class Foo; + @interface $decl[[Foo]] @end Fo^o * getFoo() { return 0; @@ -696,8 +703,8 @@ )objc", R"objc(//objc - @class $decl[[Foo]]; - @interface Foo + @class Foo; + @interface $decl[[Foo]] @end @implementation $def[[Foo]] @end