kbobyrev created this revision.
kbobyrev added reviewers: ilya-biryukov, kadircet.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: usaxena95, arphaman, mgrang, jkorous, MaskRay.
When asked for references during cross-file rename, index might return implicit
references to the renamed symbol (such as those in macro expansions). To fix
the incorrect behavior, this patch introduces basic filtering machinery which
ensures that all ranges where renaming is about to be applied actually contains
the identifier the user asked to rename.
https://reviews.llvm.org/D71598
Files:
clang-tools-extra/clangd/SourceCode.cpp
clang-tools-extra/clangd/SourceCode.h
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/unittests/RenameTests.cpp
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -728,6 +728,22 @@
llvm::StringRef FooH;
llvm::StringRef FooCC;
} Cases[] = {
+ {
+ // 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",
+ },
{
// classes.
R"cpp(
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -14,17 +14,22 @@
#include "Selection.h"
#include "SourceCode.h"
#include "index/SymbolCollector.h"
+#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclTemplate.h"
#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
#include "clang/Tooling/Refactoring/Rename/USRFindingAction.h"
#include "clang/Tooling/Syntax/Tokens.h"
#include "llvm/ADT/None.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/FormatVariadic.h"
#include <algorithm>
+#include <string>
+#include <vector>
namespace clang {
namespace clangd {
@@ -340,12 +345,11 @@
//
// FIXME: Add range patching heuristics to detect staleness of the index, and
// report to users.
-// FIXME: Our index may return implicit references, which are not eligible for
-// rename, we should filter out these references.
llvm::Expected<FileEdits> renameOutsideFile(
const NamedDecl &RenameDecl, llvm::StringRef MainFilePath,
llvm::StringRef NewName, const SymbolIndex &Index,
- llvm::function_ref<llvm::Expected<std::string>(PathRef)> GetFileContent) {
+ llvm::function_ref<llvm::Expected<std::string>(PathRef)> GetFileContent,
+ const SourceManager &SM) {
auto AffectedFiles =
findOccurrencesOutsideFile(RenameDecl, MainFilePath, Index);
if (!AffectedFiles)
@@ -359,10 +363,16 @@
elog("Fail to read file content: {0}", AffectedFileCode.takeError());
continue;
}
- auto RenameRanges =
- adjustRenameRanges(*AffectedFileCode, RenameDecl.getNameAsString(),
- std::move(FileAndOccurrences.second),
- RenameDecl.getASTContext().getLangOpts());
+
+ // Filter out possible implicit references returned from the index.
+ const auto FilteredRanges = filterRenameRanges(
+ SM, FileAndOccurrences.first(),
+ RenameDecl.getASTContext().getLangOpts(), *AffectedFileCode,
+ std::move(FileAndOccurrences.second), RenameDecl.getNameAsString());
+
+ auto RenameRanges = adjustRenameRanges(
+ *AffectedFileCode, RenameDecl.getNameAsString(),
+ std::move(FilteredRanges), RenameDecl.getASTContext().getLangOpts());
if (!RenameRanges) {
// Our heuristics fails to adjust rename ranges to the current state of
// the file, it is most likely the index is stale, so we give up the
@@ -508,7 +518,7 @@
if (RInputs.Index) {
auto OtherFilesEdits =
renameOutsideFile(*RenameDecl, RInputs.MainFilePath, RInputs.NewName,
- *RInputs.Index, GetFileContent);
+ *RInputs.Index, GetFileContent, SM);
if (!OtherFilesEdits)
return OtherFilesEdits.takeError();
Results = std::move(*OtherFilesEdits);
Index: clang-tools-extra/clangd/SourceCode.h
===================================================================
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -298,6 +298,14 @@
bool isHeaderFile(llvm::StringRef FileName,
llvm::Optional<LangOptions> LangOpts = llvm::None);
+/// Removes ranges with implicit references to the renamed symbol (e.g. in macro
+/// expansions).
+std::vector<Range> filterRenameRanges(const SourceManager &SM,
+ StringRef Filename,
+ const LangOptions &Opts, StringRef Code,
+ std::vector<Range> RenameRanges,
+ StringRef OldName);
+
} // namespace clangd
} // namespace clang
#endif
Index: clang-tools-extra/clangd/SourceCode.cpp
===================================================================
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -214,6 +214,12 @@
return P;
}
+SourceLocation positionToSourceLoc(const SourceManager &SM, Position Pos,
+ StringRef Filename) {
+ const auto File = SM.getFileManager().getFile(Filename);
+ return SM.translateFileLineCol(*File, Pos.line, Pos.character);
+}
+
bool isSpelledInSource(SourceLocation Loc, const SourceManager &SM) {
if (Loc.isMacroID()) {
std::string PrintLoc = SM.getSpellingLoc(Loc).printToString(SM);
@@ -1127,5 +1133,31 @@
return Lang != types::TY_INVALID && types::onlyPrecompileType(Lang);
}
+std::vector<Range> filterRenameRanges(const SourceManager &SM,
+ StringRef Filename,
+ const LangOptions &Opts, StringRef Code,
+ std::vector<Range> RenameRanges,
+ StringRef OldName) {
+ // Sort candidates for renaming and incrementally iterate through them.
+ llvm::sort(RenameRanges);
+ auto CurrentRange = begin(RenameRanges);
+ std::vector<Range> Result;
+ lex(Code, Opts, [&](const clang::Token &Tok, const SourceManager &SM) {
+ if (Tok.getKind() != tok::raw_identifier)
+ return;
+ auto Range = getTokenRange(SM, Opts, Tok.getLocation());
+ if (!Range)
+ return;
+ if (Tok.getRawIdentifier() != OldName)
+ return;
+ // Get the next candidate if the candidate iterator lags behind.
+ while (*CurrentRange < *Range && CurrentRange != end(RenameRanges))
+ ++CurrentRange;
+ if (*CurrentRange == *Range)
+ Result.push_back(*CurrentRange);
+ });
+ return Result;
+}
+
} // namespace clangd
} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
