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