[PATCH] D70480: [clangd] Use expansion location when the ref is inside macros.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: ilya-biryukov.
Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

Previously, xrefs has inconsistent behavior when the reference is inside
macro body:

- AST-based xrefs (for main file) uses the expansion location;
- our index uses the spelling location;

This patch makes our index use expansion location, which is consistent with
AST-based xrefs, and kythe as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70480

Files:
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp


Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -644,6 +644,24 @@
                                   HaveRanges(Header.ranges()))));
 }
 
+TEST_F(SymbolCollectorTest, RefsOnMacros) {
+  CollectorOpts.RefFilter = RefKind::All;
+  CollectorOpts.RefsInHeaders = true;
+  Annotations Header(R"(
+    #define TYPE(X) X
+    #define FOO Foo
+    class [[Foo]] {};
+    void test() {
+      TYPE([[Foo]]) foo;
+      [[FOO]] foo2;
+    }
+  )");
+  CollectorOpts.RefFilter = RefKind::All;
+  runSymbolCollector(Header.code(), "");
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
+                                  HaveRanges(Header.ranges()))));
+}
+
 TEST_F(SymbolCollectorTest, HeaderAsMainFile) {
   CollectorOpts.RefFilter = RefKind::All;
   Annotations Header(R"(
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -275,10 +275,9 @@
   // Mark D as referenced if this is a reference coming from the main file.
   // D may not be an interesting symbol, but it's cheaper to check at the end.
   auto &SM = ASTCtx->getSourceManager();
-  auto SpellingLoc = SM.getSpellingLoc(Loc);
   if (Opts.CountReferences &&
       (Roles & static_cast<unsigned>(index::SymbolRole::Reference)) &&
-      SM.getFileID(SpellingLoc) == SM.getMainFileID())
+      SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID())
     ReferencedDecls.insert(ND);
 
   auto ID = getSymbolID(ND);
@@ -312,8 +311,9 @@
     return true;
   // Do not store references to main-file symbols.
   if (CollectRef && !IsMainFileOnly && !isa<NamespaceDecl>(ND) &&
-      (Opts.RefsInHeaders || SM.getFileID(SpellingLoc) == SM.getMainFileID()))
-    DeclRefs[ND].emplace_back(SpellingLoc, Roles);
+      (Opts.RefsInHeaders ||
+       SM.getFileID(SM.getFileLoc(Loc)) == SM.getMainFileID()))
+    DeclRefs[ND].emplace_back(SM.getFileLoc(Loc), Roles);
   // Don't continue indexing if this is a mere reference.
   if (IsOnlyRef)
     return true;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D70480.230209.patch
Type: text/x-patch
Size: 2329 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191120/aa82039b/attachment.bin>