Index: clangd/XRefs.cpp =================================================================== --- clangd/XRefs.cpp +++ clangd/XRefs.cpp @@ -79,10 +79,10 @@ bool handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles, - ArrayRef Relations, FileID FID, - unsigned Offset, + ArrayRef Relations, + SourceLocation Loc, index::IndexDataConsumer::ASTNodeInfo ASTNode) override { - if (isSearchedLocation(FID, Offset)) { + if (Loc == SearchedLocation) { // Find and add definition declarations (for GoToDefinition). // We don't use parameter `D`, as Parameter `D` is the canonical // declaration, which is the first declaration of a redeclarable @@ -98,18 +98,12 @@ } private: - bool isSearchedLocation(FileID FID, unsigned Offset) const { - const SourceManager &SourceMgr = AST.getSourceManager(); - return SourceMgr.getFileOffset(SearchedLocation) == Offset && - SourceMgr.getFileID(SearchedLocation) == FID; - } - void finish() override { // Also handle possible macro at the searched location. Token Result; auto &Mgr = AST.getSourceManager(); - if (!Lexer::getRawToken(SearchedLocation, Result, Mgr, AST.getLangOpts(), - false)) { + if (!Lexer::getRawToken(Mgr.getSpellingLoc(SearchedLocation), Result, Mgr, + AST.getLangOpts(), false)) { if (Result.is(tok::raw_identifier)) { PP.LookUpIdentifierInfo(Result); } @@ -127,16 +121,7 @@ MacroInfo *MacroInf = MacroDef.getMacroInfo(); if (MacroInf) { MacroInfos.push_back(MacroDecl{IdentifierInfo->getName(), MacroInf}); - // Clear all collected delcarations if this is a macro search. - // - // In theory, there should be no declarataions being collected when we - // search a source location that refers to a macro. - // The occurrence location returned by `handleDeclOccurence` is - // limited (FID, Offset are from expansion location), we will collect - // all declarations inside the macro. - // - // FIXME: Avoid adding decls from inside macros in handlDeclOccurence. - Decls.clear(); + assert(Decls.empty()); } } } @@ -252,21 +237,19 @@ bool handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles, - ArrayRef Relations, FileID FID, - unsigned Offset, + ArrayRef Relations, + SourceLocation Loc, index::IndexDataConsumer::ASTNodeInfo ASTNode) override { const SourceManager &SourceMgr = AST.getSourceManager(); - if (SourceMgr.getMainFileID() != FID || + SourceLocation HighlightStartLoc = SourceMgr.getFileLoc(Loc); + if (SourceMgr.getMainFileID() != SourceMgr.getFileID(HighlightStartLoc) || std::find(Decls.begin(), Decls.end(), D) == Decls.end()) { return true; } SourceLocation End; const LangOptions &LangOpts = AST.getLangOpts(); - SourceLocation StartOfFileLoc = SourceMgr.getLocForStartOfFile(FID); - SourceLocation HightlightStartLoc = StartOfFileLoc.getLocWithOffset(Offset); - End = - Lexer::getLocForEndOfToken(HightlightStartLoc, 0, SourceMgr, LangOpts); - SourceRange SR(HightlightStartLoc, End); + End = Lexer::getLocForEndOfToken(HighlightStartLoc, 0, SourceMgr, LangOpts); + SourceRange SR(HighlightStartLoc, End); DocumentHighlightKind Kind = DocumentHighlightKind::Text; if (static_cast(index::SymbolRole::Write) & Roles) Index: clangd/index/SymbolCollector.h =================================================================== --- clangd/index/SymbolCollector.h +++ clangd/index/SymbolCollector.h @@ -59,8 +59,8 @@ bool handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles, - ArrayRef Relations, FileID FID, - unsigned Offset, + ArrayRef Relations, + SourceLocation Loc, index::IndexDataConsumer::ASTNodeInfo ASTNode) override; SymbolSlab takeSymbols() { return std::move(Symbols).build(); } Index: clangd/index/SymbolCollector.cpp =================================================================== --- clangd/index/SymbolCollector.cpp +++ clangd/index/SymbolCollector.cpp @@ -225,7 +225,7 @@ // Always return true to continue indexing. bool SymbolCollector::handleDeclOccurence( const Decl *D, index::SymbolRoleSet Roles, - ArrayRef Relations, FileID FID, unsigned Offset, + ArrayRef Relations, SourceLocation Loc, index::IndexDataConsumer::ASTNodeInfo ASTNode) { assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set."); assert(CompletionAllocator && CompletionTUInfo); @@ -235,9 +235,10 @@ // Mark D as referenced if this is a reference coming from the main file. // D may not be an interesting symbol, but it's cheaper to check at the end. + auto &SM = ASTCtx->getSourceManager(); if (Opts.CountReferences && (Roles & static_cast(index::SymbolRole::Reference)) && - ASTCtx->getSourceManager().getMainFileID() == FID) + SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID()) ReferencedDecls.insert(ND); // Don't continue indexing if this is a mere reference. Index: unittests/clangd/SymbolCollectorTests.cpp =================================================================== --- unittests/clangd/SymbolCollectorTests.cpp +++ unittests/clangd/SymbolCollectorTests.cpp @@ -250,6 +250,7 @@ class Y; class Z {}; // not used anywhere Y* y = nullptr; // used in header doesn't count + #define GLOBAL_Z(name) Z name; )"; const std::string Main = R"( W* w = nullptr; @@ -258,6 +259,7 @@ class V; V* v = nullptr; // Used, but not eligible for indexing. class Y{}; // definition doesn't count as a reference + GLOBAL_Z(z); // Not a reference to Z, we don't spell the type. )"; CollectorOpts.CountReferences = true; runSymbolCollector(Header, Main); Index: unittests/clangd/XRefsTests.cpp =================================================================== --- unittests/clangd/XRefsTests.cpp +++ unittests/clangd/XRefsTests.cpp @@ -222,6 +222,18 @@ } )cpp", + R"cpp(// Macro argument + int [[i]]; + #define ADDRESSOF(X) &X; + int *j = ADDRESSOF(^i); + )cpp", + + R"cpp(// Symbol concatenated inside macro (not supported) + int *pi; + #define POINTER(X) p # X; + int i = *POINTER(^i); + )cpp", + R"cpp(// Forward class declaration class Foo; class [[Foo]] {}; @@ -249,8 +261,11 @@ for (const char *Test : Tests) { Annotations T(Test); auto AST = build(T.code()); + std::vector> ExpectedLocations; + for (const auto &R : T.ranges()) + ExpectedLocations.push_back(RangeIs(R)); EXPECT_THAT(findDefinitions(AST, T.point()), - ElementsAre(RangeIs(T.range()))) + ElementsAreArray(ExpectedLocations)) << Test; } }