Author: hokein Date: Fri Jan 12 06:21:10 2018 New Revision: 322370 URL: http://llvm.org/viewvc/llvm-project?rev=322370&view=rev Log: [clangd] Don't navigate to forward class declaration when go to definition.
Summary: For some cases, GoToDefinition will navigate to the forward class declaration, we should always navigate to the class definition. Reviewers: sammccall Reviewed By: sammccall Subscribers: klimek, ilya-biryukov, cfe-commits Differential Revision: https://reviews.llvm.org/D41661 Modified: clang-tools-extra/trunk/clangd/XRefs.cpp clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp Modified: clang-tools-extra/trunk/clangd/XRefs.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=322370&r1=322369&r2=322370&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/XRefs.cpp (original) +++ clang-tools-extra/trunk/clangd/XRefs.cpp Fri Jan 12 06:21:10 2018 @@ -14,6 +14,20 @@ namespace clangd { using namespace llvm; namespace { +// Get the definition from a given declaration `D`. +// Return nullptr if no definition is found, or the declaration type of `D` is +// not supported. +const Decl* GetDefinition(const Decl* D) { + assert(D); + if (const auto *TD = dyn_cast<TagDecl>(D)) + return TD->getDefinition(); + else if (const auto *VD = dyn_cast<VarDecl>(D)) + return VD->getDefinition(); + else if (const auto *FD = dyn_cast<FunctionDecl>(D)) + return FD->getDefinition(); + return nullptr; +} + /// Finds declarations locations that a given source location refers to. class DeclarationAndMacrosFinder : public index::IndexDataConsumer { std::vector<const Decl *> Decls; @@ -50,8 +64,18 @@ public: ArrayRef<index::SymbolRelation> Relations, FileID FID, unsigned Offset, index::IndexDataConsumer::ASTNodeInfo ASTNode) override { - if (isSearchedLocation(FID, Offset)) - Decls.push_back(D); + if (isSearchedLocation(FID, Offset)) { + // Find and add definition declarations (for GoToDefinition). + // We don't use parameter `D`, as Parameter `D` is the canonical + // declaration, which is the first declaration of a redeclarable + // declaration, and it could be a forward declaration. + if (const auto* Def = GetDefinition(D)) { + Decls.push_back(Def); + } else { + // Couldn't find a definition, fall back to use `D`. + Decls.push_back(D); + } + } return true; } Modified: clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp?rev=322370&r1=322369&r2=322370&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp (original) +++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp Fri Jan 12 06:21:10 2018 @@ -203,6 +203,18 @@ TEST(GoToDefinition, All) { #define MACRO 2 #undef macro )cpp", + + R"cpp(// Forward class declaration + class Foo; + [[class Foo {}]]; + F^oo* foo(); + )cpp", + + R"cpp(// Function declaration + void foo(); + void g() { f^oo(); } + [[void foo() {}]] + )cpp", }; for (const char *Test : Tests) { Annotations T(Test); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
