tom-anders created this revision.
tom-anders added a reviewer: kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
tom-anders requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
Fixes https://github.com/clangd/clangd/issues/170
This patch actually consists of 2 fixes:
1. Add handling for UsingShadowDecl to canonicalRenameDecl(). This fixes the
issue described in https://github.com/clangd/clangd/issues/170.
2. Avoid the "there are multiple symbols under the cursor error" by applying
similar logic as in https://reviews.llvm.org/D133664. This also partly fixes
https://github.com/clangd/clangd/issues/586.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D135489
Files:
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
@@ -817,6 +817,18 @@
char [[da^ta]];
} @end
)cpp",
+
+ // Issue 170: Rename symbol introduced by UsingDecl
+ R"cpp(
+ namespace ns { void [[f^oo]](); }
+
+ using ns::[[f^oo]];
+
+ void f() {
+ [[f^oo]]();
+ auto p = &[[f^oo]];
+ }
+ )cpp",
};
llvm::StringRef NewName = "NewName";
for (llvm::StringRef T : Tests) {
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -133,6 +133,10 @@
if (const VarDecl *OriginalVD = VD->getInstantiatedFromStaticDataMember())
return canonicalRenameDecl(OriginalVD);
}
+ if (const auto *UD = dyn_cast<UsingShadowDecl>(D)) {
+ if (const auto* TargetDecl = UD->getTargetDecl())
+ return canonicalRenameDecl(TargetDecl);
+ }
return dyn_cast<NamedDecl>(D->getCanonicalDecl());
}
@@ -157,6 +161,23 @@
return Result;
}
+// For something like
+// namespace ns { void foo(); }
+// void bar() { using ns::foo; f^oo(); }
+// locateDeclAt() will return a BaseUsingDecl and foo's actual declaration.
+// For renaming, we're only interested in foo's declaration, so drop the other
one
+void filterBaseUsingDecl(llvm::DenseSet<const NamedDecl *>& Decls) {
+ if (Decls.size() == 2) {
+ auto FirstDecl = Decls.begin();
+ auto SecondDecl = std::next(Decls.begin());
+
+ if (llvm::isa<BaseUsingDecl>(*FirstDecl))
+ Decls.erase(FirstDecl);
+ else if (llvm::isa<BaseUsingDecl>(*SecondDecl))
+ Decls.erase(SecondDecl);
+ }
+}
+
// By default, we exclude symbols from system headers and protobuf symbols as
// renaming these symbols would change system/generated files which are
unlikely
// to be good candidates for modification.
@@ -737,6 +758,7 @@
return makeError(ReasonToReject::UnsupportedSymbol);
auto DeclsUnderCursor = locateDeclAt(AST, IdentifierToken->location());
+ filterBaseUsingDecl(DeclsUnderCursor);
if (DeclsUnderCursor.empty())
return makeError(ReasonToReject::NoSymbolFound);
if (DeclsUnderCursor.size() > 1)
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -817,6 +817,18 @@
char [[da^ta]];
} @end
)cpp",
+
+ // Issue 170: Rename symbol introduced by UsingDecl
+ R"cpp(
+ namespace ns { void [[f^oo]](); }
+
+ using ns::[[f^oo]];
+
+ void f() {
+ [[f^oo]]();
+ auto p = &[[f^oo]];
+ }
+ )cpp",
};
llvm::StringRef NewName = "NewName";
for (llvm::StringRef T : Tests) {
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -133,6 +133,10 @@
if (const VarDecl *OriginalVD = VD->getInstantiatedFromStaticDataMember())
return canonicalRenameDecl(OriginalVD);
}
+ if (const auto *UD = dyn_cast<UsingShadowDecl>(D)) {
+ if (const auto* TargetDecl = UD->getTargetDecl())
+ return canonicalRenameDecl(TargetDecl);
+ }
return dyn_cast<NamedDecl>(D->getCanonicalDecl());
}
@@ -157,6 +161,23 @@
return Result;
}
+// For something like
+// namespace ns { void foo(); }
+// void bar() { using ns::foo; f^oo(); }
+// locateDeclAt() will return a BaseUsingDecl and foo's actual declaration.
+// For renaming, we're only interested in foo's declaration, so drop the other one
+void filterBaseUsingDecl(llvm::DenseSet<const NamedDecl *>& Decls) {
+ if (Decls.size() == 2) {
+ auto FirstDecl = Decls.begin();
+ auto SecondDecl = std::next(Decls.begin());
+
+ if (llvm::isa<BaseUsingDecl>(*FirstDecl))
+ Decls.erase(FirstDecl);
+ else if (llvm::isa<BaseUsingDecl>(*SecondDecl))
+ Decls.erase(SecondDecl);
+ }
+}
+
// By default, we exclude symbols from system headers and protobuf symbols as
// renaming these symbols would change system/generated files which are unlikely
// to be good candidates for modification.
@@ -737,6 +758,7 @@
return makeError(ReasonToReject::UnsupportedSymbol);
auto DeclsUnderCursor = locateDeclAt(AST, IdentifierToken->location());
+ filterBaseUsingDecl(DeclsUnderCursor);
if (DeclsUnderCursor.empty())
return makeError(ReasonToReject::NoSymbolFound);
if (DeclsUnderCursor.size() > 1)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
