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


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;
   }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to