[PATCH] D133979: [clangd] XRef functions treat visible class definitions as primary.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

This means go-to-type will take us to a class definition in most cases.
As an exception, if the class is declared in a header and defined in a CC file,
then these are treated as a decl/def pair.

This is (fairly) consistent with how the index behaves, though the index
benefits from aggregation across translation units so does a simpler "main file"
check rather than explicitly "is this a header".


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133979

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
@@ -568,8 +568,8 @@
       )cpp",
 
       R"cpp(// Forward class declaration
-        class $decl[[Foo]];
-        class $def[[Foo]] {};
+        class Foo;
+        class [[Foo]] {};
         F^oo* foo();
       )cpp",
 
@@ -1030,6 +1030,33 @@
     }
   }
 }
+
+TEST(LocateSymbol, TypeForwardDeclaredInHeader) {
+  Annotations Code(R"cpp(
+    class [[X]] {};
+    ^X *x;
+  )cpp");
+
+  TestTU TU;
+  TU.HeaderCode = "class X;";
+  TU.Code = Code.code();
+
+  auto AST = TU.build();
+  auto Index = TU.index();
+  LocatedSymbol S = locateSymbolAt(AST, Code.point(), Index.get()).front();
+  EXPECT_EQ(S.Name, "X");
+  auto Filename = [&](const Location &L) {
+    return llvm::sys::path::filename(L.uri.file());
+  };
+  EXPECT_EQ(Filename(S.PreferredDeclaration), TU.HeaderFilename);
+  EXPECT_EQ(Filename(*S.Definition), TU.Filename);
+
+  S = findType(AST, Code.point()).front();
+  EXPECT_EQ(S.Name, "X");
+  EXPECT_EQ(Filename(S.PreferredDeclaration), TU.HeaderFilename);
+  EXPECT_EQ(Filename(*S.Definition), TU.Filename);
+}
+
 TEST(LocateSymbol, ValidSymbolID) {
   auto T = Annotations(R"cpp(
     #define MACRO(x, y) ((x) + (y))
Index: clang-tools-extra/clangd/XRefs.cpp
===================================================================
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -283,6 +283,30 @@
   // instead of failing.
   D = llvm::cast<NamedDecl>(D->getCanonicalDecl());
 
+  // Prefer C++ class definitions over a forward declaration.
+  if (const auto *TD = llvm::dyn_cast<TagDecl>(D)) {
+    if (const auto *Def = TD->getDefinition()) {
+      // Exception: forward decl is exposed in a header, definition is not.
+      // (This exception matches the spirit of our indexing heuristics).
+      if (/*PreferDef=*/[&] {
+        if (Def == TD)
+          return true;
+        const auto &SM = D->getASTContext().getSourceManager();
+        FileID DefFile = SM.getDecomposedExpansionLoc(Def->getLocation()).first;
+        if (DefFile != SM.getMainFileID())
+          return true; // definition is in a header.
+        FileID DeclFile = SM.getDecomposedExpansionLoc(D->getLocation()).first;
+        if (DefFile == DeclFile)
+          return true; // def/decl are equally visible.
+        // We've established that the definition is in the main file, and the
+        // decl is some other header.
+        return isHeaderFile(SM.getFileEntryRefForID(DefFile)->getName(),
+                            D->getASTContext().getLangOpts());
+      }())
+        D = Def;
+    }
+  }
+
   // Prefer Objective-C class/protocol definitions over the forward declaration.
   if (const auto *ID = dyn_cast<ObjCInterfaceDecl>(D))
     if (const auto *DefinitionID = ID->getDefinition())


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D133979.460511.patch
Type: text/x-patch
Size: 2974 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220915/ce4f4749/attachment.bin>