kbobyrev updated this revision to Diff 239503. kbobyrev added a comment. Move added rename unit test to the end of the list.
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72746/new/ https://reviews.llvm.org/D72746 Files: clang-tools-extra/clangd/index/Ref.h clang-tools-extra/clangd/index/SymbolCollector.cpp clang-tools-extra/clangd/index/SymbolCollector.h clang-tools-extra/clangd/refactor/Rename.cpp clang-tools-extra/clangd/unittests/RenameTests.cpp clang/include/clang/Tooling/Syntax/Tokens.h clang/lib/Tooling/Syntax/Tokens.cpp
Index: clang/lib/Tooling/Syntax/Tokens.cpp =================================================================== --- clang/lib/Tooling/Syntax/Tokens.cpp +++ clang/lib/Tooling/Syntax/Tokens.cpp @@ -256,21 +256,27 @@ llvm::ArrayRef<syntax::Token> syntax::spelledTokensTouching(SourceLocation Loc, - const syntax::TokenBuffer &Tokens) { + llvm::ArrayRef<syntax::Token> Tokens) { assert(Loc.isFileID()); - llvm::ArrayRef<syntax::Token> All = - Tokens.spelledTokens(Tokens.sourceManager().getFileID(Loc)); auto *Right = llvm::partition_point( - All, [&](const syntax::Token &Tok) { return Tok.location() < Loc; }); - bool AcceptRight = Right != All.end() && Right->location() <= Loc; - bool AcceptLeft = Right != All.begin() && (Right - 1)->endLocation() >= Loc; + Tokens, [&](const syntax::Token &Tok) { return Tok.location() < Loc; }); + bool AcceptRight = Right != Tokens.end() && Right->location() <= Loc; + bool AcceptLeft = + Right != Tokens.begin() && (Right - 1)->endLocation() >= Loc; return llvm::makeArrayRef(Right - (AcceptLeft ? 1 : 0), Right + (AcceptRight ? 1 : 0)); } +llvm::ArrayRef<syntax::Token> +syntax::spelledTokensTouching(SourceLocation Loc, + const syntax::TokenBuffer &Tokens) { + return spelledTokensTouching( + Loc, Tokens.spelledTokens(Tokens.sourceManager().getFileID(Loc))); +} + const syntax::Token * syntax::spelledIdentifierTouching(SourceLocation Loc, - const syntax::TokenBuffer &Tokens) { + llvm::ArrayRef<syntax::Token> Tokens) { for (const syntax::Token &Tok : spelledTokensTouching(Loc, Tokens)) { if (Tok.kind() == tok::identifier) return &Tok; @@ -278,6 +284,13 @@ return nullptr; } +const syntax::Token * +syntax::spelledIdentifierTouching(SourceLocation Loc, + const syntax::TokenBuffer &Tokens) { + return spelledIdentifierTouching( + Loc, Tokens.spelledTokens(Tokens.sourceManager().getFileID(Loc))); +} + std::vector<const syntax::Token *> TokenBuffer::macroExpansions(FileID FID) const { auto FileIt = Files.find(FID); Index: clang/include/clang/Tooling/Syntax/Tokens.h =================================================================== --- clang/include/clang/Tooling/Syntax/Tokens.h +++ clang/include/clang/Tooling/Syntax/Tokens.h @@ -313,11 +313,22 @@ const SourceManager *SourceMgr; }; +/// The spelled tokens that overlap or touch a spelling location Loc. +/// This always returns 0-2 tokens. +llvm::ArrayRef<syntax::Token> +spelledTokensTouching(SourceLocation Loc, llvm::ArrayRef<syntax::Token> Tokens); + /// The spelled tokens that overlap or touch a spelling location Loc. /// This always returns 0-2 tokens. llvm::ArrayRef<syntax::Token> spelledTokensTouching(SourceLocation Loc, const syntax::TokenBuffer &Tokens); +/// The identifier token that overlaps or touches a spelling location Loc. +/// If there is none, returns nullptr. +const syntax::Token * +spelledIdentifierTouching(SourceLocation Loc, + llvm::ArrayRef<syntax::Token> Tokens); + /// The identifier token that overlaps or touches a spelling location Loc. /// If there is none, returns nullptr. const syntax::Token * Index: clang-tools-extra/clangd/unittests/RenameTests.cpp =================================================================== --- clang-tools-extra/clangd/unittests/RenameTests.cpp +++ clang-tools-extra/clangd/unittests/RenameTests.cpp @@ -33,7 +33,7 @@ // Convert a Range to a Ref. Ref refWithRange(const clangd::Range &Range, const std::string &URI) { Ref Result; - Result.Kind = RefKind::Reference; + Result.Kind = RefKind::Reference | RefKind::Spelled; Result.Location.Start.setLine(Range.start.line); Result.Location.Start.setColumn(Range.start.character); Result.Location.End.setLine(Range.end.line); @@ -837,7 +837,7 @@ { // variables. R"cpp( - static const int [[VA^R]] = 123; + static const int [[VA^R]] = 123; )cpp", R"cpp( #include "foo.h" @@ -868,6 +868,22 @@ } )cpp", }, + { + // Implicit references in macro expansions. + R"cpp( + class [[Fo^o]] {}; + #define FooFoo Foo + #define FOO Foo + )cpp", + R"cpp( + #include "foo.h" + void bar() { + [[Foo]] x; + FOO y; + FooFoo z; + } + )cpp", + }, }; for (const auto& T : Cases) { Index: clang-tools-extra/clangd/refactor/Rename.cpp =================================================================== --- clang-tools-extra/clangd/refactor/Rename.cpp +++ clang-tools-extra/clangd/refactor/Rename.cpp @@ -296,6 +296,8 @@ bool HasMore = Index.refs(RQuest, [&](const Ref &R) { if (AffectedFiles.size() > MaxLimitFiles) return; + if (!static_cast<unsigned>(R.Kind & RefKind::Spelled)) + return; if (auto RefFilePath = filePath(R.Location, /*HintFilePath=*/MainFile)) { if (*RefFilePath != MainFile) AffectedFiles[*RefFilePath].push_back(toRange(R.Location)); Index: clang-tools-extra/clangd/index/SymbolCollector.h =================================================================== --- clang-tools-extra/clangd/index/SymbolCollector.h +++ clang-tools-extra/clangd/index/SymbolCollector.h @@ -19,6 +19,7 @@ #include "clang/Index/IndexDataConsumer.h" #include "clang/Index/IndexSymbol.h" #include "clang/Sema/CodeCompleteConsumer.h" +#include "clang/Tooling/Syntax/Tokens.h" #include "llvm/ADT/DenseMap.h" #include "llvm/Support/Regex.h" #include <functional> @@ -169,6 +170,7 @@ // Cache whether to index a file or not. llvm::DenseMap<FileID, bool> FilesToIndexCache; llvm::DenseMap<FileID, bool> HeaderIsSelfContainedCache; + llvm::DenseMap<FileID, std::vector<syntax::Token>> FilesToTokensCache; }; } // namespace clangd Index: clang-tools-extra/clangd/index/SymbolCollector.cpp =================================================================== --- clang-tools-extra/clangd/index/SymbolCollector.cpp +++ clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -28,6 +28,7 @@ #include "clang/Index/IndexingAction.h" #include "clang/Index/USRGeneration.h" #include "clang/Lex/Preprocessor.h" +#include "clang/Tooling/Syntax/Tokens.h" #include "llvm/Support/Casting.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/MemoryBuffer.h" @@ -180,7 +181,16 @@ } RefKind toRefKind(index::SymbolRoleSet Roles) { - return static_cast<RefKind>(static_cast<unsigned>(RefKind::All) & Roles); + RefKind Result = RefKind::Unknown; + if (Roles & static_cast<unsigned>(index::SymbolRole::Declaration)) + Result |= RefKind::Declaration; + if (Roles & static_cast<unsigned>(index::SymbolRole::Definition)) + Result |= RefKind::Definition; + if (Roles & static_cast<unsigned>(index::SymbolRole::Reference)) + Result |= RefKind::Reference; + if (!(Roles & static_cast<unsigned>(index::SymbolRole::Implicit))) + Result |= RefKind::Spelled; + return Result; } bool shouldIndexRelation(const index::SymbolRelation &R) { @@ -291,7 +301,7 @@ // occurrence inside the base-specifier. processRelations(*ND, *ID, Relations); - bool CollectRef = static_cast<unsigned>(Opts.RefFilter) & Roles; + bool CollectRef = static_cast<unsigned>(Opts.RefFilter & toRefKind(Roles)); bool IsOnlyRef = !(Roles & (static_cast<unsigned>(index::SymbolRole::Declaration) | static_cast<unsigned>(index::SymbolRole::Definition))); @@ -577,11 +587,32 @@ CollectRef(IDAndRefs.first, LocAndRole); } // Populate Refs slab from DeclRefs. - if (auto MainFileURI = GetURI(SM.getMainFileID())) { - for (const auto &It : DeclRefs) { - if (auto ID = getSymbolID(It.first)) { - for (const auto &LocAndRole : It.second) + const auto MainFileID = SM.getMainFileID(); + if (const auto MainFileURI = GetURI(MainFileID)) { + assert(ASTCtx && "ASTContext must be set."); + // FIXME: It's better to use TokenBuffer by passing spelled tokens from the + // caller of SymbolCollector. + if (!FilesToTokensCache.count(MainFileID)) + FilesToTokensCache[MainFileID] = + syntax::tokenize(MainFileID, SM, ASTCtx->getLangOpts()); + const auto Tokens = FilesToTokensCache[MainFileID]; + for (auto &DeclAndRef : DeclRefs) { + if (auto ID = getSymbolID(DeclAndRef.first)) { + for (auto &LocAndRole : DeclAndRef.second) { + // Check if the referenced symbol is spelled exactly the same way the + // corresponding NamedDecl is. If it isn't, mark this reference as + // implicit. An example of implicit references would be a macro + // expansion. + const auto *IdentifierToken = + spelledIdentifierTouching(LocAndRole.first, Tokens); + DeclarationName Name = DeclAndRef.first->getDeclName(); + if (!IdentifierToken || + (Name.isIdentifier() && + Name.getAsString() != IdentifierToken->text(SM))) + LocAndRole.second |= + static_cast<uint32_t>(index::SymbolRole::Implicit); CollectRef(*ID, LocAndRole); + } } } } Index: clang-tools-extra/clangd/index/Ref.h =================================================================== --- clang-tools-extra/clangd/index/Ref.h +++ clang-tools-extra/clangd/index/Ref.h @@ -27,10 +27,11 @@ /// This is a bitfield which can be combined from different kinds. enum class RefKind : uint8_t { Unknown = 0, - Declaration = static_cast<uint8_t>(index::SymbolRole::Declaration), - Definition = static_cast<uint8_t>(index::SymbolRole::Definition), - Reference = static_cast<uint8_t>(index::SymbolRole::Reference), - All = Declaration | Definition | Reference, + Declaration = 1 << 0, + Definition = 1 << 1, + Reference = 1 << 2, + Spelled = 1 << 3, + All = Declaration | Definition | Reference | Spelled, }; inline RefKind operator|(RefKind L, RefKind R) {
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits