diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -1339,14 +1339,23 @@ void ClangdLSPServer::onReference(const ReferenceParams &Params, Callback> Reply) { - Server->findReferences(Params.textDocument.uri.file(), Params.position, - Opts.CodeComplete.Limit, - [Reply = std::move(Reply)]( - llvm::Expected Refs) mutable { - if (!Refs) - return Reply(Refs.takeError()); - return Reply(std::move(Refs->References)); - }); + Server->findReferences( + Params.textDocument.uri.file(), Params.position, Opts.CodeComplete.Limit, + [Reply = std::move(Reply), + IncludeDecl(Params.context.includeDeclaration)]( + llvm::Expected Refs) mutable { + if (!Refs) + return Reply(Refs.takeError()); + // Filter out declarations if the client asked. + std::vector Result; + Result.reserve(Refs->References.size()); + for (auto &Ref : Refs->References) { + bool IsDecl = Ref.Attributes & ReferencesResult::Declaration; + if (IncludeDecl || !IsDecl) + Result.push_back(std::move(Ref.Loc)); + } + return Reply(std::move(Result)); + }); } void ClangdLSPServer::onGoToImplementation( diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -1472,8 +1472,13 @@ }; llvm::json::Value toJSON(const CallHierarchyOutgoingCall &); +struct ReferenceContext { + /// Include the declaration of the current symbol. + bool includeDeclaration = false; +}; + struct ReferenceParams : public TextDocumentPositionParams { - // For now, no options like context.includeDeclaration are supported. + ReferenceContext context; }; bool fromJSON(const llvm::json::Value &, ReferenceParams &, llvm::json::Path); diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp --- a/clang-tools-extra/clangd/Protocol.cpp +++ b/clang-tools-extra/clangd/Protocol.cpp @@ -1219,10 +1219,17 @@ O.map("direction", R.direction); } +bool fromJSON(const llvm::json::Value &Params, ReferenceContext &R, + llvm::json::Path P) { + llvm::json::ObjectMapper O(Params, P); + return O && O.mapOptional("includeDeclaration", R.includeDeclaration); +} + bool fromJSON(const llvm::json::Value &Params, ReferenceParams &R, llvm::json::Path P) { TextDocumentPositionParams &Base = R; - return fromJSON(Params, Base, P); + llvm::json::ObjectMapper O(Params, P); + return fromJSON(Params, Base, P) && O && O.mapOptional("context", R.context); } llvm::json::Value toJSON(SymbolTag Tag) { diff --git a/clang-tools-extra/clangd/XRefs.h b/clang-tools-extra/clangd/XRefs.h --- a/clang-tools-extra/clangd/XRefs.h +++ b/clang-tools-extra/clangd/XRefs.h @@ -79,9 +79,20 @@ Position Pos); struct ReferencesResult { - std::vector References; + // Bitmask describing whether the occurrence is a declaration, definition etc. + enum ReferenceAttributes : unsigned { + Declaration = 1 << 0, + Definition = 1 << 1, + }; + struct Reference { + Location Loc; + unsigned Attributes = 0; + }; + std::vector References; bool HasMore = false; }; +llvm::raw_ostream &operator<<(llvm::raw_ostream &, + const ReferencesResult::Reference &); /// Returns implementations at a specified \p Pos: /// - overrides for a virtual method; 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 @@ -1312,9 +1312,13 @@ auto Refs = IDToRefs.find(MacroSID); if (Refs != IDToRefs.end()) { for (const auto &Ref : Refs->second) { - Location Result; - Result.range = Ref.Rng; - Result.uri = URIMainFile; + ReferencesResult::Reference Result; + Result.Loc.range = Ref.Rng; + Result.Loc.uri = URIMainFile; + if (Ref.IsDefinition) { + Result.Attributes |= ReferencesResult::Declaration; + Result.Attributes |= ReferencesResult::Definition; + } Results.References.push_back(std::move(Result)); } } @@ -1367,9 +1371,15 @@ }), MainFileRefs.end()); for (const auto &Ref : MainFileRefs) { - Location Result; - Result.range = Ref.range(SM); - Result.uri = URIMainFile; + ReferencesResult::Reference Result; + Result.Loc.range = Ref.range(SM); + Result.Loc.uri = URIMainFile; + if (Ref.Role & static_cast(index::SymbolRole::Declaration)) + Result.Attributes |= ReferencesResult::Declaration; + // clang-index doesn't report definitions as declarations, but they are. + if (Ref.Role & static_cast(index::SymbolRole::Definition)) + Result.Attributes |= + ReferencesResult::Definition | ReferencesResult::Declaration; Results.References.push_back(std::move(Result)); } if (Index && Results.References.size() <= Limit) { @@ -1397,8 +1407,15 @@ // Avoid indexed results for the main file - the AST is authoritative. if (!LSPLoc || LSPLoc->uri.file() == *MainFilePath) return; - - Results.References.push_back(std::move(*LSPLoc)); + ReferencesResult::Reference Result; + Result.Loc = std::move(*LSPLoc); + if ((R.Kind & RefKind::Declaration) == RefKind::Declaration) + Result.Attributes |= ReferencesResult::Declaration; + // FIXME: our index should definitely store def | decl separately! + if ((R.Kind & RefKind::Definition) == RefKind::Definition) + Result.Attributes |= + ReferencesResult::Declaration | ReferencesResult::Definition; + Results.References.push_back(std::move(Result)); }); } if (Results.References.size() > Limit) { @@ -1469,6 +1486,16 @@ return OS; } +llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, + const ReferencesResult::Reference &R) { + OS << R.Loc; + if (R.Attributes & ReferencesResult::Declaration) + OS << " [decl]"; + if (R.Attributes & ReferencesResult::Definition) + OS << " [def]"; + return OS; +} + template static llvm::Optional declToHierarchyItem(const NamedDecl &ND) { ASTContext &Ctx = ND.getASTContext(); diff --git a/clang-tools-extra/clangd/test/references.test b/clang-tools-extra/clangd/test/references.test --- a/clang-tools-extra/clangd/test/references.test +++ b/clang-tools-extra/clangd/test/references.test @@ -1,28 +1,24 @@ # RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}} --- -{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"int x; int y = x;"}}} +{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{ + "uri":"test:///main.cpp", + "languageId":"cpp", + "version":1, + "text":"int x; int y = x;" +}}} --- -{"jsonrpc":"2.0","id":1,"method":"textDocument/references","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":0,"character":4}}} +{"jsonrpc":"2.0","id":1,"method":"textDocument/references","params":{ + "textDocument":{"uri":"test:///main.cpp"}, + "position":{"line":0,"character":4}, + "context":{"includeDeclaration": false} +}} # CHECK: "id": 1 # CHECK-NEXT: "jsonrpc": "2.0", # CHECK-NEXT: "result": [ # CHECK-NEXT: { # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 5, -# CHECK-NEXT: "line": 0 -# CHECK-NEXT: }, -# CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 4, -# CHECK-NEXT: "line": 0 -# CHECK-NEXT: } -# CHECK-NEXT: }, -# CHECK-NEXT: "uri": "{{.*}}/main.cpp" -# CHECK-NEXT: }, -# CHECK-NEXT: { -# CHECK-NEXT: "range": { -# CHECK-NEXT: "end": { # CHECK-NEXT: "character": 16, # CHECK-NEXT: "line": 0 # CHECK-NEXT: }, diff --git a/clang-tools-extra/clangd/unittests/PreambleTests.cpp b/clang-tools-extra/clangd/unittests/PreambleTests.cpp --- a/clang-tools-extra/clangd/unittests/PreambleTests.cpp +++ b/clang-tools-extra/clangd/unittests/PreambleTests.cpp @@ -415,6 +415,8 @@ } } +MATCHER_P(referenceRangeIs, R, "") { return arg.Loc.range == R; } + TEST(PreamblePatchTest, RefsToMacros) { struct { const char *const Baseline; @@ -450,9 +452,9 @@ ASSERT_TRUE(AST); const auto &SM = AST->getSourceManager(); - std::vector> ExpectedLocations; + std::vector> ExpectedLocations; for (const auto &R : Modified.ranges()) - ExpectedLocations.push_back(Field(&Location::range, R)); + ExpectedLocations.push_back(referenceRangeIs(R)); for (const auto &P : Modified.points()) { auto *MacroTok = AST->getTokens().spelledTokenAt(SM.getComposedLoc( 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 @@ -37,6 +37,7 @@ namespace clangd { namespace { +using ::testing::AllOf; using ::testing::ElementsAre; using ::testing::Eq; using ::testing::IsEmpty; @@ -290,7 +291,8 @@ MATCHER_P(Sym, Name, "") { return arg.Name == Name; } -MATCHER_P(RangeIs, R, "") { return arg.range == R; } +MATCHER_P(RangeIs, R, "") { return arg.Loc.range == R; } +MATCHER_P(AttrsAre, A, "") { return arg.Attributes == A; } TEST(LocateSymbol, WithIndex) { Annotations SymbolHeader(R"cpp( @@ -1688,11 +1690,34 @@ << Test; } +void checkFindRefs(llvm::StringRef Test, bool UseIndex = false) { + Annotations T(Test); + auto TU = TestTU::withCode(T.code()); + auto AST = TU.build(); + std::vector> ExpectedLocations; + for (const auto &R : T.ranges()) + ExpectedLocations.push_back(AllOf(RangeIs(R), AttrsAre(0u))); + // $def is actually shorthand for both definition and declaration. + // If we have cases that are definition-only, we should change this. + for (const auto &R : T.ranges("def")) + ExpectedLocations.push_back( + AllOf(RangeIs(R), AttrsAre(ReferencesResult::Definition | + ReferencesResult::Declaration))); + for (const auto &R : T.ranges("decl")) + ExpectedLocations.push_back( + AllOf(RangeIs(R), AttrsAre(ReferencesResult::Declaration))); + EXPECT_THAT( + findReferences(AST, T.point(), 0, UseIndex ? TU.index().get() : nullptr) + .References, + UnorderedElementsAreArray(ExpectedLocations)) + << Test; +} + TEST(FindReferences, WithinAST) { const char *Tests[] = { R"cpp(// Local variable int main() { - int [[foo]]; + int $def[[foo]]; [[^foo]] = 2; int test1 = [[foo]]; } @@ -1700,7 +1725,7 @@ R"cpp(// Struct namespace ns1 { - struct [[Foo]] {}; + struct $def[[Foo]] {}; } // namespace ns1 int main() { ns1::[[Fo^o]]* Params; @@ -1708,15 +1733,15 @@ )cpp", R"cpp(// Forward declaration - class [[Foo]]; - class [[Foo]] {}; + class $decl[[Foo]]; + class $def[[Foo]] {}; int main() { [[Fo^o]] foo; } )cpp", R"cpp(// Function - int [[foo]](int) {} + int $def[[foo]](int) {} int main() { auto *X = &[[^foo]]; [[foo]](42); @@ -1725,7 +1750,7 @@ R"cpp(// Field struct Foo { - int [[foo]]; + int $def[[foo]]; Foo() : [[foo]](0) {} }; int main() { @@ -1735,8 +1760,8 @@ )cpp", R"cpp(// Method call - struct Foo { int [[foo]](); }; - int Foo::[[foo]]() {} + struct Foo { int $decl[[foo]](); }; + int Foo::$def[[foo]]() {} int main() { Foo f; f.[[^foo]](); @@ -1745,7 +1770,7 @@ R"cpp(// Constructor struct Foo { - [[F^oo]](int); + $decl[[F^oo]](int); }; void foo() { Foo f = [[Foo]](42); @@ -1753,14 +1778,14 @@ )cpp", R"cpp(// Typedef - typedef int [[Foo]]; + typedef int $def[[Foo]]; int main() { [[^Foo]] bar; } )cpp", R"cpp(// Namespace - namespace [[ns]] { + namespace $decl[[ns]] { // FIXME: def? struct Foo {}; } // namespace ns int main() { [[^ns]]::Foo foo; } @@ -1770,7 +1795,7 @@ #define TYPE(X) X #define FOO Foo #define CAT(X, Y) X##Y - class [[Fo^o]] {}; + class $def[[Fo^o]] {}; void test() { TYPE([[Foo]]) foo; [[FOO]] foo2; @@ -1780,7 +1805,7 @@ )cpp", R"cpp(// Macros - #define [[MA^CRO]](X) (X+1) + #define $def[[MA^CRO]](X) (X+1) void test() { int x = [[MACRO]]([[MACRO]](1)); } @@ -1788,31 +1813,31 @@ R"cpp(// Macro outside preamble int breakPreamble; - #define [[MA^CRO]](X) (X+1) + #define $def[[MA^CRO]](X) (X+1) void test() { int x = [[MACRO]]([[MACRO]](1)); } )cpp", R"cpp( - int [[v^ar]] = 0; + int $def[[v^ar]] = 0; void foo(int s = [[var]]); )cpp", R"cpp( template - class [[Fo^o]] {}; + class $def[[Fo^o]] {}; void func([[Foo]]); )cpp", R"cpp( template - class [[Foo]] {}; + class $def[[Foo]] {}; void func([[Fo^o]]); )cpp", R"cpp(// Not touching any identifiers. struct Foo { - [[~]]Foo() {}; + $def[[~]]Foo() {}; }; void foo() { Foo f; @@ -1821,28 +1846,20 @@ )cpp", R"cpp(// Lambda capture initializer void foo() { - int [[w^aldo]] = 42; + int $def[[w^aldo]] = 42; auto lambda = [x = [[waldo]]](){}; } )cpp", R"cpp(// Renaming alias template class Vector {}; - using [[^X]] = Vector; + using $def[[^X]] = Vector; [[X]] x1; Vector x2; Vector y; )cpp", }; - for (const char *Test : Tests) { - Annotations T(Test); - auto AST = TestTU::withCode(T.code()).build(); - std::vector> ExpectedLocations; - for (const auto &R : T.ranges()) - ExpectedLocations.push_back(RangeIs(R)); - EXPECT_THAT(findReferences(AST, T.point(), 0).References, - ElementsAreArray(ExpectedLocations)) - << Test; - } + for (const char *Test : Tests) + checkFindRefs(Test); } TEST(FindReferences, IncludeOverrides) { @@ -1850,24 +1867,16 @@ R"cpp( class Base { public: - virtual void [[f^unc]]() = 0; + virtual void $decl[[f^unc]]() = 0; }; class Derived : public Base { public: - void [[func]]() override; + void $decl[[func]]() override; // FIXME: ref, not decl }; void test(Derived* D) { D->[[func]](); })cpp"; - Annotations T(Test); - auto TU = TestTU::withCode(T.code()); - auto AST = TU.build(); - std::vector> ExpectedLocations; - for (const auto &R : T.ranges()) - ExpectedLocations.push_back(RangeIs(R)); - EXPECT_THAT(findReferences(AST, T.point(), 0, TU.index().get()).References, - ElementsAreArray(ExpectedLocations)) - << Test; + checkFindRefs(Test, /*UseIndex=*/true); } TEST(FindReferences, MainFileReferencesOnly) { @@ -1886,7 +1895,7 @@ )cpp"; auto AST = TU.build(); - std::vector> ExpectedLocations; + std::vector> ExpectedLocations; for (const auto &R : Code.ranges()) ExpectedLocations.push_back(RangeIs(R)); EXPECT_THAT(findReferences(AST, Code.point(), 0).References, @@ -1897,7 +1906,7 @@ TEST(FindReferences, ExplicitSymbols) { const char *Tests[] = { R"cpp( - struct Foo { Foo* [[self]]() const; }; + struct Foo { Foo* $decl[[self]]() const; }; void f() { Foo foo; if (Foo* T = foo.[[^self]]()) {} // Foo member call expr. @@ -1907,7 +1916,7 @@ R"cpp( struct Foo { Foo(int); }; Foo f() { - int [[b]]; + int $def[[b]]; return [[^b]]; // Foo constructor expr. } )cpp", @@ -1915,18 +1924,18 @@ R"cpp( struct Foo {}; void g(Foo); - Foo [[f]](); + Foo $decl[[f]](); void call() { g([[^f]]()); // Foo constructor expr. } )cpp", R"cpp( - void [[foo]](int); - void [[foo]](double); + void $decl[[foo]](int); + void $decl[[foo]](double); namespace ns { - using ::[[fo^o]]; + using ::$decl[[fo^o]]; } )cpp", @@ -1936,23 +1945,14 @@ }; int test() { - X [[a]]; + X $def[[a]]; [[a]].operator bool(); if ([[a^]]) {} // ignore implicit conversion-operator AST node } )cpp", }; - for (const char *Test : Tests) { - Annotations T(Test); - auto AST = TestTU::withCode(T.code()).build(); - std::vector> ExpectedLocations; - for (const auto &R : T.ranges()) - ExpectedLocations.push_back(RangeIs(R)); - ASSERT_THAT(ExpectedLocations, Not(IsEmpty())); - EXPECT_THAT(findReferences(AST, T.point(), 0).References, - ElementsAreArray(ExpectedLocations)) - << Test; - } + for (const char *Test : Tests) + checkFindRefs(Test); } TEST(FindReferences, NeedsIndexForSymbols) { @@ -1968,7 +1968,7 @@ findReferences(AST, Main.point(), 0, /*Index=*/nullptr).References, ElementsAre(RangeIs(Main.range()))); Annotations IndexedMain(R"cpp( - int main() { [[f^oo]](); } + int [[foo]]() { return 42; } )cpp"); // References from indexed files are included. @@ -1978,7 +1978,10 @@ IndexedTU.HeaderCode = Header; EXPECT_THAT( findReferences(AST, Main.point(), 0, IndexedTU.index().get()).References, - ElementsAre(RangeIs(Main.range()), RangeIs(IndexedMain.range()))); + ElementsAre(RangeIs(Main.range()), + AllOf(RangeIs(IndexedMain.range()), + AttrsAre(ReferencesResult::Declaration | + ReferencesResult::Definition)))); auto LimitRefs = findReferences(AST, Main.point(), /*Limit*/ 1, IndexedTU.index().get()); EXPECT_EQ(1u, LimitRefs.References.size());