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 @@ -872,6 +872,7 @@ struct Reference { syntax::Token SpelledTok; index::SymbolRoleSet Role; + SymbolID Target; Range range(const SourceManager &SM) const { return halfOpenToRange(SM, SpelledTok.range(SM).toCharRange(SM)); @@ -906,13 +907,15 @@ SourceLocation Loc, index::IndexDataConsumer::ASTNodeInfo ASTNode) override { const SourceManager &SM = AST.getSourceManager(); - if (!isInsideMainFile(Loc, SM) || - TargetIDs.find(getSymbolID(D)) == TargetIDs.end()) + if (!isInsideMainFile(Loc, SM)) + return true; + SymbolID ID = getSymbolID(D); + if (!TargetIDs.contains(ID)) return true; const auto &TB = AST.getTokens(); Loc = SM.getFileLoc(Loc); if (const auto *Tok = TB.spelledTokenAt(Loc)) - References.push_back({*Tok, Roles}); + References.push_back({*Tok, Roles, ID}); return true; } @@ -1297,7 +1300,7 @@ return {}; } - RefsRequest Req; + llvm::DenseSet IDs, Overrides; const auto *IdentifierAtCursor = syntax::spelledIdentifierTouching(*CurLoc, AST.getTokens()); @@ -1322,7 +1325,7 @@ Results.References.push_back(std::move(Result)); } } - Req.IDs.insert(MacroSID); + IDs.insert(MacroSID); } } else { // Handle references to Decls. @@ -1336,7 +1339,6 @@ if (auto ID = getSymbolID(D)) Targets.insert(ID); - llvm::DenseSet Overrides; if (Index) { RelationsRequest FindOverrides; FindOverrides.Predicate = RelationKind::OverriddenBy; @@ -1374,16 +1376,18 @@ 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; + // Overrides are always considered references, not defs/decls. + if (!Overrides.contains(Ref.Target)) { + 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) { - Req.IDs = std::move(Overrides); for (const Decl *D : Decls) { // Not all symbols can be referenced from outside (e.g. // function-locals). @@ -1392,13 +1396,17 @@ if (D->getParentFunctionOrMethod()) continue; if (auto ID = getSymbolID(D)) - Req.IDs.insert(ID); + IDs.insert(ID); } } } // Now query the index for references from other files. - if (!Req.IDs.empty() && Index && Results.References.size() <= Limit) { + auto QueryIndex = [&](llvm::DenseSet IDs, bool AllowAttributes) { + RefsRequest Req; + Req.IDs = std::move(IDs); Req.Limit = Limit; + if (Req.IDs.empty() || !Index || Results.References.size() > Limit) + return; Results.HasMore |= Index->refs(Req, [&](const Ref &R) { // No need to continue process if we reach the limit. if (Results.References.size() > Limit) @@ -1409,15 +1417,21 @@ return; 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; + if (AllowAttributes) { + 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)); }); - } + }; + QueryIndex(std::move(IDs), /*AllowAttributes=*/true); + // FIXME: Currently we surface decls/defs/refs of derived methods. + // Maybe we'd prefer decls/defs of derived methods, and refs of base methods? + QueryIndex(std::move(Overrides), /*AllowAttributes=*/false); if (Results.References.size() > Limit) { Results.HasMore = true; Results.References.resize(Limit); diff --git a/clang-tools-extra/clangd/index/Index.h b/clang-tools-extra/clangd/index/Index.h --- a/clang-tools-extra/clangd/index/Index.h +++ b/clang-tools-extra/clangd/index/Index.h @@ -104,11 +104,12 @@ lookup(const LookupRequest &Req, llvm::function_ref Callback) const = 0; - /// Finds all occurrences (e.g. references, declarations, definitions) of a - /// symbol and applies \p Callback on each result. + /// Finds all occurrences (e.g. references, declarations, definitions) of + /// symbols and applies \p Callback on each result. /// /// Results should be returned in arbitrary order. /// The returned result must be deep-copied if it's used outside Callback. + /// FIXME: there's no indication which result references which symbol. /// /// Returns true if there will be more results (limited by Req.Limit); virtual bool refs(const RefsRequest &Req, 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 @@ -1871,7 +1871,7 @@ }; class Derived : public Base { public: - void $decl[[func]]() override; // FIXME: ref, not decl + void [[func]]() override; }; void test(Derived* D) { D->[[func]]();