[clangd] Use identifier range as the definition range.
authorHaojian Wu <hokein@google.com>
Fri, 9 Mar 2018 14:00:34 +0000 (14:00 +0000)
committerHaojian Wu <hokein@google.com>
Fri, 9 Mar 2018 14:00:34 +0000 (14:00 +0000)
Summary: This also matches the range in symbol index.

Reviewers: sammccall

Subscribers: klimek, ilya-biryukov, jkorous-apple, ioeric, cfe-commits

Differential Revision: https://reviews.llvm.org/D44247

llvm-svn: 327129

clang-tools-extra/clangd/AST.cpp [new file with mode: 0644]
clang-tools-extra/clangd/AST.h [new file with mode: 0644]
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/unittests/clangd/ClangdTests.cpp
clang-tools-extra/unittests/clangd/XRefsTests.cpp

diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp
new file mode 100644 (file)
index 0000000..5d549c3
--- /dev/null
@@ -0,0 +1,42 @@
+//===--- AST.cpp - Utility AST functions  -----------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "AST.h"
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/Basic/SourceManager.h"
+
+namespace clang {
+namespace clangd {
+using namespace llvm;
+
+SourceLocation findNameLoc(const clang::Decl* D) {
+  const auto& SM = D->getASTContext().getSourceManager();
+  // FIXME: Revisit the strategy, the heuristic is limitted when handling
+  // macros, we should use the location where the whole definition occurs.
+  SourceLocation SpellingLoc = SM.getSpellingLoc(D->getLocation());
+  if (D->getLocation().isMacroID()) {
+    std::string PrintLoc = SpellingLoc.printToString(SM);
+    if (llvm::StringRef(PrintLoc).startswith("<scratch") ||
+        llvm::StringRef(PrintLoc).startswith("<command line>")) {
+      // We use the expansion location for the following symbols, as spelling
+      // locations of these symbols are not interesting to us:
+      //   * symbols formed via macro concatenation, the spelling location will
+      //     be "<scratch space>"
+      //   * symbols controlled and defined by a compile command-line option
+      //     `-DName=foo`, the spelling location will be "<command line>".
+      SpellingLoc = SM.getExpansionRange(D->getLocation()).first;
+    }
+  }
+  return SpellingLoc;
+}
+
+} // namespace clangd
+} // namespace clang
diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h
new file mode 100644 (file)
index 0000000..b8cf074
--- /dev/null
@@ -0,0 +1,34 @@
+//===--- AST.h - Utility AST functions  -------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// Various code that examines C++ source code using AST.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H_
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H_
+
+#include "clang/Basic/SourceLocation.h"
+
+namespace clang {
+class SourceManager;
+class Decl;
+
+namespace clangd {
+
+/// Find the identifier source location of the given D.
+///
+/// The returned location is usually the spelling location where the name of the
+/// decl occurs in the code.
+SourceLocation findNameLoc(const clang::Decl* D);
+
+} // namespace clangd
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H_
index d12a6b3..5b613a6 100644 (file)
@@ -3,6 +3,7 @@ set(LLVM_LINK_COMPONENTS
   )
 
 add_clang_library(clangDaemon
+  AST.cpp
   ClangdLSPServer.cpp
   ClangdServer.cpp
   ClangdUnit.cpp
index 0744a4f..0f9b8a5 100644 (file)
@@ -7,6 +7,7 @@
 //
 //===---------------------------------------------------------------------===//
 #include "XRefs.h"
+#include "AST.h"
 #include "Logger.h"
 #include "SourceCode.h"
 #include "URI.h"
@@ -133,7 +134,7 @@ private:
 };
 
 llvm::Optional<Location>
-getDeclarationLocation(ParsedAST &AST, const SourceRange &ValSourceRange) {
+makeLocation(ParsedAST &AST, const SourceRange &ValSourceRange) {
   const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
   const LangOptions &LangOpts = AST.getASTContext().getLangOpts();
   SourceLocation LocStart = ValSourceRange.getBegin();
@@ -200,8 +201,9 @@ std::vector<Location> findDefinitions(ParsedAST &AST, Position Pos) {
   std::vector<const Decl *> Decls = DeclMacrosFinder->takeDecls();
   std::vector<MacroDecl> MacroInfos = DeclMacrosFinder->takeMacroInfos();
 
-  for (auto Item : Decls) {
-    auto L = getDeclarationLocation(AST, Item->getSourceRange());
+  for (auto D : Decls) {
+    auto Loc = findNameLoc(D);
+    auto L = makeLocation(AST, SourceRange(Loc, Loc));
     if (L)
       Result.push_back(*L);
   }
@@ -209,7 +211,7 @@ std::vector<Location> findDefinitions(ParsedAST &AST, Position Pos) {
   for (auto Item : MacroInfos) {
     SourceRange SR(Item.Info->getDefinitionLoc(),
                    Item.Info->getDefinitionEndLoc());
-    auto L = getDeclarationLocation(AST, SR);
+    auto L = makeLocation(AST, SR);
     if (L)
       Result.push_back(*L);
   }
index 93bd3f7..e237e13 100644 (file)
@@ -8,6 +8,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "SymbolCollector.h"
+#include "../AST.h"
 #include "../CodeCompletionStrings.h"
 #include "../Logger.h"
 #include "../URI.h"
@@ -184,30 +185,16 @@ getIncludeHeader(llvm::StringRef QName, const SourceManager &SM,
 llvm::Optional<SymbolLocation> getSymbolLocation(
     const NamedDecl &D, SourceManager &SM, const SymbolCollector::Options &Opts,
     const clang::LangOptions &LangOpts, std::string &FileURIStorage) {
-  SourceLocation SpellingLoc = SM.getSpellingLoc(D.getLocation());
-  if (D.getLocation().isMacroID()) {
-    std::string PrintLoc = SpellingLoc.printToString(SM);
-    if (llvm::StringRef(PrintLoc).startswith("<scratch") ||
-        llvm::StringRef(PrintLoc).startswith("<command line>")) {
-      // We use the expansion location for the following symbols, as spelling
-      // locations of these symbols are not interesting to us:
-      //   * symbols formed via macro concatenation, the spelling location will
-      //     be "<scratch space>"
-      //   * symbols controlled and defined by a compile command-line option
-      //     `-DName=foo`, the spelling location will be "<command line>".
-      SpellingLoc = SM.getExpansionRange(D.getLocation()).first;
-    }
-  }
-
-  auto U = toURI(SM, SM.getFilename(SpellingLoc), Opts);
+  SourceLocation NameLoc = findNameLoc(&D);
+  auto U = toURI(SM, SM.getFilename(NameLoc), Opts);
   if (!U)
     return llvm::None;
   FileURIStorage = std::move(*U);
   SymbolLocation Result;
   Result.FileURI = FileURIStorage;
-  Result.StartOffset = SM.getFileOffset(SpellingLoc);
+  Result.StartOffset = SM.getFileOffset(NameLoc);
   Result.EndOffset = Result.StartOffset + clang::Lexer::MeasureTokenLength(
-                                              SpellingLoc, SM, LangOpts);
+                                              NameLoc, SM, LangOpts);
   return std::move(Result);
 }
 
index 1102d2c..a09d49a 100644 (file)
@@ -435,9 +435,9 @@ int main() { return 0; }
 TEST_F(ClangdVFSTest, ReparseOpenedFiles) {
   Annotations FooSource(R"cpp(
 #ifdef MACRO
-$one[[static void bob() {}]]
+static void $one[[bob]]() {}
 #else
-$two[[static void bob() {}]]
+static void $two[[bob]]() {}
 #endif
 
 int main () { bo^b (); return 0; }
index c2dd053..2617b4a 100644 (file)
@@ -119,7 +119,7 @@ TEST(GoToDefinition, All) {
   const char *Tests[] = {
       R"cpp(// Local variable
         int main() {
-          [[int bonjour]];
+          int [[bonjour]];
           ^bonjour = 2;
           int test1 = bonjour;
         }
@@ -127,7 +127,7 @@ TEST(GoToDefinition, All) {
 
       R"cpp(// Struct
         namespace ns1 {
-        [[struct MyClass {}]];
+        struct [[MyClass]] {};
         } // namespace ns1
         int main() {
           ns1::My^Class* Params;
@@ -135,21 +135,21 @@ TEST(GoToDefinition, All) {
       )cpp",
 
       R"cpp(// Function definition via pointer
-        [[int foo(int) {}]]
+        int [[foo]](int) {}
         int main() {
           auto *X = &^foo;
         }
       )cpp",
 
       R"cpp(// Function declaration via call
-        [[int foo(int)]];
+        int [[foo]](int);
         int main() {
           return ^foo(42);
         }
       )cpp",
 
       R"cpp(// Field
-        struct Foo { [[int x]]; };
+        struct Foo { int [[x]]; };
         int main() {
           Foo bar;
           bar.^x;
@@ -158,27 +158,27 @@ TEST(GoToDefinition, All) {
 
       R"cpp(// Field, member initializer
         struct Foo {
-          [[int x]];
+          int [[x]];
           Foo() : ^x(0) {}
         };
       )cpp",
 
       R"cpp(// Field, GNU old-style field designator
-        struct Foo { [[int x]]; };
+        struct Foo { int [[x]]; };
         int main() {
           Foo bar = { ^x : 1 };
         }
       )cpp",
 
       R"cpp(// Field, field designator
-        struct Foo { [[int x]]; };
+        struct Foo { int [[x]]; };
         int main() {
           Foo bar = { .^x = 2 };
         }
       )cpp",
 
       R"cpp(// Method call
-        struct Foo { [[int x()]]; };
+        struct Foo { int [[x]](); };
         int main() {
           Foo bar;
           bar.^x();
@@ -186,7 +186,7 @@ TEST(GoToDefinition, All) {
       )cpp",
 
       R"cpp(// Typedef
-        [[typedef int Foo]];
+        typedef int [[Foo]];
         int main() {
           ^Foo bar;
         }
@@ -199,9 +199,9 @@ TEST(GoToDefinition, All) {
       )cpp", */
 
       R"cpp(// Namespace
-        [[namespace ns {
+        namespace [[ns]] {
         struct Foo { static void bar(); }
-        }]] // namespace ns
+        } // namespace ns
         int main() { ^ns::Foo::bar(); }
       )cpp",
 
@@ -215,14 +215,26 @@ TEST(GoToDefinition, All) {
 
       R"cpp(// Forward class declaration
         class Foo;
-        [[class Foo {}]];
+        class [[Foo]] {};
         F^oo* foo();
       )cpp",
 
       R"cpp(// Function declaration
         void foo();
         void g() { f^oo(); }
-        [[void foo() {}]]
+        void [[foo]]() {}
+      )cpp",
+
+      R"cpp(
+        #define FF(name) class name##_Test {};
+        [[FF]](my);
+        void f() { my^_Test a; }
+      )cpp",
+
+      R"cpp(
+         #define FF() class [[Test]] {};
+         FF();
+         void f() { T^est a; }
       )cpp",
   };
   for (const char *Test : Tests) {
@@ -236,7 +248,7 @@ TEST(GoToDefinition, All) {
 
 TEST(GoToDefinition, RelPathsInCompileCommand) {
   Annotations SourceAnnotations(R"cpp(
-[[int foo]];
+int [[foo]];
 int baz = f^oo;
 )cpp");