Index: clang-tools-extra/clangd/refactor/Rename.h =================================================================== --- clang-tools-extra/clangd/refactor/Rename.h +++ clang-tools-extra/clangd/refactor/Rename.h @@ -77,6 +77,7 @@ /// Exposed for testing only. /// /// REQUIRED: Indexed and Lexed are sorted. +/// REQUIRED: Indexed.size() <= Lexed.size(). llvm::Optional> getMappedRanges(ArrayRef Indexed, ArrayRef Lexed); /// Evaluates how good the mapped result is. 0 indicates a perfect match. Index: clang-tools-extra/clangd/refactor/Rename.cpp =================================================================== --- clang-tools-extra/clangd/refactor/Rename.cpp +++ clang-tools-extra/clangd/refactor/Rename.cpp @@ -425,6 +425,22 @@ LexedIndex + 1, Fuel, MatchedCB); } +// Assume that index is stale/has returned invalid results that can be filtered +// by finding the intersection between both sets. +// REQUIRES: Indexed and Lexed are sorted. +llvm::Optional> filterIndexResults(ArrayRef Indexed, + ArrayRef Lexed) { + assert(std::is_sorted(Indexed.begin(), Indexed.end())); + assert(std::is_sorted(Lexed.begin(), Lexed.end())); + assert(Indexed.size() > Lexed.size()); + std::vector Result; + std::set_intersection(Lexed.begin(), Lexed.end(), Indexed.begin(), + Indexed.end(), std::back_inserter(Result)); + if (Result.empty()) + return llvm::None; + return Result; +} + } // namespace llvm::Expected rename(const RenameInputs &RInputs) { @@ -571,8 +587,16 @@ } // Details: -// - lex the draft code to get all rename candidates, this yields a superset -// of candidates. +// - lex the draft code to get all rename candidates, this yields a set of +// candidates. It may be a superset of candidates returned from the index +// in case index is stale and there are new references to the renamed entity. +// It may also be a subset of candidates from the index in case when index +// returns some incorrect results (such as implicit references), when some +// references to the renamed entity have been removed or simply when +// local variables (references to which are not stored in index) have the +// same identifier name. +// - if the lexed ranges are subset of index candidates, try to filter the +// results from index by exactly matching existing ranges from lexer. // - apply range patching heuristics to generate "authoritative" occurrences, // cases we consider: // (a) index returns a subset of candidates, we use the indexed results. @@ -591,7 +615,8 @@ std::vector Lexed = collectIdentifierRanges(Identifier, DraftCode, LangOpts); llvm::sort(Lexed); - return getMappedRanges(Indexed, Lexed); + return Indexed.size() <= Lexed.size() ? getMappedRanges(Indexed, Lexed) + : filterIndexResults(Indexed, Lexed); } llvm::Optional> getMappedRanges(ArrayRef Indexed, @@ -599,11 +624,8 @@ assert(!Indexed.empty()); assert(std::is_sorted(Indexed.begin(), Indexed.end())); assert(std::is_sorted(Lexed.begin(), Lexed.end())); + assert(Indexed.size() <= Lexed.size()); - if (Indexed.size() > Lexed.size()) { - vlog("The number of lexed occurrences is less than indexed occurrences"); - return llvm::None; - } // Fast check for the special subset case. if (std::includes(Indexed.begin(), Indexed.end(), Lexed.begin(), Lexed.end())) return Indexed.vec(); Index: clang-tools-extra/clangd/unittests/RenameTests.cpp =================================================================== --- clang-tools-extra/clangd/unittests/RenameTests.cpp +++ clang-tools-extra/clangd/unittests/RenameTests.cpp @@ -868,6 +868,22 @@ } )cpp", }, + { + // Macros and implicit references. + 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) { @@ -1013,11 +1029,6 @@ llvm::StringRef IndexedCode; llvm::StringRef LexedCode; } Tests[] = { - { - // no lexed ranges. - "[[]]", - "", - }, { // both line and column are changed, not a near miss. R"([[]])",