[PATCH] D73369: [clangd] Simplify "preferred" vs "definition" logic a bit in XRefs AST code.

[Sam McCall via Phabricator via cfe-commits]

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

Now Preferred is always the canonical (first) decl, Definition is always the def
if available.

In practice the index was already forcing this behaviour anyway, so there's no
change. (Unless you weren't using this index, in which case this patch makes
textDocument/declaration and toggling work as expected).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73369

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -337,15 +337,15 @@
       )cpp",
 
       R"cpp(// Forward class declaration
-        class Foo;
-        class [[Foo]] {};
+        class $decl[[Foo]];
+        class $def[[Foo]] {};
         F^oo* foo();
       )cpp",
 
       R"cpp(// Function declaration
-        void foo();
+        void $decl[[foo]]();
         void g() { f^oo(); }
-        void [[foo]]() {}
+        void $def[[foo]]() {}
       )cpp",
 
       R"cpp(
Index: clang-tools-extra/clangd/XRefs.cpp
===================================================================
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -254,20 +254,20 @@
       DeclRelation::TemplatePattern | DeclRelation::Alias;
   for (const NamedDecl *D : getDeclAtPosition(AST, SourceLoc, Relations)) {
     const NamedDecl *Def = getDefinition(D);
-    const NamedDecl *Preferred = Def ? Def : D;
+    if (const NamedDecl *C = llvm::dyn_cast<NamedDecl>(D->getCanonicalDecl()))
+      D = C;
 
     // If we're at the point of declaration of a template specialization,
     // it's more useful to navigate to the template declaration.
-    if (SM.getMacroArgExpandedLocation(Preferred->getLocation()) ==
+    if (SM.getMacroArgExpandedLocation(D->getLocation()) ==
         IdentStartLoc) {
-      if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(Preferred)) {
+      if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(D)) {
         D = CTSD->getSpecializedTemplate();
         Def = getDefinition(D);
-        Preferred = Def ? Def : D;
       }
     }
 
-    auto Loc = makeLocation(AST.getASTContext(), nameLocation(*Preferred, SM),
+    auto Loc = makeLocation(AST.getASTContext(), nameLocation(*D, SM),
                             *MainFilePath);
     if (!Loc)
       continue;
@@ -275,12 +275,12 @@
     Result.emplace_back();
     Result.back().Name = printName(AST.getASTContext(), *D);
     Result.back().PreferredDeclaration = *Loc;
-    // Preferred is always a definition if possible, so this check works.
-    if (Def == Preferred)
-      Result.back().Definition = *Loc;
+    if (Def)
+      Result.back().Definition = makeLocation(
+          AST.getASTContext(), nameLocation(*Def, SM), *MainFilePath);
 
     // Record SymbolID for index lookup later.
-    if (auto ID = getSymbolID(Preferred))
+    if (auto ID = getSymbolID(D))
       ResultIndex[*ID] = Result.size() - 1;
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D73369.240231.patch
Type: text/x-patch
Size: 2628 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200124/1bd9d19c/attachment.bin>