diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -405,15 +405,17 @@ log("locateSymbolNamedTextuallyAt: {0}", MaybeDeclLoc.takeError()); return; } - Location DeclLoc = *MaybeDeclLoc; - Location DefLoc; + LocatedSymbol Located; + Located.PreferredDeclaration = *MaybeDeclLoc; + Located.Name = (Sym.Name + Sym.TemplateSpecializationArgs).str(); if (Sym.Definition) { auto MaybeDefLoc = indexToLSPLocation(Sym.Definition, MainFilePath); if (!MaybeDefLoc) { log("locateSymbolNamedTextuallyAt: {0}", MaybeDefLoc.takeError()); return; } - DefLoc = *MaybeDefLoc; + Located.PreferredDeclaration = *MaybeDefLoc; + Located.Definition = *MaybeDefLoc; } if (ScoredResults.size() >= 3) { @@ -424,11 +426,6 @@ return; } - LocatedSymbol Located; - Located.Name = (Sym.Name + Sym.TemplateSpecializationArgs).str(); - Located.PreferredDeclaration = bool(Sym.Definition) ? DefLoc : DeclLoc; - Located.Definition = DefLoc; - SymbolQualitySignals Quality; Quality.merge(Sym); SymbolRelevanceSignals Relevance; diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp --- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp +++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp @@ -41,6 +41,7 @@ using ::testing::Eq; using ::testing::IsEmpty; using ::testing::Matcher; +using ::testing::UnorderedElementsAre; using ::testing::UnorderedElementsAreArray; MATCHER_P2(FileRange, File, Range, "") { @@ -264,18 +265,24 @@ << llvm::to_string(arg.PreferredDeclaration); return false; } + if (!Def && !arg.Definition) + return true; if (Def && !arg.Definition) { *result_listener << "Has no definition"; return false; } - if (Def && arg.Definition->range != *Def) { + if (!Def && arg.Definition) { + *result_listener << "Definition is " << llvm::to_string(arg.Definition); + return false; + } + if (arg.Definition->range != *Def) { *result_listener << "Definition is " << llvm::to_string(arg.Definition); return false; } return true; } ::testing::Matcher Sym(std::string Name, Range Decl) { - return Sym(Name, Decl, llvm::None); + return Sym(Name, Decl, Decl); } MATCHER_P(Sym, Name, "") { return arg.Name == Name; } @@ -891,18 +898,20 @@ // FIXME: Target the constructor as well. EXPECT_THAT(locateSymbolAt(AST, T.point("9")), ElementsAre(Sym("Foo"))); EXPECT_THAT(locateSymbolAt(AST, T.point("10")), - ElementsAre(Sym("Foo", T.range("ConstructorLoc")))); + ElementsAre(Sym("Foo", T.range("ConstructorLoc"), llvm::None))); EXPECT_THAT(locateSymbolAt(AST, T.point("11")), - ElementsAre(Sym("Foo", T.range("ConstructorLoc")))); + ElementsAre(Sym("Foo", T.range("ConstructorLoc"), llvm::None))); // These assertions are unordered because the order comes from // CXXRecordDecl::lookupDependentName() which doesn't appear to provide // an order guarantee. EXPECT_THAT(locateSymbolAt(AST, T.point("12")), - UnorderedElementsAre(Sym("bar", T.range("NonstaticOverload1")), - Sym("bar", T.range("NonstaticOverload2")))); - EXPECT_THAT(locateSymbolAt(AST, T.point("13")), - UnorderedElementsAre(Sym("baz", T.range("StaticOverload1")), - Sym("baz", T.range("StaticOverload2")))); + UnorderedElementsAre( + Sym("bar", T.range("NonstaticOverload1"), llvm::None), + Sym("bar", T.range("NonstaticOverload2"), llvm::None))); + EXPECT_THAT( + locateSymbolAt(AST, T.point("13")), + UnorderedElementsAre(Sym("baz", T.range("StaticOverload1"), llvm::None), + Sym("baz", T.range("StaticOverload2"), llvm::None))); } TEST(LocateSymbol, TextualDependent) { @@ -932,9 +941,10 @@ // interaction between locateASTReferent() and // locateSymbolNamedTextuallyAt(). auto Results = locateSymbolAt(AST, Source.point(), Index.get()); - EXPECT_THAT(Results, UnorderedElementsAre( - Sym("uniqueMethodName", Header.range("FooLoc")), - Sym("uniqueMethodName", Header.range("BarLoc")))); + EXPECT_THAT(Results, + UnorderedElementsAre( + Sym("uniqueMethodName", Header.range("FooLoc"), llvm::None), + Sym("uniqueMethodName", Header.range("BarLoc"), llvm::None))); } TEST(LocateSymbol, TemplateTypedefs) { @@ -1112,7 +1122,7 @@ // stale one. EXPECT_THAT( cantFail(runLocateSymbolAt(Server, FooCpp, FooWithoutHeader.point())), - ElementsAre(Sym("foo", FooWithoutHeader.range()))); + ElementsAre(Sym("foo", FooWithoutHeader.range(), llvm::None))); // Reset test environment. runAddDocument(Server, FooCpp, FooWithHeader.code()); @@ -1122,7 +1132,7 @@ // Use the AST being built in above request. EXPECT_THAT( cantFail(runLocateSymbolAt(Server, FooCpp, FooWithoutHeader.point())), - ElementsAre(Sym("foo", FooWithoutHeader.range()))); + ElementsAre(Sym("foo", FooWithoutHeader.range(), llvm::None))); } TEST(LocateSymbol, NearbyTokenSmoke) {