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

[sammccall]

Summary

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

[merge_guards_bot]

Unit tests: fail. 62173 tests passed, 5 failed and 815 were skipped.

failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: fail. clang-tidy found 0 errors and 1 warnings. 1 of them are added as review comments below (why?).

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[kadircet]

clang-tools-extra/clangd/XRefs.cpp

257

Is it possible for this check to ever fail? I think it is safe to just perform a llvm::cast instead of dyn_cast.
also why not perform this before getting definition?

278

maybe inline getDefinition to here

[kadircet]

also please fix clang-tidy and format warnings

[kadircet]

ping :)

[sammccall]

address comments, rebase

clang-tools-extra/clangd/XRefs.cpp

257

Done. (I thought there was such a case, but I was thinking about D vs OrigD in IndexDataConusmer, and this happens in one of the rare cases where D isn't the canonical decl of OrigD)

[sammccall]

clang-format

clang-tools-extra/clangd/XRefs.cpp

if (auto L = sourceLocationInMainFile(SM, Pos)) {
     return Result;
   }
 
   // Emit all symbol locations (declaration or definition) from AST.
   DeclRelationSet Relations =
       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;
 
     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;
   }
 
   // Now query the index for all Symbol IDs we found in the AST.
   if (Index && !ResultIndex.empty()) {
     LookupRequest QueryRequest;
     for (auto It : ResultIndex)
       QueryRequest.IDs.insert(It.first);

clang-tools-extra/clangd/unittests/XRefsTests.cpp

const char *Tests[] = {
 
       R"cpp(// Symbol concatenated inside macro (not supported)
        int *pi;
        #define POINTER(X) p ## X;
        int i = *POINTER(^i);
       )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(
         #define FF(name) class name##_Test {};
         [[FF]](my);
         void f() { my^_Test a; }
       )cpp",