[clang-tools-extra] r322370 - [clangd] Don't navigate to forward class declaration when go to definition.

[Haojian Wu via cfe-commits]

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);